RFR: 8296389: C2: PhaseCFG::convert_NeverBranch_to_Goto must handle both orders of successors
Emanuel Peter
epeter at openjdk.org
Fri Dec 2 13:55:14 UTC 2022
**Targetted for JDK21**, since this is not a new regression, but rather an old bug. P3 because creates `SIGSEGV` in product build.
The code in `PhaseCFG::convert_NeverBranch_to_Goto` looks like it is ready to have `idx == 1`, but it is not.
We would read `succ` from `_succs[1]`.
https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/block.cpp#L626
Then overwrite `_succs[0]` with `succ`, and shorten the array.
https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/block.cpp#L635-L636
And finally attempt to read `dead` from `_succs[0]`, where the dead block used to be, but was just overwritten.
https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/block.cpp#L645
**Solution**
Read `dead` before overwriting it. I also made it more robust by going via the projections, and not assuming that the projections and successors are ordered equally (though that is probably guaranteed by the matching traversal).
**Why did we never hit this bug before?**
Normal case: during matching, "succ" projection is added as output of NeverBranch before the "dead" projection leading to Halt. Thus, the outputs of NeverBranch are normally [[ "succ", "dead" ]], hence `idx == 0`.
Details: During DFS, usually we go from Halt to NeverBranch. Then via Region/Loop, take backedge, and find the "succ" edge. We already have its inputs (NeverBranch), thus we can now post-visit the live edge, and attach it to the NeverBranch first. Later, once we have processed the whole infinite loop, we post-visit out of NeverBranch to the "dead" projection edge, which we attach second.
Rare case: "dead" projection is first attached to NeverBranch, and "succ" projection is added second. We have [[ "dead", "succ" ]], hence `idx == 1`.
We have a peeled infinite loop. The NeverBranch of the peeled iteration is first visited via the "dead" projection from HaltNode. Since the peeled iteration has no backedge, we do not visit the "succ" projection yet, but instead attach "dead" projection to HaltNode already once we are done visiting everything above. Later, we come from the peeled loop's NeverBranch exit, to the "succ" projection of the peeled iteration's NeverBranch, and attach the "succ" projection.

-------------
Commit messages:
- replace tabs with spaces
- 8296389: C2: PhaseCFG::convert_NeverBranch_to_Goto must handle both orders of successors
Changes: https://git.openjdk.org/jdk/pull/11481/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11481&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8296389
Stats: 171 lines in 3 files changed: 167 ins; 1 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/11481.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/11481/head:pull/11481
PR: https://git.openjdk.org/jdk/pull/11481
More information about the hotspot-compiler-dev
mailing list