Skip to content

Commit 11ea40c

Browse files
committed
[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
1 parent 8cdbf8d commit 11ea40c

File tree

4 files changed

+28
-9
lines changed

4 files changed

+28
-9
lines changed

compiler-rt/lib/scudo/standalone/primary32.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,11 @@ template <typename Config> class SizeClassAllocator32 {
871871
RegionIndex, AllocatedGroupSize,
872872
/*MayContainLastBlockInRegion=*/true);
873873
}
874+
875+
// We may not be able to do the page release In a rare case that we may
876+
// fail on PageMap allocation.
877+
if (UNLIKELY(!Context.hasBlockMarked()))
878+
return 0;
874879
}
875880

876881
if (!Context.hasBlockMarked())

compiler-rt/lib/scudo/standalone/primary64.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,10 @@ template <typename Config> class SizeClassAllocator64 {
11051105
ReleaseOffset);
11061106
PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U,
11071107
ReleaseRangeSize, ReleaseOffset);
1108+
// We may not be able to do the page release in a rare case that we may
1109+
// fail on PageMap allocation.
1110+
if (UNLIKELY(!Context.ensurePageMapAllocated()))
1111+
return 0;
11081112

11091113
for (BatchGroup &BG : GroupToRelease) {
11101114
const uptr BatchGroupBase =

compiler-rt/lib/scudo/standalone/release.h

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,6 @@ class RegionPageMap {
235235
PackingRatioLog;
236236
BufferSize = SizePerRegion * sizeof(*Buffer) * Regions;
237237
Buffer = Buffers.getBuffer(BufferSize);
238-
DCHECK_NE(Buffer, nullptr);
239238
}
240239

241240
bool isAllocated() const { return !!Buffer; }
@@ -423,33 +422,35 @@ struct PageReleaseContext {
423422
return PageMap.isAllocated();
424423
}
425424

426-
void ensurePageMapAllocated() {
425+
bool ensurePageMapAllocated() {
427426
if (PageMap.isAllocated())
428-
return;
427+
return true;
429428
PageMap.reset(NumberOfRegions, PagesCount, FullPagesBlockCountMax);
430-
DCHECK(PageMap.isAllocated());
429+
// TODO: Log some message when we fail on PageMap allocation.
430+
return PageMap.isAllocated();
431431
}
432432

433433
// Mark all the blocks in the given range [From, to). Instead of visiting all
434434
// the blocks, we will just mark the page as all counted. Note the `From` and
435435
// `To` has to be page aligned but with one exception, if `To` is equal to the
436436
// RegionSize, it's not necessary to be aligned with page size.
437-
void markRangeAsAllCounted(uptr From, uptr To, uptr Base,
437+
bool markRangeAsAllCounted(uptr From, uptr To, uptr Base,
438438
const uptr RegionIndex, const uptr RegionSize) {
439439
DCHECK_LT(From, To);
440440
DCHECK_LE(To, Base + RegionSize);
441441
DCHECK_EQ(From % PageSize, 0U);
442442
DCHECK_LE(To - From, RegionSize);
443443

444-
ensurePageMapAllocated();
444+
if (!ensurePageMapAllocated())
445+
return false;
445446

446447
uptr FromInRegion = From - Base;
447448
uptr ToInRegion = To - Base;
448449
uptr FirstBlockInRange = roundUpSlow(FromInRegion, BlockSize);
449450

450451
// The straddling block sits across entire range.
451452
if (FirstBlockInRange >= ToInRegion)
452-
return;
453+
return true;
453454

454455
// First block may not sit at the first pape in the range, move
455456
// `FromInRegion` to the first block page.
@@ -516,14 +517,17 @@ struct PageReleaseContext {
516517
PageMap.setAsAllCountedRange(RegionIndex, getPageIndex(FromInRegion),
517518
getPageIndex(ToInRegion - 1));
518519
}
520+
521+
return true;
519522
}
520523

521524
template <class TransferBatchT, typename DecompactPtrT>
522-
void markFreeBlocksInRegion(const IntrusiveList<TransferBatchT> &FreeList,
525+
bool markFreeBlocksInRegion(const IntrusiveList<TransferBatchT> &FreeList,
523526
DecompactPtrT DecompactPtr, const uptr Base,
524527
const uptr RegionIndex, const uptr RegionSize,
525528
bool MayContainLastBlockInRegion) {
526-
ensurePageMapAllocated();
529+
if (!ensurePageMapAllocated())
530+
return false;
527531

528532
if (MayContainLastBlockInRegion) {
529533
const uptr LastBlockInRegion =
@@ -582,6 +586,8 @@ struct PageReleaseContext {
582586
}
583587
}
584588
}
589+
590+
return true;
585591
}
586592

587593
uptr getPageIndex(uptr P) { return (P >> PageSizeLog) - ReleasePageOffset; }

compiler-rt/lib/scudo/standalone/tests/release_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@ TEST(ScudoReleaseTest, RegionPageMap) {
2222
for (scudo::uptr I = 0; I < SCUDO_WORDSIZE; I++) {
2323
// Various valid counter's max values packed into one word.
2424
scudo::RegionPageMap PageMap2N(1U, 1U, 1UL << I);
25+
ASSERT_TRUE(PageMap2N.isAllocated());
2526
EXPECT_EQ(sizeof(scudo::uptr), PageMap2N.getBufferSize());
2627
// Check the "all bit set" values too.
2728
scudo::RegionPageMap PageMap2N1_1(1U, 1U, ~0UL >> I);
29+
ASSERT_TRUE(PageMap2N1_1.isAllocated());
2830
EXPECT_EQ(sizeof(scudo::uptr), PageMap2N1_1.getBufferSize());
2931
// Verify the packing ratio, the counter is Expected to be packed into the
3032
// closest power of 2 bits.
3133
scudo::RegionPageMap PageMap(1U, SCUDO_WORDSIZE, 1UL << I);
34+
ASSERT_TRUE(PageMap.isAllocated());
3235
EXPECT_EQ(sizeof(scudo::uptr) * scudo::roundUpPowerOfTwo(I + 1),
3336
PageMap.getBufferSize());
3437
}
@@ -40,6 +43,7 @@ TEST(ScudoReleaseTest, RegionPageMap) {
4043
(scudo::getPageSizeCached() / 8) * (SCUDO_WORDSIZE >> I);
4144
scudo::RegionPageMap PageMap(1U, NumCounters,
4245
1UL << ((1UL << I) - 1));
46+
ASSERT_TRUE(PageMap.isAllocated());
4347
PageMap.inc(0U, 0U);
4448
for (scudo::uptr C = 1; C < NumCounters - 1; C++) {
4549
EXPECT_EQ(0UL, PageMap.get(0U, C));

0 commit comments

Comments
 (0)