RFR: 8297730: C2: Arraycopy intrinsic throws incorrect exception

[Christian Hagedorn]

In `LibraryCallKit::inline_arraycopy()`, we emit some guards to safely perform `System.arraycopy()`. These guards are added to the IR right before the `ArrayCopy` node. If one of the emitted guards fails at runtime, we need to deoptimize and jump back to the interpreter.

If there is an additional tightly coupled allocation (array copy right after an array allocation), then the array copy can perform the initialization of the array instead of the standard array initialization. To make this work, the traps for the emitted guards for the array copy need to re-execute in the interpreter at the bci for the array allocation (fixed by [JDK-8064703](https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2014-December/016404.html)). Otherwise, the interpreter calls array copy on an uninitialized array and would crash.

This fix was not sufficient, though, as we now allocate the array twice. This leaves an uninitialized array in the heap which could cause problems for GCs. This was than fixed by [JDK-8073866](https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2015-March/017334.html) by moving the entire array allocation from before the guards to after the guards. This is possible due to the pattern that tightly coupled allocations have to obey (only other unrelated tests with UCTs, no unexpected observers of the allocation etc.).

As it turns out, this fix is still not complete. In this bug here, we've observed that the current implementation leads to a situation where we throw a wrong exception for a tightly coupled allocation that was initialized with a negative size. The reason for that is the following:

In `tightly_coupled_allocation()`, we allow tests with uncommon traps between the allocation and the array copy which we consider harmless:
https://github.com/openjdk/jdk/blob/c6d560039682ec52efa6fa7755d2aa86f20e1148/src/hotspot/share/opto/library_call.cpp#L5347-L5362

We could therefore still identify a tightly allocated allocation in the following code where we have an additional unrelated null check trap between the array allocation and the array copy:

byte[] b = new byte[-1];
int x = byArr.length; // with null_check trap
// <guards for arraycopy>
System.arraycopy(byArr, 0, b, 0, x);

We perform the optimization to use the array copy to initialize the array and move the allocation down. This gives us:

int x = byArr.length; // with null_check trap
// <guards for arraycopy>
byte[] b = new byte[-1];
System.arraycopy(arg1, 0, b, 0, x);

If `byArr` is now `null` at runtime, we take the `null_check` trap when executing `byArr.length` and jump to the interpreter. We re-execute this access and throw a `NullPointerException`. But this is wrong: We actually should have thrown a `NegativeArraySizeException` instead because the array allocation with the negative size should have been executed first. But we do not since we've moved the allocation down to the array copy.

To solve this, I propose to replace all "unrelated uncommon traps" between the array allocation and the array copy (or more precisely the emitted guards) with uncommon traps that have the same state as the dedicated uncommon trap of the emitted array copy guards (i.e. the state before the array allocation). However, we need to be more careful and cannot simply reuse the same trap as we could end up with dominance problems: The dedicated emitted guard trap could use improved nodes dependent on the true projection of some guards. But the unrelated uncommon traps come before these guards traps. Additionally, we need to make sure to use the original deoptimization reason and action of the unrelated uncommon traps.

I've refactored some code to make the implementation easier:
- reused the safepoint creation code for the state before the array allocation for the uncommon trap of the emitted guards
- refactored and reused code to skip over harmless tests with uncommon traps in `tightly_allocated_allocation()`
- some renamings to make the code easier to read

Thanks,
Christian

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

Commit messages:
 - 8297730: C2: Arraycopy intrinsic throws incorrect exception

Changes: https://git.openjdk.org/jdk/pull/12120/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12120&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8297730
  Stats: 470 lines in 3 files changed: 414 ins; 32 del; 24 mod
  Patch: https://git.openjdk.org/jdk/pull/12120.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12120/head:pull/12120

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