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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

Please test the case when alloca can not be found.
See https://bugs.llvm.org/show_bug.cgi?id=41481 for the reference.

With this change msan will start detecting new bugs, ex.:
for (int i = 0; ...; ++i) {

  int x;
  if (i == 0) x=0;
  read(x);

}

It can also slow down some programs a lot.

Do you plan to remove SanitizeMemory condition from shouldEmitLifetimeMarkers ?



================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2556
+        llvm::findAllocaForValue(I.getArgOperand(1), AllocaForValue);
+    if (!AI)
+      return;
----------------
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.



================
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());
----------------
Let's call this function InstrumentAlloca, and rename instrumentAllocaUserspace to poisonAllocaUserspace.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60617