[compiler-rt] [scudo] Allow to resize allocation ring buffer (PR #82683)

Christopher Ferris via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 12 15:20:12 PDT 2024


================
@@ -1594,12 +1600,33 @@ class Allocator {
     RB->StackDepotSize = StackDepotSize;
     RB->RawStackDepotMap = DepotMap;
 
-    atomic_store(&RingBufferAddress, reinterpret_cast<uptr>(RB),
-                 memory_order_release);
+    swapOutRingBuffer(RB);
     static_assert(sizeof(AllocationRingBuffer) %
                           alignof(typename AllocationRingBuffer::Entry) ==
                       0,
                   "invalid alignment");
+    return true;
+  }
+
+  void swapOutRingBuffer(AllocationRingBuffer *NewRB) {
+    // To allow resizeRingBuffer to be called in a multi-threaded context by apps,
+    // we do not actually unmap, but only madvise(DONTNEED) the pages. That way,
+    // straggler threads will not crash.
----------------
cferris1000 wrote:

I agree that the spec says that the data is either repopulated or reads back as zero. However, if multiple threads are accessing this at the same time, I would presume that the repopulate or read back zero is on a page by page basis. Is there something in the kernel that makes the operation atomic for the entire mapped data? The kernel might make this atomic by preventing all interrupts while executing the madvise.

Honestly, if this is only really used for debugging purposes, it might be better to simply let it leak away and skip the madvise completely. Maybe even log that you are leaking away the old data. Even the data going to zero requires that anyone ever touching this code has to remember that is a possibility, and might result in undetected bugs in the future.

The best way would have been to copy the data for callers rather than having pointers to it. But since this is really a debugging item, a simpler interface with some known issues is probably fine.

https://github.com/llvm/llvm-project/pull/82683


More information about the llvm-commits mailing list