[jdk17] RFR: 8269795: C2: Out of bounds array load floats above its range check in loop peeling resulting in SEGV
Christian Hagedorn
chagedorn at openjdk.java.net
Thu Jul 8 20:34:28 UTC 2021
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 dominating 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
-------------
Commit messages:
- 8269795: C2: Out of bounds array load floats above its range check in loop peeling resulting in SEGV
Changes: https://git.openjdk.java.net/jdk17/pull/235/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=235&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8269795
Stats: 77 lines in 2 files changed: 68 ins; 2 del; 7 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
More information about the hotspot-compiler-dev
mailing list