[Git][ghc/ghc][master] 4 commits: rts: Statically assert alignment of Capability
Marge Bot (@marge-bot)
gitlab at gitlab.haskell.org
Fri Mar 3 03:43:44 UTC 2023
Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC
Commits:
db83f8bb by Ben Gamari at 2023-03-02T22:43:22-05:00
rts: Statically assert alignment of Capability
In #22965 we noticed that changes in the size of `Capability` can result
in unsound behavior due to the `align` pragma claiming an alignment
which we don't in practice observe. Avoid this by statically asserting
that the size is a multiple of the alignment.
- - - - -
5f7a4a6d by Ben Gamari at 2023-03-02T22:43:22-05:00
rts: Introduce stgMallocAlignedBytes
- - - - -
8a6f745d by Ben Gamari at 2023-03-02T22:43:22-05:00
rts: Correctly align Capability allocations
Previously we failed to tell the C allocator that `Capability`s needed
to be aligned, resulting in #22965.
Fixes #22965.
Fixes #22975.
- - - - -
5464c73f by Ben Gamari at 2023-03-02T22:43:22-05:00
rts: Drop no-alignment special case for Windows
For reasons that aren't clear, we were previously not giving Capability
the same favorable alignment on Windows that we provided on other
platforms. Fix this.
- - - - -
4 changed files:
- rts/Capability.c
- rts/Capability.h
- rts/RtsUtils.c
- rts/RtsUtils.h
Changes:
=====================================
rts/Capability.c
=====================================
@@ -438,8 +438,9 @@ moreCapabilities (uint32_t from USED_IF_THREADS, uint32_t to USED_IF_THREADS)
{
for (uint32_t i = 0; i < to; i++) {
if (i >= from) {
- capabilities[i] = stgMallocBytes(sizeof(Capability),
- "moreCapabilities");
+ capabilities[i] = stgMallocAlignedBytes(sizeof(Capability),
+ CAPABILITY_ALIGNMENT,
+ "moreCapabilities");
initCapability(capabilities[i], i);
}
}
@@ -1274,7 +1275,7 @@ freeCapabilities (void)
Capability *cap = getCapability(i);
freeCapability(cap);
if (cap != &MainCapability) {
- stgFree(cap);
+ stgFreeAligned(cap);
}
}
#else
=====================================
rts/Capability.h
=====================================
@@ -28,6 +28,14 @@
#include "BeginPrivate.h"
+// We never want a Capability to overlap a cache line with
+// anything else, so round it up to a cache line size:
+#if defined(s390x_HOST_ARCH)
+#define CAPABILITY_ALIGNMENT 256
+#else
+#define CAPABILITY_ALIGNMENT 64
+#endif
+
/* N.B. This must be consistent with CapabilityPublic in RtsAPI.h */
struct Capability_ {
// State required by the STG virtual machine when running Haskell
@@ -169,14 +177,12 @@ struct Capability_ {
StgTRecHeader *free_trec_headers;
uint32_t transaction_tokens;
} // typedef Capability is defined in RtsAPI.h
- // We never want a Capability to overlap a cache line with anything
- // else, so round it up to a cache line size:
-#if defined(s390x_HOST_ARCH)
- ATTRIBUTE_ALIGNED(256)
-#elif !defined(mingw32_HOST_OS)
- ATTRIBUTE_ALIGNED(64)
-#endif
- ;
+ ATTRIBUTE_ALIGNED(CAPABILITY_ALIGNMENT)
+;
+
+// We allocate arrays of Capabilities therefore we must ensure that the size is
+// a multiple of the claimed alignment
+GHC_STATIC_ASSERT(sizeof(struct Capability_) % CAPABILITY_ALIGNMENT == 0, "Capability size does not match cache size");
#if defined(THREADED_RTS)
#define ASSERT_TASK_ID(task) ASSERT(task->id == osThreadId())
=====================================
rts/RtsUtils.c
=====================================
@@ -57,9 +57,9 @@ extern char *ctime_r(const time_t *, char *);
void *
stgMallocBytes (size_t n, char *msg)
{
- void *space;
+ void *space = malloc(n);
- if ((space = malloc(n)) == NULL) {
+ if (space == NULL) {
/* Quoting POSIX.1-2008 (which says more or less the same as ISO C99):
*
* "Upon successful completion with size not equal to 0, malloc() shall
@@ -128,6 +128,53 @@ stgFree(void* p)
free(p);
}
+// N.B. Allocations resulting from this function must be freed by
+// `stgFreeAligned`, not `stgFree`. This is necessary due to the properties of Windows' `_aligned_malloc`
+void *
+stgMallocAlignedBytes (size_t n, size_t align, char *msg)
+{
+ void *space;
+
+#if defined(mingw32_HOST_OS)
+ space = _aligned_malloc(n, align);
+#else
+ if (posix_memalign(&space, align, n)) {
+ space = NULL; // Allocation failed
+ }
+#endif
+
+ if (space == NULL) {
+ /* Quoting POSIX.1-2008 (which says more or less the same as ISO C99):
+ *
+ * "Upon successful completion with size not equal to 0, malloc() shall
+ * return a pointer to the allocated space. If size is 0, either a null
+ * pointer or a unique pointer that can be successfully passed to free()
+ * shall be returned. Otherwise, it shall return a null pointer and set
+ * errno to indicate the error."
+ *
+ * Consequently, a NULL pointer being returned by `malloc()` for a 0-size
+ * allocation is *not* to be considered an error.
+ */
+ if (n == 0) return NULL;
+
+ /* don't fflush(stdout); WORKAROUND bug in Linux glibc */
+ rtsConfig.mallocFailHook((W_) n, msg);
+ stg_exit(EXIT_INTERNAL_ERROR);
+ }
+ IF_DEBUG(zero_on_gc, memset(space, 0xbb, n));
+ return space;
+}
+
+void
+stgFreeAligned (void *p)
+{
+#if defined(mingw32_HOST_OS)
+ _aligned_free(p);
+#else
+ free(p);
+#endif
+}
+
/* -----------------------------------------------------------------------------
Stack/heap overflow
-------------------------------------------------------------------------- */
=====================================
rts/RtsUtils.h
=====================================
@@ -29,16 +29,9 @@ void *stgMallocBytes(size_t n, char *msg)
* See: https://gitlab.haskell.org/ghc/ghc/-/issues/22380
*/
-void *stgReallocBytes(void *p, size_t n, char *msg)
- STG_MALLOC1(stgFree)
- STG_ALLOC_SIZE1(2)
- STG_RETURNS_NONNULL;
-/* Note: `stgRallocBytes` can *not* be tagged as `STG_MALLOC`
- * since its return value *can* alias an existing pointer (i.e.,
- * the given pointer `p`).
- * See the documentation of the `malloc` attribute in the GCC manual
- * for more information.
- */
+void *stgMallocAlignedBytes(size_t n, size_t align, char *msg);
+
+void *stgReallocBytes(void *p, size_t n, char *msg);
void *stgCallocBytes(size_t count, size_t size, char *msg)
STG_MALLOC STG_MALLOC1(stgFree)
@@ -48,6 +41,10 @@ void *stgCallocBytes(size_t count, size_t size, char *msg)
char *stgStrndup(const char *s, size_t n)
STG_MALLOC STG_MALLOC1(stgFree);
+void *stgMallocAlignedBytes(size_t n, size_t align, char *msg);
+
+void stgFreeAligned(void *p);
+
/* -----------------------------------------------------------------------------
* Misc other utilities
* -------------------------------------------------------------------------- */
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/8919f34102cae1ff3bae95b7f53e5d93dbad7ecf...5464c73f192f76e75160e8992fe9720d943ae611
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/8919f34102cae1ff3bae95b7f53e5d93dbad7ecf...5464c73f192f76e75160e8992fe9720d943ae611
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230302/6b3cc1a0/attachment-0001.html>
More information about the ghc-commits
mailing list