[compiler-rt] [scudo] [MTE] resize stack depot for allocation ring buffer (PR #74515)

via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 18 21:32:29 PST 2024


================
@@ -1504,6 +1529,28 @@ class Allocator {
       return;
     u32 AllocationRingBufferSize =
         static_cast<u32>(getFlags()->allocation_ring_buffer_size);
+    // We store alloc and free stacks for each entry.
+    constexpr auto kStacksPerRingBufferEntry = 2;
+    u32 TabSize = static_cast<u32>(roundUpPowerOfTwo(kStacksPerRingBufferEntry *
+                                                     AllocationRingBufferSize));
+    constexpr auto kFramesPerStack = 8;
+    static_assert(isPowerOfTwo(kFramesPerStack));
+    u32 RingSize = static_cast<u32>(TabSize * kFramesPerStack);
+    DCHECK(isPowerOfTwo(RingSize));
+    static_assert(sizeof(StackDepot) % alignof(atomic_u64) == 0);
+
+    StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
+                     sizeof(atomic_u32) * TabSize;
+    MemMapT DepotMap;
+    DepotMap.map(
+        /*Addr=*/0U, roundUp(StackDepotSize, getPageSizeCached()),
+        "scudo:stack_depot");
+    RawStackDepot = reinterpret_cast<char *>(DepotMap.getBase());
+    auto *Depot = reinterpret_cast<StackDepot *>(DepotMap.getBase());
+    Depot->init(RingSize, TabSize);
+    DCHECK(Depot->isValid(StackDepotSize));
----------------
ChiaHungDuan wrote:

Sorry, I'm not saying how it's verified. I'm talking about the code structure. 

If the `DCHECK` fails, what will you review? It should be the `RingSize` and `TabSize` because they are used for the initialization. Therefore, checking the values before/in the middle of the initialization gives you the useful insight about what's going on. 

Initialization usually involves several steps, it'd be better to tell when there's something wrong instead of carrying the error to the end. If it causes any crash, that DCHECK won't help anything. Besides, at the end of initialization, I would expect that everything is fine. For example, I don't need to verify if `std::vector<int>(4, 0)` gives me 4 elements and all of them are zero.

IMO, the `DCHECK` is not used at the right place.

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


More information about the llvm-commits mailing list