[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.
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 8 19:23:05 PDT 2016
On Mon, Aug 8, 2016 at 8:54 AM, Daniel Sanders <Daniel.Sanders at imgtec.com>
wrote:
> 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.
>
Wow. So, by making a innocent-looking change in the allocator we crash the
kernel?
The change in sanitizer_allocator_size_class_map.h may have triggered some
old dormant bug in the run-time,
that somehow caused kernel to misbehave, or it just triggered a kernel bug
by changing the mmap pattern in the test.
I expect to make more changes in the same place to make it use less RAM, so
maybe the problem will disappear again.
can you test your kernel with kasan?
> > -----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::SizeClassRequiresSeparateTrans
> ferBatch(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::SizeClassRequiresSeparateTrans
> ferBatch(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::SizeClassRequiresSeparateTrans
> ferBatch(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::SizeClassRequiresSeparateTrans
> ferBatch(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::SizeClassRequiresSeparateTrans
> ferBatch(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::SizeClassRequiresSeparateTrans
> ferBatch(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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160808/5d7bc636/attachment.html>
More information about the llvm-commits
mailing list