From 85ecac5f3f368342a7b7e2374d0eded615bdda2a Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 12 Mar 2018 17:18:26 +0000 Subject: [PATCH] [sanitizer] Align & pad the allocator structures to the cacheline size v2 Summary: This is a new version of D44261, which broke some builds with older gcc, as they can't align on a constexpr, but rather require an integer (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56859) among others. We introduce `SANITIZER_CACHE_LINE_SIZE` in `sanitizer_platform.h` to be used in `ALIGNED` attributes instead of using directly `kCacheLineSize`. Reviewers: alekseyshl, thakis Reviewed By: alekseyshl Subscribers: kubamracek, delcypher, #sanitizers, llvm-commits Differential Revision: https://reviews.llvm.org/D44326 llvm-svn: 327297 --- .../sanitizer_common/sanitizer_allocator_primary32.h | 8 +++----- .../sanitizer_common/sanitizer_allocator_primary64.h | 11 ++++++----- compiler-rt/lib/sanitizer_common/sanitizer_common.h | 6 +----- compiler-rt/lib/sanitizer_common/sanitizer_platform.h | 8 ++++++++ 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h index 4e87bd7fe011..ce7b11b6295d 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary32.h @@ -266,14 +266,12 @@ class SizeClassAllocator32 { static const uptr kRegionSize = 1 << kRegionSizeLog; static const uptr kNumPossibleRegions = kSpaceSize / kRegionSize; - struct SizeClassInfo { + struct ALIGNED(SANITIZER_CACHE_LINE_SIZE) SizeClassInfo { SpinMutex mutex; IntrusiveList free_list; u32 rand_state; - char padding[kCacheLineSize - 2 * sizeof(uptr) - - sizeof(IntrusiveList)]; }; - COMPILER_CHECK(sizeof(SizeClassInfo) == kCacheLineSize); + COMPILER_CHECK(sizeof(SizeClassInfo) % kCacheLineSize == 0); uptr ComputeRegionId(uptr mem) { const uptr res = mem >> kRegionSizeLog; @@ -299,7 +297,7 @@ class SizeClassAllocator32 { } SizeClassInfo *GetSizeClassInfo(uptr class_id) { - CHECK_LT(class_id, kNumClasses); + DCHECK_LT(class_id, kNumClasses); return &size_class_info_array[class_id]; } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h index 651a64b04f2e..255981733f3d 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h @@ -80,6 +80,8 @@ class SizeClassAllocator64 { } SetReleaseToOSIntervalMs(release_to_os_interval_ms); MapWithCallbackOrDie(SpaceEnd(), AdditionalSize()); + // Check that the RegionInfo array is aligned on the CacheLine size. + DCHECK_EQ(SpaceEnd() % kCacheLineSize, 0); } s32 ReleaseToOSIntervalMs() const { @@ -584,7 +586,7 @@ class SizeClassAllocator64 { u64 last_released_bytes; }; - struct RegionInfo { + struct ALIGNED(SANITIZER_CACHE_LINE_SIZE) RegionInfo { BlockingMutex mutex; uptr num_freed_chunks; // Number of elements in the freearray. uptr mapped_free_array; // Bytes mapped for freearray. @@ -597,12 +599,11 @@ class SizeClassAllocator64 { Stats stats; ReleaseToOsInfo rtoi; }; - COMPILER_CHECK(sizeof(RegionInfo) >= kCacheLineSize); + COMPILER_CHECK(sizeof(RegionInfo) % kCacheLineSize == 0); RegionInfo *GetRegionInfo(uptr class_id) const { - CHECK_LT(class_id, kNumClasses); - RegionInfo *regions = - reinterpret_cast(SpaceBeg() + kSpaceSize); + DCHECK_LT(class_id, kNumClasses); + RegionInfo *regions = reinterpret_cast(SpaceEnd()); return ®ions[class_id]; } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h index d06d74eeb019..0a92e7768987 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h @@ -39,11 +39,7 @@ struct StackTrace; const uptr kWordSize = SANITIZER_WORDSIZE / 8; const uptr kWordSizeInBits = 8 * kWordSize; -#if defined(__powerpc__) || defined(__powerpc64__) - const uptr kCacheLineSize = 128; -#else - const uptr kCacheLineSize = 64; -#endif +const uptr kCacheLineSize = SANITIZER_CACHE_LINE_SIZE; const uptr kMaxPathLength = 4096; diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform.h index 444b0f0dd205..2fec13f3580d 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_platform.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform.h @@ -309,4 +309,12 @@ # define SANITIZER_MADVISE_DONTNEED MADV_DONTNEED #endif +// Older gcc have issues aligning to a constexpr, and require an integer. +// See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56859 among others. +#if defined(__powerpc__) || defined(__powerpc64__) +# define SANITIZER_CACHE_LINE_SIZE 128 +#else +# define SANITIZER_CACHE_LINE_SIZE 64 +#endif + #endif // SANITIZER_PLATFORM_H