[PATCH] D60617: MSan: handle llvm.lifetime.start intrinsic

[Alexander Potapenko via Phabricator via llvm-commits]

glider marked 2 inline comments as done.
glider added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2556
+        llvm::findAllocaForValue(I.getArgOperand(1), AllocaForValue);
+    if (!AI)
+      return;
----------------
eugenis wrote:
> IMHO failure to find the underlying alloca must result in poisoning of all allocas in the prologue - exactly because we don't know which one we've missed.
> Alternatively, since we know the size, we could instrument this lifetime.start anyway using an invalid origin id.
> 
I like your first proposal better.
Using an invalid origin is impractical, as we won't be able to even find the function this poisoned local belongs to.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3446
 
-  void visitAllocaInst(AllocaInst &I) {
-    setShadow(&I, getCleanShadow(&I));
-    setOrigin(&I, getCleanOrigin());
+  void delayedVisitAllocaInst(AllocaInst &I) {
     IRBuilder<> IRB(I.getNextNode());
----------------
eugenis wrote:
> Let's call this function InstrumentAlloca, and rename instrumentAllocaUserspace to poisonAllocaUserspace.
> 
same for instrumentAllocaKmsan?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60617