[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
Tue Aug 9 06:39:31 PDT 2016
> Wow. So, by making a innocent-looking change in the allocator we crash the kernel?
Yep, it seems that way. I've tried an updated kernel which seems to fix the problem (or at least makes it go away for 'ninja check-asan'). This kernel might cause some sanitizer failures but a few failures is better than not testing and I think Sagar has already posted fixes for some of them.
> can you test your kernel with kasan?
Unfortunately, the Mips arch lacks some of the prerequisites for CONFIG_KASAN at the moment. The main one we're missing is HAVE_ARCH_KASAN but there's a couple others.
From: Kostya Serebryany [mailto:kcc at google.com]
Sent: 09 August 2016 03:23
To: Daniel Sanders
Cc: 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.
On Mon, Aug 8, 2016 at 8:54 AM, Daniel Sanders <Daniel.Sanders at imgtec.com<mailto: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<mailto: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<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<mailto: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<mailto: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/20160809/d9287ed2/attachment.html>
More information about the llvm-commits
mailing list