Missing deopt for MIN_VALUE / -1 case causes crash from unhandled SIGFPE

[Jackson Davis]

I should add that this "fix" is based on the idea that letting the idiv
throw the SIGFPE is, in fact, the desired / expected behavior here. C2
handles this case by adding guards.

-Jackson

On Fri, Dec 1, 2017 at 12:04 AM, Jackson Davis <jackson at jcdav.is> wrote:

> One afternoon over Thanksgiving break I hacked together a very basic
> fuzzer for JVM JITs (https://github.com/jcdavis/kabur/) which much to my
> surprise actually managed to find an issue in Graal :)
> This minimal test case reliably crashes the JVM from an unhandled SIGFPE:
>
> public class Crasher {
>   public static int crash(int n, int d) {
>     return n/(d|1);
>   }
>   public static void main(String[] args) {
>     for(int i = 0; i < 100000; i++) {
>       crash(i, i+1);
>     }
>     crash(Integer.MIN_VALUE, -1);
>   }
> }
> Ran with -XX:+UseJVMCICompiler -XX:-TieredCompilation
> -XX:-BackgroundCompilation (Tested with local jdk9 and graal master)
>
> The source of this issue appears to be that IntegerDivRemNode.canDeoptimize
> only checks whether or not the denominator can be 0, not whether it can
> also be -1. Since d|1 is never 0, it doesn't generate the appropriate deopt
> info, so when MIN_VALUE/-1 causes a SIGFPE, the signal handler doesn't know
> what to do and dies. Adding the -1 check seems to make this to work as
> intended (AFAICT). I have a PR for a basic fix here:
> https://github.com/graalvm/graal/pull/266
>
> Unfortunately this check breaks CountedLoopTest in a non-obvious way,
> which is where I am out of my depth. It seems the test's CheckGraphPhase
> introduces a SignedDivNode, however since this runs after
> FrameStateAssignmentPhase, there is no frame data, so an assert fails in
> the lowering. This seems to be fundamentally broken regardless of my
> change, but only started failing here since the denominator has as stamp of
> (-65536,-1)
>
> -Jackson
>
>
>