[PATCH] D34135: [LVI] Add initial result to avoid infinite getValueFromCondition recursion

[Daniel Berlin via llvm-commits]

On Thu, Jun 15, 2017 at 12:20 PM, Davide Italiano via Phabricator <
reviews at reviews.llvm.org> wrote:

> davide added a comment.
>
> In https://reviews.llvm.org/D34135#781243, @anna wrote:
>
> > In https://reviews.llvm.org/D34135#780695, @dberlin wrote:
> >
> > > In https://reviews.llvm.org/D34135#780666, @anna wrote:
> > >
> > > > In https://reviews.llvm.org/D34135#780637, @efriedma wrote:
> > > >
> > > > > The sequence here is that we constant-fold a terminator (deleting
> an edge), then continue iterating through all the blocks in the function,
> and eventually crash when LVI discovers a cycle in a loop which was made
> unreachable.
> > > >
> > > >
> > > > So, the main problem here is that in each iteration of Jump
> threading, we can find the trivially dead blocks, but not the complete set
> of unreachable blocks. Also, I'm assuming transitively detecting dead
> blocks after the call to `processBlock` does not help in this case?
> > >
> > >
> > > The set of blocks it will find will depend on input order of basic
> blocks currently.
> > >  It is possible to increase it to find all non-cycle involved
> unreachable blocks  by performing the processing in the optimal iteration
> order.
> > >  It should be possible to find all unreachable blocks (including those
> involved in cycle) as you go by forming SCC's and iterating them
> individually in the right order.
> >
> >
> > Agreed. Also, there was even a discussion on changing the ordering for
> JumpThreading because of how the current ordering can cause certain blocks
> to be dead, and AA chokes on the PHIs in those blocks:
> https://bugs.llvm.org/show_bug.cgi?id=33142#c14
> >  (This might be completely irrelevant for this patch, but it's around
> the same problem of identifying  unreachability at all times in
> JumpThreading).
>
>
> Yes. As the person who did the analysis on that bug I decided to not go
> that direction (i.e. use a stack worklist) because I wasn't able to prove
> myself it was correct.
> I've seen at least another example internally where JumpThreading goes
> bananas because of bad iteration ordering. Iterating SCC's topologically
> will solve at least part of  these problems,
> I think, but I'm afraid that's a much larger change.


Yup


> I wouldn't stop this from going in but at the same time it's likely
> Zhendong's fuzzer (or other fuzzers, or simply production code :) will
> uncover similar issues to this.
>

+1 to that.

I have no problem patching this in as long as we know it's just the start
of a long ride.
In some ways, unless *this* bug is important, and more important than the
other 15 bugs it's gonna find that we *aren't* going to be able to fix
without rewriting it, i kinda wouldn't bother.

If they are important (IE occurring on real world code that matters, etc),
well, good luck :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170615/52721b1f/attachment.html>