[lworld] Integrated: 8348962: [lworld] compiler/arraycopy/TestObjectArrayClone.java fails with SIGSEGV in PhaseMacroExpand::expand_arraycopy_node

[Christian Hagedorn]

On Fri, 7 Feb 2025 16:25:36 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> When we call `clone()` on an array, we are calling the `Object::clone()` instrinsic with an array object receiver. For that, we call `inline_native_clone()`. When additionally running with `-XX:-ReduceInitialCardMarks`, `CardTableBarrierSetC2::array_copy_requires_gc_barriers()` will return true. As a result, we mark the newly created `ArrayCopyNode` for the cloning as `CloneOopArray`:
> 
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/src/hotspot/share/opto/library_call.cpp#L5631-L5632
> 
> The destination type of `ArrayCopyNode` is known to be some kind of array since we called `Object::clone()` on an array. Therefore, when expanding the node during macro expansion, the destination type will always be an array type and consequently, `top_dest` will be nun-null in `PhaseMacroExpand::expand_arraycopy_node()`: 
> 
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/src/hotspot/share/opto/macroArrayCopy.cpp#L1360
> 
> However, with reflection, we can still directly call `Object::clone()` on an array without C2 knowing that it's an array. This is done in the failing test when calling `TestObjectArrayClone::testCloneObjectWithMethodHandle()`:
> 
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/test/hotspot/jtreg/compiler/arraycopy/TestObjectArrayClone.java#L192-L194
> 
> `clone` is the `Object::clone()` method and `obj` is a `String[]` but passed in as an `Object` to the method. As a result, C2 just knows that the destination type is `Object`. Therefore, `top_dest` will be null here (i.e. not an array type):
> 
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/src/hotspot/share/opto/macroArrayCopy.cpp#L1360
> 
> Later, we check whether the destination is a flat array:
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/src/hotspot/share/opto/macroArrayCopy.cpp#L1382
> 
> Since `top_dest` is null, we crash with a segfault.
> 
> The solution is to just check whether `top_dest` is non-null. Note that in this case, we do not need a membar. When it turns out that the array behind `Object` is flat during runtime, we will go to the slow path and make a call to `clone()`:
> 
> https://github.com/openjdk/valhalla/blob/d42ac5c615e34a1d6ba20cfec5aa34316eafc374/src/hotspot/share/opto/library_call.cpp#L5611-L5613
> 
> I added the failing flag combination to the existing test which covers this bug.
> 
> Thanks,
> Christian

This pull request has now been integrated.

Changeset: d868876c
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/valhalla/commit/d868876ca10fc3f93a32164c378ecdb6c9488bbf
Stats:     14 lines in 2 files changed: 6 ins; 0 del; 8 mod

8348962: [lworld] compiler/arraycopy/TestObjectArrayClone.java fails with SIGSEGV in PhaseMacroExpand::expand_arraycopy_node

Reviewed-by: thartmann

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

PR: https://git.openjdk.org/valhalla/pull/1356