[jdk17] RFR: 8270461: ZGC: Invalid oop passed to ZBarrierSetRuntime::load_barrier_on_oop_array

[Vladimir Kozlov]

On Fri, 16 Jul 2021 13:59:02 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> For object arrays, C2's clone intrinsic emits calls to the `oop_disjoint_arraycopy_uninit` stub. With ZGC, load barriers on the source array elements are applied via `BarrierSetAssembler::arraycopy_prologue` before copying to the destination array: https://github.com/openjdk/jdk17/blob/1e3b418a53a080a53827989393362338b43dd363/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp#L2400-L2403
> 
> The problem is that `BarrierSetC2::arraycopy_payload_base_offset` may 8-byte align the array offset to ensure that a copy in 8-byte chunks of the 8-byte aligned object is possible: https://github.com/openjdk/jdk17/blob/1e3b418a53a080a53827989393362338b43dd363/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp#L658-L662
> Now with ` -XX:-UseCompressedClassPointers`, the offset starts at the 4-byte length field of the array (8 bytes mark word + 8 byte klass = 16 byte). That is fine if we don't need load barriers and can copy the array as `T_LONG` but with ZGC we crash in `ZBarrier::mark` because the element is not a valid oop.
> 
> I propose to simply set the offset to the first array element when cloning Object arrays with ZGC. We can still copy in 8 byte chunks because the oop elements are 8 byte on 64-bit (and ZGC is only supported on 64-bit).
> 
> I found this when investigating intermittent ZGC crashes in project Valhalla. The bug was introduced by [JDK-8268125](https://bugs.openjdk.java.net/browse/JDK-8268125) in JDK 17. The code is quite messy and will hopefully be cleaned up by [JDK-8268020](https://bugs.openjdk.java.net/browse/JDK-8268020).
> 
> Thanks,
> Tobias

Should we fix `arraycopy_payload_base_offset()` instead? The placement of  the fix looks strange: `src_offset` and `dest_offset` are some input nodes from `ArrayCopyNode` but you replacing them with one constant.
The fix is valid because the code is guarded by `ac->is_clone_array()` check. But I think `src_offset` and `dest_offset` should be already corrected when `ZBarrierSetC2::clone_at_expansion()` is called.

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

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