<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 5/15/20 11:07 AM, Arthur Eubanks via
      llvm-dev wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAPW48soXzJDvjNw4DafF7LFGsz+q3606JVrNQT3iUy-BLWyepQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">On Thu, May 14, 2020 at 1:21 PM Evgenii Stepanov via llvm-dev <
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:

</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">Sanitizer passes really should not run before the inliner. For example,
ASan moves all allocas into a "mega-alloca" to obtain fixed frame layout
for reporting purposes. It also inserts a fake stack check in the function
prologue which will get duplicated (but will probably still work) after
inlining.

MSan removes readnone/readonly from all functions because they all update
shadow which is a memory write. Doing this early will suppress all kinds of
IPO.

I think ASan (and probably MSan) does the same thing as TSan regarding
memory intrinsics because it needs the runtime library to be able to tell
which memcpy/memset calls are application logic (and therefore need to
check/update shadow) and which are compiler-inserted after sanitizer
instrumentation. Doing this too early suppresses some optimizations.

It's probably OK to run the passes a little earlier (but not before
inlining), but I'd be careful to weigh the benefits of that against the
loss of performance. If the goal is to catch more bugs (specifically, any
undefined behavior that gets optimized out before instrumentation), I'd
rather do something in the frontend and in the IR passes suppress those
optimizations in sanitized functions.

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">I think I've seen some function attributes added when sanitizers are
enabled, maybe we can use those to e.g. not strip lifetime intrinsics. I'll
look into that.</pre>
    </blockquote>
    <p>I guess that is an option we have to see how invasive this would
      be.</p>
    <p>I could imagine to use the `isDroppable` interface for this as we
      wanted</p>
    <p>to include lifetime markers and other things in there anyway.
      Passes can</p>
    <p>replace their explicit handling of lifetime markers for an
      `isDroppable`</p>
    <p> check that is influenced by the function attributes.<br>
    </p>
    <p><br>
    </p>
    <p>Would it make sense to have an option that allows the user to
      modify</p>
    <p> the sanitizer position, thus exposing the trade off to them?<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
cite="mid:CAPW48soXzJDvjNw4DafF7LFGsz+q3606JVrNQT3iUy-BLWyepQ@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">

On Thu, May 14, 2020 at 12:56 PM Eric Christopher via llvm-dev <
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:

</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">

On Thu, May 14, 2020 at 3:13 AM Fedor Sergeev via llvm-dev <
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:

</pre>
          <blockquote type="cite">
            <pre class="moz-quote-pre" wrap="">

On 5/14/20 5:33 AM, Arthur Eubanks via llvm-dev wrote:

</pre>
            <blockquote type="cite">
              <pre class="moz-quote-pre" wrap="">Is it the case that with the legacy PM there is no inlining at either
</pre>
            </blockquote>
            <pre class="moz-quote-pre" wrap="">-O2 or -O3 and with newPM there is? Or is there something else going on?
Legacy PM inlines at -O2/-O3, new PM inlines at -O1/-O2/-O3. These cases
where inlining occurs also coincide with the test failure. I agree that
inlining itself isn't the issue, but it seems to contribute to the
stripping of lifetime intrinsics and thus the test failure.

ASan needs lifetime intrinsics to implement use-after-scope. It may just
be luck that the legacy PM's -O1 happens to not strip lifetime intrinsics
before ASan gets to them?

I tried moving the *San passes before inlining and now the tests that I
was having trouble with pass, but new tests that specifically test inlining
fail, so I think it's probably fairly important to run *San passes after
inlining. But I tried moving them right after inlining (before
SROA/something gets to optimize things out) and some TSan tests now fail. I
found out TSan expects to be run after pretty much all other passes so that
it can change any memset/memcpy intrinsics into just the normal function
call to memset()/memcpy(). But if we move TSan to be run before
InstCombine, then InstCombine will change calls to memset()/memcpy() back
to the intrinsics and the tests don't like that. I think it's reasonable to
treat all *San passes the same way, so I don't think putting ASan somewhere
different from TSan makes sense. But that means there's no good place to
put these sanitizers.

I feel like a lot of these sanitizer tests just happened to work at -O1
under the legacy PM. I think I'll try having the new PM not inline when any
sanitizers are enabled and see if I can keep the house of cards standing.
Or is it a bad idea to change the PM behavior based on the detection of
sanitizers, since sanitizers won't be testing the same code you'd get
without sanitizing (e.g. a lot of the inlining tests would lose a lot of
their value)?

