[compiler-rt] [NFC] Make RingBuffer an atomic pointer (PR #82547)

via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 21 16:59:41 PST 2024


================
@@ -1544,42 +1557,46 @@ class Allocator {
     u32 RingSize = static_cast<u32>(TabSize * kFramesPerStack);
     DCHECK(isPowerOfTwo(RingSize));
 
-    StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
-                     sizeof(atomic_u32) * TabSize;
+    auto StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
+                          sizeof(atomic_u32) * TabSize;
     MemMapT DepotMap;
     DepotMap.map(
         /*Addr=*/0U, roundUp(StackDepotSize, getPageSizeCached()),
         "scudo:stack_depot");
-    Depot = reinterpret_cast<StackDepot *>(DepotMap.getBase());
+    auto *Depot = reinterpret_cast<StackDepot *>(DepotMap.getBase());
     Depot->init(RingSize, TabSize);
-    RawStackDepotMap = DepotMap;
 
     MemMapT MemMap;
     MemMap.map(
         /*Addr=*/0U,
         roundUp(ringBufferSizeInBytes(AllocationRingBufferSize),
                 getPageSizeCached()),
         "scudo:ring_buffer");
-    RawRingBuffer = reinterpret_cast<char *>(MemMap.getBase());
-    RawRingBufferMap = MemMap;
-    RingBufferElements = AllocationRingBufferSize;
+    auto *RB = reinterpret_cast<AllocationRingBuffer *>(MemMap.getBase());
+    RB->RawRingBufferMap = MemMap;
+    RB->RingBufferElements = AllocationRingBufferSize;
+    RB->Depot = Depot;
+    RB->StackDepotSize = StackDepotSize;
+    RB->RawStackDepotMap = DepotMap;
+
+    atomic_store(&RingBuffer, RB, memory_order_release);
     static_assert(sizeof(AllocationRingBuffer) %
                           alignof(typename AllocationRingBuffer::Entry) ==
                       0,
                   "invalid alignment");
   }
 
   void unmapRingBuffer() {
-    auto *RingBuffer = getRingBuffer();
-    if (RingBuffer != nullptr) {
-      RawRingBufferMap.unmap(RawRingBufferMap.getBase(),
-                             RawRingBufferMap.getCapacity());
-    }
-    RawRingBuffer = nullptr;
-    if (Depot) {
-      RawStackDepotMap.unmap(RawStackDepotMap.getBase(),
-                             RawStackDepotMap.getCapacity());
+    AllocationRingBuffer *RB = getRingBuffer();
+    if (RB != nullptr) {
+      // N.B. because RawStackDepotMap is part of RawRingBufferMap, the order
+      // is very important.
+      RB->RawStackDepotMap.unmap(RB->RawStackDepotMap.getBase(),
+                                 RB->RawStackDepotMap.getCapacity());
+      RB->RawRingBufferMap.unmap(RB->RawRingBufferMap.getBase(),
+                                 RB->RawRingBufferMap.getCapacity());
     }
+    atomic_store(&RingBuffer, nullptr, memory_order_release);
----------------
ChiaHungDuan wrote:

I would move this into `if (RB != nullptr)`, otherwise, it seems a redundant operation for the case that buffer is not enabled.

Besides, I would early return if `RB == nullptr` to remove the nesting

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


More information about the llvm-commits mailing list