RFR: 8286182: C2: crash with SIGFPE when executing compiled code

[Christian Hagedorn]

On Wed, 18 May 2022 00:11:56 GMT, Quan Anh Mai <duke at openjdk.java.net> wrote:

>> The fix looks reasonable to address the same problems in [JDK-8257822](https://bugs.openjdk.java.net/browse/JDK-8257822) and [JDK-8248552](https://bugs.openjdk.java.net/browse/JDK-8248552) for the new nodes `Div` and `Mod` nodes.
>> 
>>>  The problem is that NoOvfDivI does not only depend on the zero-divisor check but a possible overflow check as well. So with this fix it is still possible for a SIGFPE to occur.
>> 
>> Do you have a failing test for this case? We've been seeing a lot of SIGFPE failures lately with Java Fuzzer. I have to walk through them to see if I can find a case that is still failing with this fix. Will get back with the result of this analysis.
>> 
>>> IIUC this trouble comes from the fact that on x86 a Div node must be pinned to its zero-divisor check but may float with regards to other control nodes. Maybe we can remove all this special handling and simply catch SIGFPE instead? The result is guaranteed to not be used in those cases so we may not worry about the correctness of the compiled code.
>> 
>> I'm not sure if we should rely on signal catching to fix the cases where a division is wrongly floating above its zero check. I think we should not intentionally leave a graph in a broken state with the intention to fix it later at runtime.
>
>> Do you have a failing test for this case? We've been seeing a lot of SIGFPE failures lately with Java Fuzzer. I have to walk through them to see if I can find a case that is still failing with this fix. Will get back with the result of this analysis.
> 
> Overflow only happens with dividend = MIN_VALUE and divisor = -1 so it would be hard to have a failing test for this case. I will try to come up with one.
> 
>> I'm not sure if we should rely on signal catching to fix the cases where a division is wrongly floating above its zero check. I think we should not intentionally leave a graph in a broken state with the intention to fix it later at runtime.
> 
> IIUC the graph is broken only because the division raises SIGFPE for invalid inputs. If we choose to ignore the signal instead then we can treat the Div nodes similar to how we treat other nodes such as Add or Sub and let them freely float around.
> 
> Thanks.

I've checked all our fuzzer crashes and the current fix works for all of them. But given that there is potentially another issue with the overflow check mentioned by @merykitty, we could indeed think about backing the changes out for JDK 19 as we are getting closer to the fork. I'd suggest to wait and see for now if @merykitty was able to find a case where we still crash.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8726