RFR: 8331717: C2: Crash with SIGFPE Because Loop Predication Wrongly Hoists Division Requiring Zero Check

[Vladimir Kozlov]

On Tue, 10 Dec 2024 16:09:26 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

> Fixes a bug in loop predication where not strictly invariant tests involving divisions or modulo are pulled out of the loop.
> 
> The bug can be seen in this code:
> 
> 
> public class Reduced {
>     static int iArr[] = new int[100];
> 
>     public static void main(String[] strArr) {
>         for (int i = 0; i < 10000; i++) {
>             test();
>         }
>     }
> 
>     static void test() {
>         int i1 = 0;
> 
>         for (int i4 : iArr) {
>             i4 = i1;
>             try {
>                 iArr[0] = 1 / i4;
>                 i4 = iArr[2 / i4]; // Source of the crash
>             } catch (ArithmeticException a_e) {
>             }
>         }
>     }
> }
> 
> 
> The crucial element is the division `2 / i4`. Since it is used to access an array, it is the input to a range check. See node 230:
> <img width="699" alt="Screenshot 2024-12-11 at 15 14 47" src="https://github.com/user-attachments/assets/0b2ed978-7135-4a7e-bd10-25c0ffe7a9bb" />
> 
> Loop predication will try to pull this range check together with its input, the division, before the `for` loop. Due to a bug in Invariance::compute_invariance loop predication is allowed to do so, which results in the division being pulled out without its non-zero check. 322 is a clone of 230 placed before the loop head without any zero check for the divisor:
> 
> <img width="798" alt="Screenshot 2024-12-11 at 15 11 48" src="https://github.com/user-attachments/assets/9d4911cc-9967-4b7a-9969-98e01a55cd0d" />
> 
> To fix this, Invariance::compute_invariance must check that the node not only `depends_only_on_test()` but also that it has `no_dependent_zero_check(n)`.
> 
> Similar past bug, which introduced `no_dependent_zero_check`: https://github.com/openjdk/jdk16/pull/9

Looks good for this issue.

Would be interesting if we can collapse such graph by propagating `I1 == 0` through `i4` into zero check.
As separate RFE.

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

Marked as reviewed by kvn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22666#pullrequestreview-2496601515
PR Comment: https://git.openjdk.org/jdk/pull/22666#issuecomment-2536907055