[scudo] releaseToOSMaybe can fail if it can't allocate PageMap am: 2993cdd8d6

Original change: https://googleplex-android-review.googlesource.com/c/platform/external/scudo/+/23436940

Change-Id: Idf2ecffce152f14a3ca2de92eecbc9cfb85932da
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

4 files changed, 28 insertions, 9 deletions

diff --git a/standalone/primary32.h b/standalone/primary32.h
index 726db754f24..b3d6e53dfca 100644
--- a/standalone/primary32.h
+++ b/standalone/primary32.h
@@ -871,6 +871,11 @@ private:
                                        RegionIndex, AllocatedGroupSize,
                                        /*MayContainLastBlockInRegion=*/true);
       }
+
+      // We may not be able to do the page release In a rare case that we may
+      // fail on PageMap allocation.
+      if (UNLIKELY(!Context.hasBlockMarked()))
+        return 0;
     }
 
     if (!Context.hasBlockMarked())
diff --git a/standalone/primary64.h b/standalone/primary64.h
index 39248376eaa..d3a1aea7400 100644
--- a/standalone/primary64.h
+++ b/standalone/primary64.h
@@ -1105,6 +1105,10 @@ private:
                                             ReleaseOffset);
     PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U,
                                ReleaseRangeSize, ReleaseOffset);
+    // We may not be able to do the page release in a rare case that we may
+    // fail on PageMap allocation.
+    if (UNLIKELY(!Context.ensurePageMapAllocated()))
+      return 0;
 
     for (BatchGroup &BG : GroupToRelease) {
       const uptr BatchGroupBase =
diff --git a/standalone/release.h b/standalone/release.h
index dadf6529c0f..9ffc88df4f3 100644
--- a/standalone/release.h
+++ b/standalone/release.h
@@ -235,7 +235,6 @@ public:
         PackingRatioLog;
     BufferSize = SizePerRegion * sizeof(*Buffer) * Regions;
     Buffer = Buffers.getBuffer(BufferSize);
-    DCHECK_NE(Buffer, nullptr);
   }
 
   bool isAllocated() const { return !!Buffer; }
@@ -423,25 +422,27 @@ struct PageReleaseContext {
     return PageMap.isAllocated();
   }
 
-  void ensurePageMapAllocated() {
+  bool ensurePageMapAllocated() {
     if (PageMap.isAllocated())
-      return;
+      return true;
     PageMap.reset(NumberOfRegions, PagesCount, FullPagesBlockCountMax);
-    DCHECK(PageMap.isAllocated());
+    // TODO: Log some message when we fail on PageMap allocation.
+    return PageMap.isAllocated();
   }
 
   // Mark all the blocks in the given range [From, to). Instead of visiting all
   // the blocks, we will just mark the page as all counted. Note the `From` and
   // `To` has to be page aligned but with one exception, if `To` is equal to the
   // RegionSize, it's not necessary to be aligned with page size.
-  void markRangeAsAllCounted(uptr From, uptr To, uptr Base,
+  bool markRangeAsAllCounted(uptr From, uptr To, uptr Base,
                              const uptr RegionIndex, const uptr RegionSize) {
     DCHECK_LT(From, To);
     DCHECK_LE(To, Base + RegionSize);
     DCHECK_EQ(From % PageSize, 0U);
     DCHECK_LE(To - From, RegionSize);
 
-    ensurePageMapAllocated();
+    if (!ensurePageMapAllocated())
+      return false;
 
     uptr FromInRegion = From - Base;
     uptr ToInRegion = To - Base;
@@ -449,7 +450,7 @@ struct PageReleaseContext {
 
     // The straddling block sits across entire range.
     if (FirstBlockInRange >= ToInRegion)
-      return;
+      return true;
 
     // First block may not sit at the first pape in the range, move
     // `FromInRegion` to the first block page.
@@ -516,14 +517,17 @@ struct PageReleaseContext {
       PageMap.setAsAllCountedRange(RegionIndex, getPageIndex(FromInRegion),
                                    getPageIndex(ToInRegion - 1));
     }
+
+    return true;
   }
 
   template <class TransferBatchT, typename DecompactPtrT>
-  void markFreeBlocksInRegion(const IntrusiveList<TransferBatchT> &FreeList,
+  bool markFreeBlocksInRegion(const IntrusiveList<TransferBatchT> &FreeList,
                               DecompactPtrT DecompactPtr, const uptr Base,
                               const uptr RegionIndex, const uptr RegionSize,
                               bool MayContainLastBlockInRegion) {
-    ensurePageMapAllocated();
+    if (!ensurePageMapAllocated())
+      return false;
 
     if (MayContainLastBlockInRegion) {
       const uptr LastBlockInRegion =
@@ -582,6 +586,8 @@ struct PageReleaseContext {
         }
       }
     }
+
+    return true;
   }
 
   uptr getPageIndex(uptr P) { return (P >> PageSizeLog) - ReleasePageOffset; }
diff --git a/standalone/tests/release_test.cpp b/standalone/tests/release_test.cpp
index b6ec9fc6c77..41f0b161a74 100644
--- a/standalone/tests/release_test.cpp
+++ b/standalone/tests/release_test.cpp
@@ -22,13 +22,16 @@ TEST(ScudoReleaseTest, RegionPageMap) {
   for (scudo::uptr I = 0; I < SCUDO_WORDSIZE; I++) {
     // Various valid counter's max values packed into one word.
     scudo::RegionPageMap PageMap2N(1U, 1U, 1UL << I);
+    ASSERT_TRUE(PageMap2N.isAllocated());
     EXPECT_EQ(sizeof(scudo::uptr), PageMap2N.getBufferSize());
     // Check the "all bit set" values too.
     scudo::RegionPageMap PageMap2N1_1(1U, 1U, ~0UL >> I);
+    ASSERT_TRUE(PageMap2N1_1.isAllocated());
     EXPECT_EQ(sizeof(scudo::uptr), PageMap2N1_1.getBufferSize());
     // Verify the packing ratio, the counter is Expected to be packed into the
     // closest power of 2 bits.
     scudo::RegionPageMap PageMap(1U, SCUDO_WORDSIZE, 1UL << I);
+    ASSERT_TRUE(PageMap.isAllocated());
     EXPECT_EQ(sizeof(scudo::uptr) * scudo::roundUpPowerOfTwo(I + 1),
               PageMap.getBufferSize());
   }
@@ -40,6 +43,7 @@ TEST(ScudoReleaseTest, RegionPageMap) {
         (scudo::getPageSizeCached() / 8) * (SCUDO_WORDSIZE >> I);
     scudo::RegionPageMap PageMap(1U, NumCounters,
                                        1UL << ((1UL << I) - 1));
+    ASSERT_TRUE(PageMap.isAllocated());
     PageMap.inc(0U, 0U);
     for (scudo::uptr C = 1; C < NumCounters - 1; C++) {
       EXPECT_EQ(0UL, PageMap.get(0U, C));