[compiler-rt] 11ea40c - [scudo] releaseToOSMaybe can fail if it can't allocate PageMap
Chia-hung Duan via llvm-commits
llvm-commits at lists.llvm.org
Thu May 25 11:09:12 PDT 2023
Author: Chia-hung Duan
Date: 2023-05-25T18:07:55Z
New Revision: 11ea40cff5413057d823a4b3ac5ac419b674dc56
URL: https://github.com/llvm/llvm-project/commit/11ea40cff5413057d823a4b3ac5ac419b674dc56
DIFF: https://github.com/llvm/llvm-project/commit/11ea40cff5413057d823a4b3ac5ac419b674dc56.diff
LOG: [scudo] releaseToOSMaybe can fail if it can't allocate PageMap
PageMap is allocated with MAP_ALLOWNOMEM if there's no static buffer
left. So it can be failed and return nullptr without any assertion
triggered. Instead of crashing in the releaseToOSMaybe in the middle,
just return and let the program handles the page failure.
Reviewed By: cferris
Differential Revision: https://reviews.llvm.org/D151379
Added:
Modified:
compiler-rt/lib/scudo/standalone/primary32.h
compiler-rt/lib/scudo/standalone/primary64.h
compiler-rt/lib/scudo/standalone/release.h
compiler-rt/lib/scudo/standalone/tests/release_test.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h
index 726db754f245e..b3d6e53dfca26 100644
--- a/compiler-rt/lib/scudo/standalone/primary32.h
+++ b/compiler-rt/lib/scudo/standalone/primary32.h
@@ -871,6 +871,11 @@ template <typename Config> class SizeClassAllocator32 {
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/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 39248376eaac7..d3a1aea740033 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -1105,6 +1105,10 @@ template <typename Config> class SizeClassAllocator64 {
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/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
index dadf6529c0f2c..9ffc88df4f3f9 100644
--- a/compiler-rt/lib/scudo/standalone/release.h
+++ b/compiler-rt/lib/scudo/standalone/release.h
@@ -235,7 +235,6 @@ class RegionPageMap {
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/compiler-rt/lib/scudo/standalone/tests/release_test.cpp b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
index b6ec9fc6c777e..41f0b161a74ba 100644
--- a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
+++ b/compiler-rt/lib/scudo/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));
More information about the llvm-commits
mailing list