[scudo] Slightly improve the handling of last block in a region

[Chia-hungDuan]

Summary

Instead of going through all those trailing blocks, just count the
number and increase the counter at once.

[Chia-hungDuan]

Add more comments

[cferris]

Are there tests that already cover the modified code?

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

499–500

Is this comment still valid? It doesn't seem like the code marks anything unless there are some trailing blocks.

[Chia-hungDuan]

Add more comment

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

499–500

Yes, we are still handling the trailing blocks (which is the "pretend" blocks mentioned in the comment). The difference is we don't iterate each trailing block.

Added more comments

[Chia-hungDuan]

In D145419#4176234, @cferris wrote:

Are there tests that already cover the modified code?

Yes, our tests have covered the handling of the last block in the region already.

[cferris]

LGTM

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

NOINLINE uptr releaseToOSMaybe(SizeClassInfo *Sci, uptr ClassId,
     };
     for (BatchGroup &BG : Sci->FreeList) {
       const uptr PushedBytesDelta =
           BG.PushedBlocks - BG.PushedBlocksAtLastCheckpoint;
       if (PushedBytesDelta * BlockSize < PageSize)
         continue;
 
       const uptr GroupBase = decompactGroupBase(BG.CompactPtrGroupBase);
-      uptr AllocatedGroupSize =
-          GroupBase == CurGroupBase ? Sci->CurrentRegionAllocated : GroupSize;
+      // The `GroupSize` may not be divided by `BlockSize`, which means there is
+      // an unused space at the end of Region. Exclude that space to avoid
+      // unused page map entry.
+      uptr AllocatedGroupSize = GroupBase == CurGroupBase
+                                    ? Sci->CurrentRegionAllocated
+                                    : roundDownSlow(GroupSize, BlockSize);
       if (AllocatedGroupSize == 0)
         continue;
 
       // TransferBatches are pushed in front of BG.Batches. The first one may
       // not have all caches used.
       const uptr NumBlocks = (BG.Batches.size() - 1) * BG.MaxCachedPerBatch +
                              BG.Batches.front()->getCount();
       const uptr BytesInBG = NumBlocks * BlockSize;

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

void markFreeBlocksInRegion(const IntrusiveList<TransferBatchT> &FreeList,
                               DecompactPtrT DecompactPtr, const uptr Base,
                               const uptr RegionIndex, const uptr RegionSize,
                               bool MayContainLastBlockInRegion) {
     ensurePageMapAllocated();
 
     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;
-      }
+      const uptr PInRegion = LastBlockInRegion + BlockSize;
+      DCHECK_LT(RoundedRegionSize - PInRegion, PageSize);
+      uptr NumTrailingBlocks =
+          roundUpSlow(RoundedRegionSize - PInRegion, BlockSize) / BlockSize;
+      if (NumTrailingBlocks > 0)
+        PageMap.incN(RegionIndex, getPageIndex(PInRegion), NumTrailingBlocks);
     }
 
     // Iterate over free chunks and count how many free chunks affect each
     // allocated page.
     if (BlockSize <= PageSize && PageSize % BlockSize == 0) {
       // Each chunk affects one page only.
       for (const auto &It : FreeList) {
         for (u16 I = 0; I < It.getCount(); I++) {