While trying newPM with inliner turned off on O1 is fine and probably a
right approach on digging into the cause of the failure,
actually changing Ox to be that different depending on sanitizer mode
seems to be a bad idea to me.


</pre>
          </blockquote>
          <pre class="moz-quote-pre" wrap="">Agreed. I don't want to turn off the inliner at O1. Reduce it down for
sure, but not turn it off. If you need help investigating these let me know
and we can try to help.

-eric


</pre>
          <blockquote type="cite">
            <pre class="moz-quote-pre" wrap="">regards,
  Fedor.


On Wed, May 13, 2020 at 3:14 PM Johannes Doerfert <
<a class="moz-txt-link-abbreviated" href="mailto:johannesdoerfert@gmail.com">johannesdoerfert@gmail.com</a>> wrote:

</pre>
            <blockquote type="cite">
              <pre class="moz-quote-pre" wrap="">
On 5/13/20 3:31 PM, David Blaikie via llvm-dev wrote:

I believe it's meant to run after /some/ optimizations to make it a bit
more efficient, while not so optimized that it misses opportunities to
detect bugs - but I could be wrong there. I'll leave it up to other folks
to chime in.

I think that is right. The more transformations you run the more UB you
can also "loose" as it is defined to something by the transformation.

Lifetime markers are an example. Once removed, which is generally legal
in IR, you cannot argue accesses after the end are UB.


On Wed, May 13, 2020 at 1:04 PM Arthur Eubanks <a class="moz-txt-link-rfc2396E" href="mailto:aeubanks@google.com"><aeubanks@google.com></a> <a class="moz-txt-link-rfc2396E" href="mailto:aeubanks@google.com"><aeubanks@google.com></a> wrote:


Just tested it out, that test does indeed fail under the old PM at -O3 and
even at -O2.

If the ASan pass runs after optimizations and is designed to detect
undefined behavior at runtime, I don't see how it can be super reliable at
higher optimization levels.

On Wed, May 13, 2020 at 12:39 PM David Blaikie <a class="moz-txt-link-rfc2396E" href="mailto:dblaikie@gmail.com"><dblaikie@gmail.com></a> <a class="moz-txt-link-rfc2396E" href="mailto:dblaikie@gmail.com"><dblaikie@gmail.com></a> wrote:


+some sanitizer/new pass manager folks



On Wed, May 13, 2020 at 12:22 PM Arthur Eubanks via llvm-dev <a class="moz-txt-link-rfc2396E" href="mailto:llvm-dev@lists.llvm.org"><llvm-dev@lists.llvm.org></a> wrote:


Hi,

I've been trying to burn down the remaining sanitizer failures under the
new pass manager. Right now I'm stuck on some ASan tests.

Some ASan tests run under -O1. There are a couple differences between
the old and new pass managers under -O1, e.g. the old PM doesn't inline
whereas the new PM does. The differences seem to cause some lifetime
intrinsics to get stripped out (e.g. via SROA, InstCombine). It might be
due to ASan specifically testing undefined behavior, and different
optimizations run means different behavior. For a specific example,
use-after-scope-dtor-order.cpp runs under -O1 and fails under the new PM
because SROA strips out the lifetime intrinsics and by the time the ASan
pass runs it doesn't find the lifetime intrinsics to add its own
instrumentation.


That, to me, sounds like a real bug in the optimizations/asan
implementation if this choice fo optimizations makes the diagnosis go away.
(is ASan able to diagnose the problem at -O3 (where I guess SROA and other
things run) with the legacy pass manager?)



What's the proper way to resolve this? Run the tests under -O0? Change
the passes pipeline under the new PM when ASan (and maybe other sanitizers)
is detected?
_______________________________________________
LLVM Developers mailing <a class="moz-txt-link-abbreviated" href="mailto:listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>


_______________________________________________
LLVM Developers mailing <a class="moz-txt-link-abbreviated" href="mailto:listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>


</pre>
            </blockquote>
            <pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing <a class="moz-txt-link-abbreviated" href="mailto:listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">listllvm-dev@lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>


_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>

</pre>
          </blockquote>
          <pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>

</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>

</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
</pre>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
    </blockquote>
  </body>
</html>