[compiler-rt] f835250 - [scudo][standalone] Allow fallback to secondary if primary is full

[Kostya Kortchinsky via llvm-commits]

Author: Kostya Kortchinsky
Date: 2020-03-20T07:55:20-07:00
New Revision: f8352502a355fb080300e2cc9de5629614a6c00f

URL: https://github.com/llvm/llvm-project/commit/f8352502a355fb080300e2cc9de5629614a6c00f
DIFF: https://github.com/llvm/llvm-project/commit/f8352502a355fb080300e2cc9de5629614a6c00f.diff

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

Summary:
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.

Reviewers: hctim, cferris, eugenis, morehouse, pcc

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D76430

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 8456dc82d20e..1aa93510d22a 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -260,8 +260,8 @@ class Allocator {
     }
     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 @@ class Allocator {
       // 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)

diff  --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index ce1b2824788d..a6f29a2610ed 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -417,7 +417,7 @@ TEST(ScudoCombinedTest, ReleaseToOS) {
   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 @@ TEST(ScudoCombinedTest, FullRegion) {
                                                            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);
 }