[scudo] Deallocate the AllocatorRingBuffer too in unmapTestOnly

[fabio-d]

Summary

The AllocatorRingBuffer is allocated dynamically when Allocator is
initialized. This patch adds a corresponding deinitialization call in
unmapTestOnly, to avoid running out of virtual memory if the tests are run
a large number of times on memory-constrained platforms.

[paulkirth]

Can you provide a more complete summary of the problem, and how this addresses it? If there is an tracking issue on github, it's fine to keep the summary terse if the bug covers it in more detail, but this probably needs more context for anyone coming along later to understand why this is required, and the problem you're trying to solve.

[Chia-hungDuan]

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

1523

It looks like we already store the size, can we use this instead? Besides, we may want reset them to zero like

RawRingBuffer = nullptr;
RingBuffer->Size = 0;

1530

I may suggest some name like deallocateRingBuffer or destroyRingBuffer

[fabio-d]

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

1523

Done, thanks! There was no need to set the size back to 0 because RingBuffer->Size is actually stored in the memory region that is unmapped.

1530

Renamed to destroyRingBuffer, thanks!

[paulkirth]

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

1530

fly-by-comment: you may want to rename the initialization function something like mapAndInitialzeRingBuffer and this to unmapRingBuffer, since 1) that more closely matches what's going on, and 2) the relationship is obvious from the naming.

alternatively using allocate/deallocate instead of map is equally good.

[Chia-hungDuan]

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

1519–1520

BTW, I think this roundUp() is something we want to fix as well. Just add a note here.

[pcc]

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

1532

Why not call getRingBufferSize()?

[Chia-hungDuan]

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

1532

Oh I forgot we have this.
BTW, even if we use getRingBufferSize(), we still need to do the roundUp. The need of roundUp will be fixed later.

[fabio-d]

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

1530

Renamed to mapAndInitializeRingBuffer and unmapRingBuffer as you suggested.

1532

Thanks! I've updated the CL to use getRingBufferSize() while keeping the roundUp

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

void init() {
     Stats.init();
     const s32 ReleaseToOsIntervalMs = getFlags()->release_to_os_interval_ms;
     Primary.init(ReleaseToOsIntervalMs);
     Secondary.init(&Stats, ReleaseToOsIntervalMs);
     Quarantine.init(
         static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
         static_cast<uptr>(getFlags()->thread_local_quarantine_size_kb << 10));
 
-    initRingBuffer();
+    mapAndInitializeRingBuffer();
   }
 
   // Initialize the embedded GWP-ASan instance. Requires the main allocator to
   // be functional, best called from PostInitCallback.
   void initGwpAsan() {
 #ifdef GWP_ASAN_HOOKS
     gwp_asan::options::Options Opt;
     Opt.Enabled = getFlags()->GWP_ASAN_Enabled;
#ifdef GWP_ASAN_HOOKS
   }
 #endif // GWP_ASAN_HOOKS
 
   ALWAYS_INLINE void initThreadMaybe(bool MinimalInit = false) {
     TSDRegistry.initThreadMaybe(this, MinimalInit);
   }
 
   void unmapTestOnly() {
+    unmapRingBuffer();
     TSDRegistry.unmapTestOnly(this);
     Primary.unmapTestOnly();
     Secondary.unmapTestOnly();
 #ifdef GWP_ASAN_HOOKS
     if (getFlags()->GWP_ASAN_InstallSignalHandlers)
       gwp_asan::segv_handler::uninstallSignalHandlers();
     GuardedAlloc.uninitTestOnly();
 #endif // GWP_ASAN_HOOKS
return &reinterpret_cast<typename AllocationRingBuffer::Entry *>(
         &RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
   }
   static const typename AllocationRingBuffer::Entry *
   getRingBufferEntry(const char *RawRingBuffer, uptr N) {
     return &reinterpret_cast<const typename AllocationRingBuffer::Entry *>(
         &RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
   }
 
-  void initRingBuffer() {
+  void mapAndInitializeRingBuffer() {
     u32 AllocationRingBufferSize =
         static_cast<u32>(getFlags()->allocation_ring_buffer_size);
     if (AllocationRingBufferSize < 1)
       return;
-    MapPlatformData Data = {};
     RawRingBuffer = static_cast<char *>(
         map(/*Addr=*/nullptr,
             roundUp(ringBufferSizeInBytes(AllocationRingBufferSize),
                     getPageSizeCached()),
-            "AllocatorRingBuffer", /*Flags=*/0, &Data));
+            "AllocatorRingBuffer"));
     auto *RingBuffer = reinterpret_cast<AllocationRingBuffer *>(RawRingBuffer);
     RingBuffer->Size = AllocationRingBufferSize;
     static_assert(sizeof(AllocationRingBuffer) %
                           alignof(typename AllocationRingBuffer::Entry) ==
                       0,
                   "invalid alignment");
   }
 
+  void unmapRingBuffer() {
+    unmap(RawRingBuffer, roundUp(getRingBufferSize(), getPageSizeCached()));
+    RawRingBuffer = nullptr;
+  }
+
   static constexpr size_t ringBufferSizeInBytes(u32 AllocationRingBufferSize) {
     return sizeof(AllocationRingBuffer) +
            AllocationRingBufferSize *
                sizeof(typename AllocationRingBuffer::Entry);
   }
 
   inline AllocationRingBuffer *getRingBuffer() {
     return reinterpret_cast<AllocationRingBuffer *>(RawRingBuffer);
   }
 };
 
 } // namespace scudo
 
 #endif // SCUDO_COMBINED_H_