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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis accepted this revision.
eugenis added a comment.
This revision is now accepted and ready to land.

LGTM with a nit



================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:1135
+
+    storeRingBufferEntry(untagPointer(Ptr), Trace, Tid, Size, 0, 0);
+  }
----------------
pcc wrote:
> eugenis wrote:
> > Why create this ring buffer entry while the allocation is still alive?
> > This would add some complexity to error reporting, but we could iterate over the live secondary allocations to find the right one. That should reduce ring buffer traffic by 2x (not counting primary of course).
> > 
> > 
> It would add quite a bit of complexity. malloc_iterate is implemented via pointer chasing, whereas `__scudo_get_error_info` expects a few fixed blocks of data to be copied. Implementing the pointer chasing on the `__scudo_get_error_info` side would probably require callbacks to read memory, or something like that. It doesn't seem worth it to reduce ring buffer traffic for secondary allocations, which are quite uncommon anyway.
Would `__atomic_signal_fence(__ATOMIC_SEQ_CST)` be more appropriate?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94212