[PATCH] D94212: scudo: Add support for tracking stack traces of secondary allocations.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In D94212#2488097 <https://reviews.llvm.org/D94212#2488097>, @eugenis wrote:

> Can we have a setting where the secondary ring buffer is used for primary allocations, too? The primary record could be pushed when the chunk is about to be reused.

Let's do that in a separate change.



================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:1306
+    for (uptr I = Pos - 1; I != Pos - 1 - SecondaryRingBuffer::NumEntries;
+         --I) {
+      auto *Entry = &RingBuffer.Entries[I % SecondaryRingBuffer::NumEntries];
----------------
eugenis wrote:
> This loop seems expensive. Why not store alloc and dealloc as two independent events? It will use a bit more memory, but given the same memory budget, dealloc events should survive longer because they will be generally closer to the end of the ring (and they are the most interesting ones).
> 
Hmm, I wouldn't expect it to be that expensive compared to the cost of the allocation itself.

I thought about storing these as two separate events but I wasn't sure that just a deallocation stack trace on its own would be useful. But having given it more thought I suppose that:
1. We don't need to concern ourselves too much with memory usage here because secondary allocations are rare enough that we can choose a relatively small buffer size, so doubling it shouldn't matter that much
2. Just the deallocation stack trace would be better than nothing. That being said, we could store the allocation trace and tid inline with the allocation and copy them into the ring buffer entry on deallocation. That would cost more memory, but as mentioned in 1) that should be inconsequential.
So let's go with the separate events.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94212/new/

https://reviews.llvm.org/D94212