RFR: 8299259: C2: Div/Mod nodes without zero check could be split through iv phi of loop resulting in SIGFPE
Christian Hagedorn
chagedorn at openjdk.org
Tue Jan 10 10:46:56 UTC 2023
On Mon, 9 Jan 2023 09:39:55 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
> A `Div/Mod` node can have an iv phi of a counted loop as a divisor input which has a type that does not include zero. As a result, the zero check is removed for the `Div/Mod` node. When splitting the `Div/Mod` node through the iv phi with `split_thru_phi()`, one clone ends up on the backedge with the loop increment node as divisor input which can include zero in its type:
>
> iv i type: [2..10]
> increment type: [0..8] // 0 on last iteration when the loop is exited
>
> for (int i = 10; i > 1; i -= 2) {
> iFld = 10 / i;
> }
>
> After split through phi:
>
> div1 = 10/10;
> loop:
> iFld = phi[div1, div2];
> if (i > 1) then exit
> i -= 2; // increment node
> div2 = 10 / i; // On last iteration: 10 / 0
> jump to loop;
> }
>
> Since we do not have a zero check anymore, the `Div/Mod` node could freely float above the loop-exit check `i > 1` and we crash with a division by zero on the last iteration because the increment node updated the trip-counter to zero and the loop-exit was not taken, yet.
>
> This was originally fixed by [JDK-8248552](https://bugs.openjdk.org/browse/JDK-8248552) for integer counted loops by bailing out of `split_thru_phi()` if a type of an input of an iv phi could be zero. However, there are two additional missing cases:
> - The same problem applies to long counted loop (triggers since [JDK-8256655](https://bugs.openjdk.org/browse/JDK-8256655) which allows iv phis of `LongCountedLoop` to be optimized).
> - The original fix for JDK-8248552 only checks if the phi to be split through is an iv phi by checking `counted_loop->phi()`. But we could have a chain `iv phi -> x -> Div/Mod` and already split x through the iv phi in the same split-if pass. In this case, we've created a new phi for merging the clones of `x` and we do not bail out anymore. This is fixed by simply checking if a phi has the counted loop node as control input.
>
> I've also improved the bail out to only check if the backedge type includes zero as the init type is always included in the type of the iv phi.
>
> Thanks,
> Christian
Thanks Vladimir for your review!
> I think this is only problematic for some architectures. Integer division instructions of some other architectures (some RISC architectures like PPC64) never raise any signals, so they can get executed speculatively without zero check. Should we have an extra RFE for that?
I think that might be worth to investigate in a separate RFE if these divisions by zero do not have any other side effects. Then I guess it should be safe to allow this as we would not use the result of the problematic division.
-------------
PR: https://git.openjdk.org/jdk/pull/11900
More information about the hotspot-compiler-dev
mailing list