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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

In D60617#1467093 <https://reviews.llvm.org/D60617#1467093>, @glider wrote:

> In D60617#1464971 <https://reviews.llvm.org/D60617#1464971>, @eugenis wrote:
>
> > Please test the case when alloca can not be found.
> >  See https://bugs.llvm.org/show_bug.cgi?id=41481 for the reference.
>
>
> Ok, will do. The repro in the bug doesn't produce IR containing a select statement for me, but the idea is clear.
>
> > 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.
>
> Agreed. This also makes less sense without origins - do you think we shall hide it behind track-origins>0 ?


No, I think it is a good change. But it would be nice to have an llvm flag to back out just in case.

>> Do you plan to remove SanitizeMemory condition from shouldEmitLifetimeMarkers ?
> 
> Sorry, I don't get it. We won't see any lifetime markers with that condition removed, right?
>  What's the point of this fix then?

We will see a lot more lifetime markers.
I'm talking about this:

  // Disable lifetime markers in msan builds.
  // FIXME: Remove this when msan works with lifetime markers.
  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
    return false;

http://llvm.org/viewvc/llvm-project?view=revision&revision=235613


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60617