Integrated: 8323274: C2: array load may float above range check

[Roland Westrelin]

On Tue, 30 Jan 2024 16:50:08 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> This PR includes 5 test cases in which an array load floats above its
> range check and the resulting compiled code can be made to segfault by
> passing an out of bound index to the test method. Each test case takes
> advantage of a different transformation to make the array load happen
> too early:
> 
> For instance, with TestArrayAccessAboveRCAfterSplitIf:
> 
> 
> if (k == 2) {
>     v = array1[i];
>     array = array1;
>     if (l == m) {
>     }
> } else {
>     v = array2[i];
>     array = array2;
> }
> v += array[i]; // range check + array load
> 
> 
> The range check is split through phi:
> 
> 
> if (k == 2) {
>     v = array1[i];
>     array = array1;
>     if (l == m) {
>     }
>     // range check here
> } else {
>     v = array2[i];
>     array = array2;
>     // range check here
> }
> v += array[i]; // array load
> 
> 
> Then an identical dominating range check is found:
> 
> 
> if (k == 2) {
>     v = array1[i]; // range check here
>     array = array1;
>     if (l == m) {
>     }
> } else {
>     v = array2[i];  // range check here
>     array = array2;
> }
> v += array[i]; // array load
> 
> 
> Then a branch dies:
> 
> 
> v = array1[i]; // range check here
> array = array1;
> if (l == m) {
> }
> v += array[i]; // array load
> 
> 
> The array load is dependent on the `if (l == m) {` condition. An
> identical dominating condition is then found which causes the control
> dependent range check to float above the range check.
> 
> Something similar can be triggered with:
> 
> - TestArrayAccessAboveRCAfterPartialPeeling: sometimes, during partial
>   peeling a load is assigned the loop head as control so something
>   gets in between the range check and an array load and steps similar
>   to the above can cause the array load to float above its range check.
>   
> - TestArrayAccessAboveRCAfterUnswitching: cloning a loop body adds
>   regions on exits of the loop and nodes that only have uses out of
>   the loop can end up control dependent on one of the regions. In the
>   test case, unswitching is what causes the cloning to happen. Again
>   similar steps as above make the array load floats above its range
>   check. I suppose similar bugs could be triggered with other loop
>   transformations that rely on loop body cloning.
>   
> TestArrayAccessAboveRCAfterSinking is a bit different in that it can
> change the control of an array load to be the projection of some
> arbitrary test. That test can then be replaced by a dominating one
> causing the array to float.
> 
> Finally, in TestArrayAccessAboveRCForArrayCopyLoad, an array copy is
> converted to a series of loads/stores that's guarded by a test for
> `srcPos < dstPos...

This pull request has now been integrated.

Changeset: 4406915e
Author:    Roland Westrelin <roland at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/4406915ebce4266b3eb4a238382fff3c2c1d1739
Stats:     672 lines in 9 files changed: 668 ins; 0 del; 4 mod

8323274: C2: array load may float above range check

Reviewed-by: epeter, thartmann

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

PR: https://git.openjdk.org/jdk/pull/17635