[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 08:54:26 PDT 2016
Python becomes unusable after 'Asan-mipsel-inline-Test/AddressSanitizer.AllocDeallocMismatch' fails. I've also found that the regressions go away if I revert the changes to sanitizer_allocator_size_class_map.h and sanitizer_allocator_test.cc but keep everything else.
> -----Original Message-----
> From: Daniel Sanders
> Sent: 08 August 2016 13:08
> To: 'Kostya Serebryany'; llvm-commits at lists.llvm.org
> Subject: RE: [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.
>
> 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