[PATCH] D76430: [scudo][standalone] Allow fallback to secondary if primary is full

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
cryptoad added reviewers: hctim, cferris, eugenis, morehouse, pcc.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.
cryptoad updated this revision to Diff 251385.
cryptoad added a comment.

Initialize `Block` to `nullptr`.


We introduced a way to fallback to the immediately larger size class for
the Primary in the event a region was full, but in the event of the largest
size class, we would just fail.

This change allows to fallback to the Secondary when the last region of
the Primary is full. We also expand the trick to all platforms as opposed
to being Android only, and update the test to cover the new case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76430

Files:
  compiler-rt/lib/scudo/standalone/combined.h
  compiler-rt/lib/scudo/standalone/tests/combined_test.cpp


Index: compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
===================================================================
--- compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -417,7 +417,7 @@
   Allocator->releaseToOS();
 }
 
-// Verify that when a region gets full, Android will still manage to
+// Verify that when a region gets full, the allocator will still manage to
 // fulfill the allocation through a larger size class.
 TEST(ScudoCombinedTest, FullRegion) {
   using AllocatorT = scudo::Allocator<DeathConfig>;
@@ -429,26 +429,25 @@
                                                            Deleter);
   Allocator->reset();
 
-  const scudo::uptr Size = 1000U;
-  const scudo::uptr MaxNumberOfChunks =
-      (1U << DeathRegionSizeLog) /
-      DeathConfig::DeathSizeClassMap::getSizeByClassId(1U);
-  void *P;
   std::vector<void *> V;
   scudo::uptr FailedAllocationsCount = 0;
-  for (scudo::uptr I = 0; I <= MaxNumberOfChunks; I++) {
-    P = Allocator->allocate(Size, Origin);
-    if (!P)
-      FailedAllocationsCount++;
-    else
-      V.push_back(P);
+  for (scudo::uptr ClassId = 1U;
+       ClassId <= DeathConfig::DeathSizeClassMap::LargestClassId; ClassId++) {
+    const scudo::uptr Size =
+        DeathConfig::DeathSizeClassMap::getSizeByClassId(ClassId);
+    const scudo::uptr MaxNumberOfChunks = (1U << DeathRegionSizeLog) / Size;
+    void *P;
+    for (scudo::uptr I = 0; I <= MaxNumberOfChunks; I++) {
+      P = Allocator->allocate(Size - 64U, Origin);
+      if (!P)
+        FailedAllocationsCount++;
+      else
+        V.push_back(P);
+    }
   }
   while (!V.empty()) {
     Allocator->deallocate(V.back(), Origin);
     V.pop_back();
   }
-  if (SCUDO_ANDROID)
-    EXPECT_EQ(FailedAllocationsCount, 0U);
-  else
-    EXPECT_GT(FailedAllocationsCount, 0U);
+  EXPECT_EQ(FailedAllocationsCount, 0U);
 }
Index: compiler-rt/lib/scudo/standalone/combined.h
===================================================================
--- compiler-rt/lib/scudo/standalone/combined.h
+++ compiler-rt/lib/scudo/standalone/combined.h
@@ -260,8 +260,8 @@
     }
     DCHECK_LE(Size, NeededSize);
 
-    void *Block;
-    uptr ClassId;
+    void *Block = nullptr;
+    uptr ClassId = 0;
     uptr SecondaryBlockEnd;
     if (LIKELY(PrimaryT::canAllocate(NeededSize))) {
       ClassId = SizeClassMap::getClassIdBySize(NeededSize);
@@ -273,20 +273,19 @@
       // is the region being full. In that event, retry once using the
       // immediately larger class (except if the failing class was already the
       // largest). This will waste some memory but will allow the application to
-      // not fail.
-      if (SCUDO_ANDROID) {
-        if (UNLIKELY(!Block)) {
-          if (ClassId < SizeClassMap::LargestClassId)
-            Block = TSD->Cache.allocate(++ClassId);
-        }
+      // not fail. If dealing with the largest class, fallback to the Secondary.
+      if (UNLIKELY(!Block)) {
+        if (ClassId < SizeClassMap::LargestClassId)
+          Block = TSD->Cache.allocate(++ClassId);
+        else
+          ClassId = 0;
       }
       if (UnlockRequired)
         TSD->unlock();
-    } else {
-      ClassId = 0;
+    }
+    if (UNLIKELY(ClassId == 0))
       Block = Secondary.allocate(NeededSize, Alignment, &SecondaryBlockEnd,
                                  ZeroContents);
-    }
 
     if (UNLIKELY(!Block)) {
       if (Options.MayReturnNull)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76430.251385.patch
Type: text/x-patch
Size: 3479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200319/97f0720c/attachment.bin>