RFR: 8296412: Special case infinite loops with unmerged backedges in IdealLoopTree::check_safepts [v2]

[Emanuel Peter]

> **Context**
> During parsing, we insert SafePoints if we jump from higher to lower bci (`maybe_add_safepoint` is called for every if, goto etc).
> https://github.com/openjdk/jdk/blob/8c472e481676ed0ef475c4989477d5714880c59e/src/hotspot/share/opto/parse.hpp#L490-L494
> Generally, this alligns with backedges: the assumption is that the loop-head sits at the smallest bci of all blocks in the loop. So every jump back to the loop-head goes from higher to lower bci, hence we place a SafePoint just before the jump.
> 
> Also: the first `build_loop_tree` may not attach an infinite loop to the loop-tree. If during the same loop-opts-phase we go to `beautify_loops` and it requires us rebuilding the loop-tree (eg because some other loop did `merge_many_backedges`), we call `build_loop_tree` again, and this time around we do detect the infinite loop (it now has a NeverBranch exit, so it is attached because of that).
> Afterwards, we call `IdealLoopTree::check_safepts`, which tries to find SafePoints on all backedges. Normally, we have SafePoints on all backedges, just before we go back to the head.
> 
> **Problem case**
> My jasm fuzzer produced some infinite loops that have the following form:
> The loop head is not at the smallest bci (bytecode index) of all blocks in the loop. So the SafePoints are placed somewhere in the body of the loop, just before an if branches into the two backedges. Because this is an infinite loop, it is only attached to the loop-tree in `build_loop_tree` after `beautify_loops`, so the two backedges were not merged.
> When we call `IdealLoopTree::check_safepts`, we start with the inner loop, where we find the SafePoint above the if. Then we go to the outer loop. We don't find a SafePoint before we find the inner body. Now we decide to skip the inner body (which implies skipping the SafePoint in the body). The code assumes after skipping the inner loop, we are still in the outer loop. This is not true, because inner and outer loop have the same loop head (the backedges were not merged). We trigger an assert that checks that we are still in the outer loop (`nested loop`).
> 
> Why did we not find this earlier?
> We have not extensively tested infinite loops before. Also, we have not tested loops with loop-heads that are not at the smallest bci of the loop. However, with my bytecode fuzzer I can find these issues. It is also more likely with irreducible loops: there at least one loop-entry cannot be at the smallest bci. Irreducible loops are not processed by `maybe_add_safepoint`, but once it only has a singe entry, it is not irreducible any more, and so it can happen that a loop-entry becomes loop head that does not have the smallest bci.
> 
> **Solution**
> We could fix `maybe_add_safepoint` to not depend on bci, but rather the loop-tree from `ciTypeFlow`. That would be complex, and risky. That is not justified just for infinite loops, and even infinite loops where the loop head is not at the lowest bci.
> 
> I decided to simply special case infinite loops. I detect if we have an outer loop with the same head as an inner loop. This should not happen, as we must have merged those backedges. Except if it is an infinite loop: We can break the scan, as we have already reached the loop's head.

Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:

  Christian's review suggestion

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/11706/files
  - new: https://git.openjdk.org/jdk/pull/11706/files/782c2cfa..2a02b338

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=11706&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11706&range=00-01

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

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