[PATCH] D85227: [Draft][MSAN] Cache stack traces and chained origins

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: compiler-rt/lib/msan/msan_origin.h:130
     if (flags()->origin_history_per_stack_limit > 0) {
-      int use_count = h.use_count();
-      if (use_count > flags()->origin_history_per_stack_limit) return prev;
+      // int use_count = h.use_count();
+      // if (use_count > flags()->origin_history_per_stack_limit) return prev;
----------------
guiand wrote:
> I'm only not really sure what to do about these use counts. Updating them in the depot somewhat defeats the purpose of trying to prevent depot accesses through caching.
The purpose of this is to limit the number of new chained origins that one access trace can create. We could cache this value in l1 and l2, and when we are about to create a chained origin, double-check against the global stack depot and refresh the caches.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1304
+  BasicBlock *insertTracePrologue(BasicBlock *B) {
+    BasicBlock *ret = SplitBlock(B, B->getFirstNonPHI());
+    IRBuilder<> IRB(B->getFirstNonPHI());
----------------
KMSan uses this SplitBlock thing to avoid visiting the prologue setup instructions, but it has a cost: it turns all allocas into dynamic allocas, as they are no longer in the entry basic block.

Try to avoid this by turning ActualFnStart into an instruction pointer, that should help with the code size and maybe performance a little.

This could be what is causing the local shadow addresses to be pre-calculated.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3774
     IRBuilder<> IRB(&I);
+    if (ClCachedUnwinding) {
+      // XOR out the current stack frame's hash to indicate returning up a
----------------
The same should be done on ResumeInst and probably more, otherwise this would keep generating unique hash values in an exception-enabled code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85227