[scudo] Call getStats when the region is exhausted

[Chia-hungDuan]

Summary

Because of lock contention, we temporarily disabled the printing of
regions' status when it's exhausted. Given that it's useful when the
Region OOM happens, this CL brings it back without lock contention.

[cryptoad]

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

112

Both this one and the one below should probably be const.

146

Looks like there might an extra semi-colon here.

155

Is the change from ==1 to >=2 related to the issue at hand?

170

I agree that it should die if we can't get BatchClassId blocks, otherwise it's going to spiral quickly into something that might be harder to debug.

[Chia-hungDuan]

Address review comment

[Chia-hungDuan]

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

155

Oh sorry, it's not related to any issue. I thought that may be more clear. If you think it's better to keep as before, I'll change it back

170

Done and abort

[cferris]

Just small nits.

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

112

Another small nit, this is probably better named RegionIsExhausted.

125

Might be better expressed as:

requires locking each region

176

Theoretically

[Chia-hungDuan]

Address review comments

[cferris]

LGTM

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

if (PrimaryBase)
       unmap(reinterpret_cast<void *>(PrimaryBase), PrimarySize, UNMAP_ALL,
             &Data);
     PrimaryBase = 0U;
   }
 
   TransferBatch *popBatch(CacheT *C, uptr ClassId) {
     DCHECK_LT(ClassId, NumClasses);
     RegionInfo *Region = getRegionInfo(ClassId);
+    bool PrintStats = false;
+    {
       ScopedLock L(Region->Mutex);
       TransferBatch *B = popBatchImpl(C, ClassId);
-    if (UNLIKELY(!B)) {
-      if (UNLIKELY(!populateFreeList(C, ClassId, Region)))
-        return nullptr;
+      if (LIKELY(B)) {
+        Region->Stats.PoppedBlocks += B->getCount();
+        return B;
+      }
+
+      const bool RegionIsExhausted = Region->Exhausted;
+      if (UNLIKELY(RegionIsExhausted ||
+                   !populateFreeList(C, ClassId, Region))) {
+        PrintStats = !RegionIsExhausted && Region->Exhausted;
+      } else {
         B = popBatchImpl(C, ClassId);
         // if `populateFreeList` succeeded, we are supposed to get free blocks.
         DCHECK_NE(B, nullptr);
-    }
         Region->Stats.PoppedBlocks += B->getCount();
         return B;
       }
+    }
+
+    // Note that `getStats()` requires locking each region so we can't call it
+    // while locking the Region->Mutex in the above.
+    if (UNLIKELY(PrintStats)) {
+      ScopedString Str;
+      getStats(&Str);
+      Str.append(
+          "Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
+          RegionSize >> 20, getSizeByClassId(ClassId));
+      Str.output();
+    }
+    return nullptr;
+  }
 
   // Push the array of free blocks to the designated batch group.
   void pushBlocks(CacheT *C, uptr ClassId, CompactPtrT *Array, u32 Size) {
     DCHECK_LT(ClassId, NumClasses);
     DCHECK_GT(Size, 0);
 
     RegionInfo *Region = getRegionInfo(ClassId);
     if (ClassId == SizeClassMap::BatchClassId) {
+      bool PrintStats = false;
+      {
         ScopedLock L(Region->Mutex);
         // Constructing a batch group in the free list will use two blocks in
         // BatchClassId. If we are pushing BatchClassId blocks, we will use the
         // blocks in the array directly (can't delegate local cache which will
         // cause a recursive allocation). However, The number of free blocks may
         // be less than two. Therefore, populate the free list before inserting
         // the blocks.
-      if (Size == 1 && UNLIKELY(!populateFreeList(C, ClassId, Region)))
-        return;
-      pushBlocksImpl(C, ClassId, Array, Size);
+        if (Size >= 2U) {
+          pushBlocksImpl(C, SizeClassMap::BatchClassId, Array, Size);
           Region->Stats.PushedBlocks += Size;
+        } else {
+          const bool RegionIsExhausted = Region->Exhausted;
+          if (UNLIKELY(
+                  RegionIsExhausted ||
+                  !populateFreeList(C, SizeClassMap::BatchClassId, Region))) {
+            PrintStats = !RegionIsExhausted && Region->Exhausted;
+          }
+        }
+      }
+
+      // Note that `getStats()` requires the lock of each region so we can't
+      // call it while locking the Region->Mutex in the above.
+      if (UNLIKELY(PrintStats)) {
+        ScopedString Str;
+        getStats(&Str);
+        Str.append(
+            "Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
+            RegionSize >> 20, getSizeByClassId(ClassId));
+        Str.output();
+        // Theoretically, BatchClass shouldn't be used up. Abort immediately
+        // when it happens.
+        reportOutOfBatchClass();
+      }
       return;
     }
 
     // TODO(chiahungduan): Consider not doing grouping if the group size is not
     // greater than the block size with a certain scale.
 
     // Sort the blocks so that blocks belonging to the same group can be pushed
     // together.
NOINLINE bool populateFreeList(CacheT *C, uptr ClassId, RegionInfo *Region) {
     const uptr TotalUserBytes = Region->AllocatedUser + MaxCount * Size;
     // Map more space for blocks, if necessary.
     if (TotalUserBytes > MappedUser) {
       // Do the mmap for the user memory.
       const uptr MapSize =
           roundUpTo(TotalUserBytes - MappedUser, MapSizeIncrement);
       const uptr RegionBase = RegionBeg - getRegionBaseByClassId(ClassId);
       if (UNLIKELY(RegionBase + MappedUser + MapSize > RegionSize)) {
-        if (!Region->Exhausted) {
-          Region->Exhausted = true;
-          ScopedString Str;
-          // FIXME: getStats() needs to go over all the regions and
-          // will take the locks of them. Which means we will try to recursively
-          // acquire the `Region->Mutex` which is not supported. It will be
-          // better to log this somewhere else.
-          // getStats(&Str);
-          Str.append(
-              "Scudo OOM: The process has exhausted %zuM for size class %zu.\n",
-              RegionSize >> 20, Size);
-          Str.output();
-        }
         return false;
       }
       if (MappedUser == 0)
         Region->Data = Data;
       if (UNLIKELY(!map(
               reinterpret_cast<void *>(RegionBeg + MappedUser), MapSize,
               "scudo:primary",
               MAP_ALLOWNOMEM | MAP_RESIZABLE |

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

 
 // Sanity checks related error.
 void NORETURN reportSanityCheckError(const char *Field);
 
 // Combined allocator errors.
 void NORETURN reportAlignmentTooBig(uptr Alignment, uptr MaxAlignment);
 void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,
                                          uptr MaxSize);
+void NORETURN reportOutOfBatchClass();
 void NORETURN reportOutOfMemory(uptr RequestedSize);
 void NORETURN reportSoftRSSLimit(uptr RssLimitMb);
 void NORETURN reportHardRSSLimit(uptr RssLimitMb);
 enum class AllocatorAction : u8 {
   Recycling,
   Deallocating,
   Reallocating,
   Sizing,

compiler-rt/lib/scudo/standalone/report.cpp

 void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,
                                          uptr MaxSize) {
   ScopedErrorReport Report;
   Report.append("requested allocation size %zu (%zu after adjustments) exceeds "
                 "maximum supported size of %zu\n",
                 UserSize, TotalSize, MaxSize);
 }
 
+void NORETURN reportOutOfBatchClass() {
+  ScopedErrorReport Report;
+  Report.append("BatchClass region is used up, can't hold any free block\n");
+}
+
 void NORETURN reportOutOfMemory(uptr RequestedSize) {
   ScopedErrorReport Report;
   Report.append("out of memory trying to allocate %zu bytes\n", RequestedSize);
 }
 
 static const char *stringifyAction(AllocatorAction Action) {
   switch (Action) {
   case AllocatorAction::Recycling: