RFR: 8323274: C2: array load may float above range check [v6]

Roland Westrelin roland at openjdk.org
Fri Feb 9 14:01:17 UTC 2024


> 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...

Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:

  fix broken try_sink_out_of_loop

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/17635/files
  - new: https://git.openjdk.org/jdk/pull/17635/files/dc4895f7..e84ac6b3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17635&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17635&range=04-05

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17635.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17635/head:pull/17635

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


More information about the hotspot-compiler-dev mailing list