[compiler-rt] r276318 - [sanitizer] allocator: remove kPopulateSize and only use SizeClassMap::MaxCached; ensure that TransferBatch size is a power of two, refactor TransferBatch creation/destruction into separate functions.
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 8 05:08:21 PDT 2016
Hi Kostya,
This commit appears to be the root cause for the machine-killing bug I mentioned to you a couple weeks ago. I haven't figured out why it happens yet though.
Having compiler-rt at r276317 and everything else at r276320 works fine (moving llvm and clang to r276317 causes link failures), but if I then move compiler-rt to r276318 then some test cases fail in 'ninja check-asan' and the system becomes unable to start python until the machine is rebooted. After the reboot, everything is fine until 'ninja check-asan' is run again.
Sorry it's taken so long to narrow it down to one commit. I'll let you know when I've narrowed it down to a single test
> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Kostya Serebryany via llvm-commits
> Sent: 21 July 2016 19:48
> To: llvm-commits at lists.llvm.org
> Subject: [compiler-rt] r276318 - [sanitizer] allocator: remove kPopulateSize
> and only use SizeClassMap::MaxCached; ensure that TransferBatch size is a
> power of two, refactor TransferBatch creation/destruction into separate
> functions.
>
> Author: kcc
> Date: Thu Jul 21 13:47:53 2016
> New Revision: 276318
>
> URL: http://llvm.org/viewvc/llvm-project?rev=276318&view=rev
> Log:
> [sanitizer] allocator: remove kPopulateSize and only use
> SizeClassMap::MaxCached; ensure that TransferBatch size is a power of two,
> refactor TransferBatch creation/destruction into separate functions.
>
> Modified:
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_local_cache.h
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary32.h
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h
> compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_size_class_map.h
> compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_allocator_test.cc
>
> Modified: compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_local_cache.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_local_cache.h?rev=27631
> 8&r1=276317&r2=276318&view=diff
> ==========================================================
> ====================
> --- compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_local_cache.h (original)
> +++ compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_local_cache.h Thu Jul 21
> 13:47:53 2016
> @@ -88,6 +88,23 @@ struct SizeClassAllocatorLocalCache {
> }
> }
>
> + // Returns a Batch suitable for class_id.
> + // For small size classes allocates the batch from the allocator.
> + // For large size classes simply returns b.
> + Batch *CreateBatch(uptr class_id, SizeClassAllocator *allocator, Batch *b) {
> + if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> + return (Batch*)Allocate(allocator, SizeClassMap::ClassID(sizeof(Batch)));
> + return b;
> + }
> +
> + // Destroys Batch b.
> + // For small size classes deallocates b to the allocator.
> + // Does notthing for large size classes.
> + void DestroyBatch(uptr class_id, SizeClassAllocator *allocator, Batch *b) {
> + if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> + Deallocate(allocator, SizeClassMap::ClassID(sizeof(Batch)), b);
> + }
> +
> NOINLINE void Refill(SizeClassAllocator *allocator, uptr class_id) {
> InitCache();
> PerClass *c = &per_class_[class_id];
> @@ -96,18 +113,13 @@ struct SizeClassAllocatorLocalCache {
> for (uptr i = 0; i < b->count; i++)
> c->batch[i] = b->batch[i];
> c->count = b->count;
> - if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> - Deallocate(allocator, SizeClassMap::ClassID(sizeof(Batch)), b);
> + DestroyBatch(class_id, allocator, b);
> }
>
> NOINLINE void Drain(SizeClassAllocator *allocator, uptr class_id) {
> InitCache();
> PerClass *c = &per_class_[class_id];
> - Batch *b;
> - if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> - b = (Batch*)Allocate(allocator, SizeClassMap::ClassID(sizeof(Batch)));
> - else
> - b = (Batch*)c->batch[0];
> + Batch *b = CreateBatch(class_id, allocator, (Batch*)c->batch[0]);
> uptr cnt = Min(c->max_count / 2, c->count);
> for (uptr i = 0; i < cnt; i++) {
> b->batch[i] = c->batch[i];
> @@ -119,5 +131,3 @@ struct SizeClassAllocatorLocalCache {
> allocator->DeallocateBatch(&stats_, class_id, b);
> }
> };
> -
> -
>
> Modified: compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary32.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary32.h?rev=276318
> &r1=276317&r2=276318&view=diff
> ==========================================================
> ====================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary32.h
> (original)
> +++ compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary32.h Thu Jul 21
> 13:47:53 2016
> @@ -231,10 +231,7 @@ class SizeClassAllocator32 {
> Batch *b = nullptr;
> for (uptr i = reg; i < reg + n_chunks * size; i += size) {
> if (!b) {
> - if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> - b = (Batch*)c->Allocate(this, SizeClassMap::ClassID(sizeof(Batch)));
> - else
> - b = (Batch*)i;
> + b = c->CreateBatch(class_id, this, (Batch*)i);
> b->count = 0;
> }
> b->batch[b->count++] = (void*)i;
>
> Modified: compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h?rev=276318
> &r1=276317&r2=276318&view=diff
> ==========================================================
> ====================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h
> (original)
> +++ compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h Thu Jul 21
> 13:47:53 2016
> @@ -219,9 +219,6 @@ class SizeClassAllocator64 {
> uptr SpaceEnd() const { return SpaceBeg() + kSpaceSize; }
> // kRegionSize must be >= 2^32.
> COMPILER_CHECK((kRegionSize) >= (1ULL << (SANITIZER_WORDSIZE / 2)));
> - // Populate the free list with at most this number of bytes at once
> - // or with one element if its size is greater.
> - static const uptr kPopulateSize = 1 << 14;
> // Call mmap for user memory with at least this size.
> static const uptr kUserMapSize = 1 << 16;
> // Call mmap for metadata memory with at least this size.
> @@ -261,7 +258,7 @@ class SizeClassAllocator64 {
> if (b)
> return b;
> uptr size = SizeClassMap::Size(class_id);
> - uptr count = size < kPopulateSize ? SizeClassMap::MaxCached(class_id) :
> 1;
> + uptr count = SizeClassMap::MaxCached(class_id);
> uptr beg_idx = region->allocated_user;
> uptr end_idx = beg_idx + count * size;
> uptr region_beg = SpaceBeg() + kRegionSize * class_id;
> @@ -296,10 +293,7 @@ class SizeClassAllocator64 {
> Die();
> }
> for (;;) {
> - if (SizeClassMap::SizeClassRequiresSeparateTransferBatch(class_id))
> - b = (Batch*)c->Allocate(this, SizeClassMap::ClassID(sizeof(Batch)));
> - else
> - b = (Batch*)(region_beg + beg_idx);
> + b = c->CreateBatch(class_id, this, (Batch*)(region_beg + beg_idx));
> b->count = count;
> for (uptr i = 0; i < count; i++)
> b->batch[i] = (void*)(region_beg + beg_idx + i * size);
>
> Modified: compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_size_class_map.h
> URL: http://llvm.org/viewvc/llvm-project/compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_size_class_map.h?rev=27
> 6318&r1=276317&r2=276318&view=diff
> ==========================================================
> ====================
> --- compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_size_class_map.h
> (original)
> +++ compiler-
> rt/trunk/lib/sanitizer_common/sanitizer_allocator_size_class_map.h Thu Jul
> 21 13:47:53 2016
> @@ -87,14 +87,17 @@ class SizeClassMap {
>
> public:
> static const uptr kMaxNumCached = kMaxNumCachedT;
> + COMPILER_CHECK(((kMaxNumCached + 2) & (kMaxNumCached + 1)) ==
> 0);
> // We transfer chunks between central and thread-local free lists in
> batches.
> // For small size classes we allocate batches separately.
> // For large size classes we use one of the chunks to store the batch.
> + // sizeof(TransferBatch) must be a power of 2 for more efficient allocation.
> struct TransferBatch {
> TransferBatch *next;
> uptr count;
> void *batch[kMaxNumCached];
> };
> + COMPILER_CHECK((sizeof(TransferBatch) & (sizeof(TransferBatch) - 1)) ==
> 0);
>
> static const uptr kMaxSize = 1UL << kMaxSizeLog;
> static const uptr kNumClasses =
> @@ -180,7 +183,7 @@ class SizeClassMap {
> }
> };
>
> -typedef SizeClassMap<17, 128, 16> DefaultSizeClassMap;
> -typedef SizeClassMap<17, 64, 14> CompactSizeClassMap;
> +typedef SizeClassMap<17, 126, 16> DefaultSizeClassMap;
> +typedef SizeClassMap<17, 62, 14> CompactSizeClassMap;
> template<class SizeClassAllocator> struct SizeClassAllocatorLocalCache;
>
>
> Modified: compiler-
> rt/trunk/lib/sanitizer_common/tests/sanitizer_allocator_test.cc
> URL: http://llvm.org/viewvc/llvm-project/compiler-
> rt/trunk/lib/sanitizer_common/tests/sanitizer_allocator_test.cc?rev=276318
> &r1=276317&r2=276318&view=diff
> ==========================================================
> ====================
> --- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_allocator_test.cc
> (original)
> +++ compiler-
> rt/trunk/lib/sanitizer_common/tests/sanitizer_allocator_test.cc Thu Jul 21
> 13:47:53 2016
> @@ -781,7 +781,7 @@ TEST(SanitizerCommon, LargeMmapAllocator
> // Regression test for out-of-memory condition in PopulateFreeList().
> TEST(SanitizerCommon, SizeClassAllocator64PopulateFreeListOOM) {
> // In a world where regions are small and chunks are huge...
> - typedef SizeClassMap<63, 128, 16> SpecialSizeClassMap;
> + typedef SizeClassMap<63, 126, 16> SpecialSizeClassMap;
> typedef SizeClassAllocator64<kAllocatorSpace, kAllocatorSize, 0,
> SpecialSizeClassMap> SpecialAllocator64;
> const uptr kRegionSize =
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list