[jdk17] RFR: 8269795: C2: Out of bounds array load floats above its range check in loop peeling resulting in SEGV [v2]

[Christian Hagedorn]

> In the testcase, an out of bounds array load (`LoadI`) floats above its range check resulting in a segfault. The problem can be traced back to incorrectly treating an `If` node, on which the `LoadI` is control dependent, as being dominated in `PhaseIdealLoop::peeled_dom_test_elim`. As a result, `PhaseIdealLoop::dominated_by()` moves the `LoadI` out and before the loop while the range check stays inside the loop.
> 
> `peeled_dom_test_elim()` walks through the idom chain of the loop body and tries to find loop-invariant tests. Once it finds one, it not only moves this test out of the loop but also looks for other tests with the same condition node which can be removed. The condition of the dominated tests in the loop are replaced with a `ConI` in `dominated_by()`. The data dependencies on the removed dominated tests are rewired to a peeled node before the loop.
> 
> However, the current code directly checks `test->in(1)` at L527 which is only the initial `Bool` node of the dominated test before the first invocation of `dominated_by()` in the loop on L525. Afterwards, `test->in(1)` is a `ConI` node. We now continue to look for tests with an identical `ConI` condition to move their data nodes out of this loop. This is wrong and exactly happens in the testcase: The loop body still contains a not yet cleaned up (by IGVN) `If` node (with a `ConI` condition) of an eliminated skeleton predicate on which an out of bounds  `LoadI` node is control dependent. The `LoadI` is then wrongly rewired out of the loop and before its range check.
> 
> The fix is to only check against the initial `test->in(1)` `Bool` node which satisfies the conditions checked on the lines before (L518-520).
> 
> Thanks,
> Christian

Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:

  review comments by Vladimir K

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

Changes:
  - all: https://git.openjdk.java.net/jdk17/pull/235/files
  - new: https://git.openjdk.java.net/jdk17/pull/235/files/c29fd247..bd921036

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=235&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=235&range=00-01

  Stats: 12 lines in 1 file changed: 6 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/235.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/235/head:pull/235

PR: https://git.openjdk.java.net/jdk17/pull/235