[14] RFR(M): 8229496: SIGFPE (division by zero) in C2 OSR compiled method

[Tobias Hartmann]

Hi Martin,

thanks for looking at this!

On 02.09.19 18:38, Doerr, Martin wrote:
> I thought we were using implicit div by zero checks on x86 (SIGFPE gets caught in signal handler on AMD64). So was the continuation_for_implicit_div0_exception not found in ImplicitExceptionTable? Or why did the VM crash?

Yes, same as for implicit null checks, that's decided here:
http://hg.openjdk.java.net/jdk/jdk/file/2172fd713350/src/hotspot/share/opto/graphKit.cpp#l1312

I.e., we do an implicit null check if it doesn't happen frequently. However, in the failing case we
always throw and therefore an explicit zero check is used.

> I wonder if it's fine to use "if (lo1 >= 0) lo = 0" in the CastIINode / CastLLNode. Shouldn't lo > 0 stay valid?
> (Also in the existing CastIINode, but your usage is new.)

The type is intentionally worsened here. See:
http://hg.openjdk.java.net/jdk/jdk/file/2172fd713350/src/hotspot/share/opto/convertnode.cpp#l277

I'll refactor that code with https://bugs.openjdk.java.net/browse/JDK-8230382

> This issue sounds a little bit similar to 
> https://bugs.openjdk.java.net/browse/JDK-8220376
> but I guess there's more work to be done for that.

In general, the problem is that C2's type system is not able to propagate the information that a
value is "not zero". That's why I had to use TypeInt::INT for the CastII instead of something like
TypeInt::NOT_NULL.

Without having looked at your test for 8220376, I would assume that with x != 0, C2 merges the two
checks (the one that is in the Java code and the one added for the div) and therefore the div is
removed. That does not happen with x > 0.

Thanks,
Tobias