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

[Alexander Potapenko via llvm-commits]

> glider marked 2 inline comments as done.
No, I didn't. Will send updated patch tomorrow.

On Mon, Apr 15, 2019, 19:11 Alexander Potapenko via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190415/9f30a2da/attachment.html>