diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h --- a/compiler-rt/lib/scudo/standalone/primary32.h +++ b/compiler-rt/lib/scudo/standalone/primary32.h @@ -775,7 +775,7 @@ compactPtrGroupBase(compactPtr(ClassId, Sci->CurrentRegion)); ReleaseRecorder Recorder(Base); - PageReleaseContext Context(BlockSize, RegionSize, NumberOfRegions, + PageReleaseContext Context(BlockSize, NumberOfRegions, /*ReleaseSize=*/RegionSize); auto DecompactPtr = [](CompactPtrT CompactPtr) { @@ -787,10 +787,9 @@ if (PushedBytesDelta * BlockSize < PageSize) continue; + const uptr GroupBase = decompactGroupBase(BG.CompactPtrGroupBase); uptr AllocatedGroupSize = - decompactGroupBase(BG.CompactPtrGroupBase) == CurGroupBase - ? Sci->CurrentRegionAllocated - : GroupSize; + GroupBase == CurGroupBase ? Sci->CurrentRegionAllocated : GroupSize; if (AllocatedGroupSize == 0) continue; @@ -810,34 +809,25 @@ BG.PushedBlocksAtLastCheckpoint = BG.PushedBlocks; const uptr MaxContainedBlocks = AllocatedGroupSize / BlockSize; - // The first condition to do range marking is that all the blocks in the - // range need to be from the same region. In SizeClassAllocator32, this is - // true when GroupSize and RegionSize are the same. Another tricky case, - // while range marking, the last block in a region needs the logic to mark - // the last page. However, in SizeClassAllocator32, the RegionSize - // recorded in PageReleaseContext may be different from - // `CurrentRegionAllocated` of the current region. This exception excludes - // the chance of doing range marking for the current region. - const bool CanDoRangeMark = - GroupSize == RegionSize && - decompactGroupBase(BG.CompactPtrGroupBase) != CurGroupBase; - - if (CanDoRangeMark && NumBlocks == MaxContainedBlocks) { + const uptr RegionIndex = (GroupBase - Base) / RegionSize; + + if (NumBlocks == MaxContainedBlocks) { for (const auto &It : BG.Batches) for (u16 I = 0; I < It.getCount(); ++I) DCHECK_EQ(compactPtrGroupBase(It.get(I)), BG.CompactPtrGroupBase); - const uptr From = decompactGroupBase(BG.CompactPtrGroupBase); - const uptr To = From + AllocatedGroupSize; - Context.markRangeAsAllCounted(From, To, Base); + const uptr To = GroupBase + AllocatedGroupSize; + Context.markRangeAsAllCounted(GroupBase, To, GroupBase, RegionIndex, + AllocatedGroupSize); } else { - if (CanDoRangeMark) - DCHECK_LT(NumBlocks, MaxContainedBlocks); + DCHECK_LT(NumBlocks, MaxContainedBlocks); // Note that we don't always visit blocks in each BatchGroup so that we // may miss the chance of releasing certain pages that cross // BatchGroups. - Context.markFreeBlocks(BG.Batches, DecompactPtr, Base); + Context.markFreeBlocksInRegion(BG.Batches, DecompactPtr, GroupBase, + RegionIndex, AllocatedGroupSize, + /*MayContainLastBlockInRegion=*/true); } } diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h --- a/compiler-rt/lib/scudo/standalone/primary64.h +++ b/compiler-rt/lib/scudo/standalone/primary64.h @@ -935,9 +935,8 @@ const uptr ReleaseOffset = ReleaseBase - Region->RegionBeg; ReleaseRecorder Recorder(Region->RegionBeg, ReleaseOffset, &Region->Data); - PageReleaseContext Context( - BlockSize, Region->AllocatedUser, /*NumberOfRegions=*/1U, - ReleaseRangeSize, ReleaseOffset); + PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, + ReleaseRangeSize, ReleaseOffset); for (BatchGroup &BG : GroupToRelease) { BG.PushedBlocksAtLastCheckpoint = BG.PushedBlocks; @@ -949,6 +948,8 @@ ? GroupSize : AllocatedUserEnd - BatchGroupBase; const uptr BatchGroupUsedEnd = BatchGroupBase + AllocatedGroupSize; + const bool MayContainLastBlockInRegion = + BatchGroupUsedEnd == AllocatedUserEnd; const bool BlockAlignedWithUsedEnd = (BatchGroupUsedEnd - Region->RegionBeg) % BlockSize == 0; @@ -965,13 +966,16 @@ DCHECK_EQ(compactPtrGroup(It.get(I)), BG.CompactPtrGroupBase); Context.markRangeAsAllCounted(BatchGroupBase, BatchGroupUsedEnd, - Region->RegionBeg); + Region->RegionBeg, /*RegionIndex=*/0, + Region->AllocatedUser); } else { DCHECK_LT(NumBlocks, MaxContainedBlocks); // Note that we don't always visit blocks in each BatchGroup so that we // may miss the chance of releasing certain pages that cross // BatchGroups. - Context.markFreeBlocks(BG.Batches, DecompactPtr, Region->RegionBeg); + Context.markFreeBlocksInRegion( + BG.Batches, DecompactPtr, Region->RegionBeg, /*RegionIndex=*/0, + Region->AllocatedUser, MayContainLastBlockInRegion); } } diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h --- a/compiler-rt/lib/scudo/standalone/release.h +++ b/compiler-rt/lib/scudo/standalone/release.h @@ -266,10 +266,9 @@ }; struct PageReleaseContext { - PageReleaseContext(uptr BlockSize, uptr RegionSize, uptr NumberOfRegions, - uptr ReleaseSize, uptr ReleaseOffset = 0) - : BlockSize(BlockSize), RegionSize(RegionSize), - NumberOfRegions(NumberOfRegions) { + PageReleaseContext(uptr BlockSize, uptr NumberOfRegions, uptr ReleaseSize, + uptr ReleaseOffset = 0) + : BlockSize(BlockSize), NumberOfRegions(NumberOfRegions) { PageSize = getPageSizeCached(); if (BlockSize <= PageSize) { if (PageSize % BlockSize == 0) { @@ -305,15 +304,11 @@ // region marking (which includes the complexity of how to handle the last // block in a region). We may consider this after markFreeBlocks() accepts // only free blocks from the same region. - if (NumberOfRegions != 1) { - DCHECK_EQ(ReleaseSize, RegionSize); + if (NumberOfRegions != 1) DCHECK_EQ(ReleaseOffset, 0U); - } PagesCount = roundUp(ReleaseSize, PageSize) / PageSize; PageSizeLog = getLog2(PageSize); - RoundedRegionSize = roundUp(RegionSize, PageSize); - RoundedSize = NumberOfRegions * RoundedRegionSize; ReleasePageOffset = ReleaseOffset >> PageSizeLog; } @@ -333,25 +328,17 @@ // 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) { + void 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(); - const uptr FromOffset = From - Base; - const uptr ToOffset = To - Base; - - const uptr RegionIndex = - NumberOfRegions == 1U ? 0 : FromOffset / RegionSize; - if (SCUDO_DEBUG) { - const uptr ToRegionIndex = - NumberOfRegions == 1U ? 0 : (ToOffset - 1) / RegionSize; - CHECK_EQ(RegionIndex, ToRegionIndex); - } - - uptr FromInRegion = FromOffset - RegionIndex * RegionSize; - uptr ToInRegion = ToOffset - RegionIndex * RegionSize; + uptr FromInRegion = From - Base; + uptr ToInRegion = To - Base; uptr FirstBlockInRange = roundUpSlow(FromInRegion, BlockSize); // The straddling block sits across entire range. @@ -424,23 +411,26 @@ } } - template - void markFreeBlocks(const IntrusiveList &FreeList, - DecompactPtrT DecompactPtr, uptr Base) { + template + void markFreeBlocksInRegion(const IntrusiveList &FreeList, + DecompactPtrT DecompactPtr, const uptr Base, + const uptr RegionIndex, const uptr RegionSize, + bool MayContainLastBlockInRegion) { ensurePageMapAllocated(); - const uptr LastBlockInRegion = ((RegionSize / BlockSize) - 1U) * BlockSize; - - // The last block in a region may not use the entire page, so if it's free, - // we mark the following "pretend" memory block(s) as free. - auto markLastBlock = [this, LastBlockInRegion](const uptr RegionIndex) { + if (MayContainLastBlockInRegion) { + const uptr LastBlockInRegion = + ((RegionSize / BlockSize) - 1U) * BlockSize; + // The last block in a region may not use the entire page, we mark the + // following "pretend" memory block(s) as free in advance. + const uptr RoundedRegionSize = roundUp(RegionSize, PageSize); uptr PInRegion = LastBlockInRegion + BlockSize; while (PInRegion < RoundedRegionSize) { PageMap.incRange(RegionIndex, getPageIndex(PInRegion), getPageIndex(PInRegion + BlockSize - 1)); PInRegion += BlockSize; } - }; + } // Iterate over free chunks and count how many free chunks affect each // allocated page. @@ -448,14 +438,9 @@ // Each chunk affects one page only. for (const auto &It : FreeList) { for (u16 I = 0; I < It.getCount(); I++) { - const uptr P = DecompactPtr(It.get(I)) - Base; - if (P >= RoundedSize) - continue; - const uptr RegionIndex = NumberOfRegions == 1U ? 0 : P / RegionSize; - const uptr PInRegion = P - RegionIndex * RegionSize; + const uptr PInRegion = DecompactPtr(It.get(I)) - Base; + DCHECK_LT(PInRegion, RegionSize); PageMap.inc(RegionIndex, getPageIndex(PInRegion)); - if (PInRegion == LastBlockInRegion) - markLastBlock(RegionIndex); } } } else { @@ -463,15 +448,9 @@ DCHECK_GE(RegionSize, BlockSize); for (const auto &It : FreeList) { for (u16 I = 0; I < It.getCount(); I++) { - const uptr P = DecompactPtr(It.get(I)) - Base; - if (P >= RoundedSize) - continue; - const uptr RegionIndex = NumberOfRegions == 1U ? 0 : P / RegionSize; - uptr PInRegion = P - RegionIndex * RegionSize; + const uptr PInRegion = DecompactPtr(It.get(I)) - Base; PageMap.incRange(RegionIndex, getPageIndex(PInRegion), getPageIndex(PInRegion + BlockSize - 1)); - if (PInRegion == LastBlockInRegion) - markLastBlock(RegionIndex); } } } @@ -480,7 +459,6 @@ uptr getPageIndex(uptr P) { return (P >> PageSizeLog) - ReleasePageOffset; } uptr BlockSize; - uptr RegionSize; uptr NumberOfRegions; // For partial region marking, some pages in front are not needed to be // counted. @@ -488,8 +466,6 @@ uptr PageSize; uptr PagesCount; uptr PageSizeLog; - uptr RoundedRegionSize; - uptr RoundedSize; uptr FullPagesBlockCountMax; bool SameBlockCountPerPage; RegionPageMap PageMap; @@ -570,21 +546,6 @@ RangeTracker.finish(); } -// An overload releaseFreeMemoryToOS which doesn't require the page usage -// information after releasing. -template -NOINLINE void -releaseFreeMemoryToOS(const IntrusiveList &FreeList, - uptr RegionSize, uptr NumberOfRegions, uptr BlockSize, - ReleaseRecorderT &Recorder, DecompactPtrT DecompactPtr, - SkipRegionT SkipRegion) { - PageReleaseContext Context(BlockSize, /*ReleaseSize=*/RegionSize, RegionSize, - NumberOfRegions); - Context.markFreeBlocks(FreeList, DecompactPtr, Recorder.getBase()); - releaseFreeMemoryToOS(Context, Recorder, SkipRegion); -} - } // namespace scudo #endif // SCUDO_RELEASE_H_ diff --git a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp --- a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp @@ -220,12 +220,12 @@ auto SkipRegion = [](UNUSED scudo::uptr RegionIndex) { return false; }; auto DecompactPtr = [](scudo::uptr P) { return P; }; ReleasedPagesRecorder Recorder; - scudo::PageReleaseContext Context(BlockSize, - /*RegionSize=*/MaxBlocks * BlockSize, - /*NumberOfRegions=*/1U, + scudo::PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, /*ReleaseSize=*/MaxBlocks * BlockSize); ASSERT_FALSE(Context.hasBlockMarked()); - Context.markFreeBlocks(FreeList, DecompactPtr, Recorder.getBase()); + Context.markFreeBlocksInRegion(FreeList, DecompactPtr, Recorder.getBase(), + /*RegionIndex=*/0, MaxBlocks * BlockSize, + /*MayContainLastBlockInRegion=*/true); ASSERT_TRUE(Context.hasBlockMarked()); releaseFreeMemoryToOS(Context, Recorder, SkipRegion); scudo::RegionPageMap &PageMap = Context.PageMap; @@ -328,10 +328,10 @@ const scudo::uptr GroupBeg = GroupId * GroupSize; const scudo::uptr GroupEnd = GroupBeg + GroupSize; - scudo::PageReleaseContext Context(BlockSize, RegionSize, - /*NumberOfRegions=*/1U, + scudo::PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, /*ReleaseSize=*/RegionSize); - Context.markRangeAsAllCounted(GroupBeg, GroupEnd, /*Base=*/0U); + Context.markRangeAsAllCounted(GroupBeg, GroupEnd, /*Base=*/0U, + /*RegionIndex=*/0, RegionSize); scudo::uptr FirstBlock = ((GroupBeg + BlockSize - 1) / BlockSize) * BlockSize; @@ -398,10 +398,10 @@ } // Iterate each Group // Release the entire region. This is to ensure the last page is counted. - scudo::PageReleaseContext Context(BlockSize, RegionSize, - /*NumberOfRegions=*/1U, + scudo::PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, /*ReleaseSize=*/RegionSize); - Context.markRangeAsAllCounted(/*From=*/0U, /*To=*/RegionSize, /*Base=*/0); + Context.markRangeAsAllCounted(/*From=*/0U, /*To=*/RegionSize, /*Base=*/0, + /*RegionIndex=*/0, RegionSize); for (scudo::uptr Page = 0; Page < RoundedRegionSize / PageSize; ++Page) EXPECT_TRUE(Context.PageMap.isAllCounted(/*Region=*/0, Page)); } // Iterate each size class @@ -439,7 +439,7 @@ } // This follows the logic how we count the last page. It should be - // consistent with how markFreeBlocks() handles the last block. + // consistent with how markFreeBlocksInRegion() handles the last block. if (RoundedRegionSize % BlockSize != 0) ++Pages.back(); @@ -477,10 +477,12 @@ // Test marking by visiting each block. { auto DecompactPtr = [](scudo::uptr P) { return P; }; - scudo::PageReleaseContext Context( - BlockSize, RegionSize, /*NumberOfRegions=*/1U, - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase); - Context.markFreeBlocks(FreeList, DecompactPtr, /*Base=*/0U); + scudo::PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, + /*ReleaseSize=*/RegionSize - PageSize, + ReleaseBase); + Context.markFreeBlocksInRegion(FreeList, DecompactPtr, /*Base=*/0U, + /*RegionIndex=*/0, RegionSize, + /*MayContainLastBlockInRegion=*/true); for (const Batch &It : FreeList) { for (scudo::u16 I = 0; I < It.getCount(); I++) { scudo::uptr Block = It.get(I); @@ -497,10 +499,11 @@ // Test range marking. { - scudo::PageReleaseContext Context( - BlockSize, RegionSize, /*NumberOfRegions=*/1U, - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase); - Context.markRangeAsAllCounted(ReleaseBase, RegionSize, /*Base=*/0U); + scudo::PageReleaseContext Context(BlockSize, /*NumberOfRegions=*/1U, + /*ReleaseSize=*/RegionSize - PageSize, + ReleaseBase); + Context.markRangeAsAllCounted(ReleaseBase, RegionSize, /*Base=*/0U, + /*RegionIndex=*/0, RegionSize); for (scudo::uptr Page = ReleaseBase / PageSize; Page < RoundedRegionSize / PageSize; ++Page) { if (Context.PageMap.get(/*Region=*/0, Page - BasePageOffset) != @@ -515,13 +518,12 @@ // Check the buffer size of PageMap. { - scudo::PageReleaseContext Full(BlockSize, RegionSize, - /*NumberOfRegions=*/1U, + scudo::PageReleaseContext Full(BlockSize, /*NumberOfRegions=*/1U, /*ReleaseSize=*/RegionSize); Full.ensurePageMapAllocated(); - scudo::PageReleaseContext Partial( - BlockSize, RegionSize, /*NumberOfRegions=*/1U, - /*ReleaseSize=*/RegionSize - PageSize, ReleaseBase); + scudo::PageReleaseContext Partial(BlockSize, /*NumberOfRegions=*/1U, + /*ReleaseSize=*/RegionSize - PageSize, + ReleaseBase); Partial.ensurePageMapAllocated(); EXPECT_GE(Full.PageMap.getBufferSize(), Partial.PageMap.getBufferSize());