<div dir="auto">> <span style="font-family:sans-serif;font-size:12.8px">glider marked 2 inline comments as done.</span><div dir="auto"><span style="font-family:sans-serif;font-size:12.8px">No, I didn't. Will send updated patch tomorrow.</span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 15, 2019, 19:11 Alexander Potapenko via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">glider marked 2 inline comments as done.<br>
glider added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2556<br>
+        llvm::findAllocaForValue(I.getArgOperand(1), AllocaForValue);<br>
+    if (!AI)<br>
+      return;<br>
----------------<br>
eugenis wrote:<br>
> 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.<br>
> Alternatively, since we know the size, we could instrument this lifetime.start anyway using an invalid origin id.<br>
> <br>
I like your first proposal better.<br>
Using an invalid origin is impractical, as we won't be able to even find the function this poisoned local belongs to.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3446<br>
<br>
-  void visitAllocaInst(AllocaInst &I) {<br>
-    setShadow(&I, getCleanShadow(&I));<br>
-    setOrigin(&I, getCleanOrigin());<br>
+  void delayedVisitAllocaInst(AllocaInst &I) {<br>
     IRBuilder<> IRB(I.getNextNode());<br>
----------------<br>
eugenis wrote:<br>
> Let's call this function InstrumentAlloca, and rename instrumentAllocaUserspace to poisonAllocaUserspace.<br>
> <br>
same for instrumentAllocaKmsan?<br>
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D60617/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D60617/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D60617" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D60617</a><br>
<br>
<br>
<br>
</blockquote></div>