This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] guard against an oversized shift crash
ClosedPublic

Authored by spatel on Nov 27 2018, 8:29 AM.

Details

Summary

This change prevents the crash noted in the post-commit comments for rL347478 :
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20181119/605166.html

We can't guarantee that an oversized shift amount is folded away, so we have to check for it.

Note that I committed an incomplete fix for that crash with:
rL347502

But as discussed here:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20181126/605679.html
...we have to try harder.

So I'm not sure how to expose the bug now (and apparently no fuzzers have found a way yet either).

On the plus side, we have discovered that we're missing real optimizations by not simplifying nodes sooner, so the earlier fix still has value, and there's likely more value in extending that so we can simplify more opcodes and simplify when doing RAUW and/or putting nodes on the combiner worklist.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel created this revision.Nov 27 2018, 8:29 AM
bjope added a subscriber: bjope.Nov 28 2018, 11:01 AM
efriedma added inline comments.Nov 29 2018, 3:46 PM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
6105 ↗(On Diff #175489)

I think you need to use getLimitedValue() here?

spatel updated this revision to Diff 176140.Nov 30 2018, 8:46 AM
spatel marked an inline comment as done.

Patch updated:
Fixed to use "getLimitedValue" (make sure that really big values don't escape the check).

This revision is now accepted and ready to land.Nov 30 2018, 9:38 AM
This revision was automatically updated to reflect the committed changes.