[lworld] Integrated: 8345250: [lworld] C2: Array loads and stores on inexact flat arrays cause crashes

[Christian Hagedorn]

On Fri, 6 Dec 2024 17:10:20 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This PR fixes crashes that occur when trying to load/store from/to a flat array from which we statically know it is flat but we do not know the exact layout because the klass is inexact.
> 
> ### The Layout of a Flat Array is only Statically Known If the Klass is Exact
> Currently, the code for getting the array element address in `GraphKit::array_element_address()` assumes that whenever we have an array from which we know that it is flat, then we also know the exact array layout (i.e. assume the value klass is exact):
> https://github.com/openjdk/valhalla/blob/5e876d607aba68e921cb8fa6a1a012acd1735825/src/hotspot/share/opto/graphKit.cpp#L1835-L1838
> 
> This does not need to be true as shown in the following test case where we have a flat array with an inexact klass/type:
> https://github.com/openjdk/valhalla/blob/95fc9c71bffcf5b92a4cdbfb34ad9e3149266f79/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java#L4568-L4578
> 
> ### Emit Runtime Call to Load/Store from/to the Correct Array Element Address
> What we actually would need for a flat array with an inexact klass is to emit a runtime call to load/store from/to the correct array element address. We cannot do better statically because we do not know the exact layout.
> 
> ### Challenges
> However, the problem is that in `GraphKit::array_element_address()` we somehow need to bail out back to `Parse::array_load/store()` such that we can emit the runtime call there. We could use a sentinel node because we do not actually need the element address node. But this could mess with some GVN transformations in between. What we do instead is to just fall to the `else` case in `GraphKit::array_element_address()` for non-flat arrays and still construct the element address node, even though it's not needed (will be folded away later):
> https://github.com/openjdk/valhalla/blob/0858ce110da58ed7f1b634ae381e7c4eb0f37c3a/src/hotspot/share/opto/graphKit.cpp#L1840-L1849
> 
> ### Additionally Required Fixes
> 1. `TypeAryKlassPtr::cast_to_exactness()` asserts that all null-free or flat arrays are exact which is not the case as shown above. Fix: Adjust assert.
> 2. `ciArrayKlass::make()` assumes that all null-free arrays are concrete value classes which does not need to be the case as shown above. Fix: Add check whether the klass is an inline type (which is only true for concrete inline type klasses).
> 3. `Parse::array_store_check()` needs to pass `null_free` to the `gen_checkcast()` in case we have a null-free flat array that is inexact (i.e. either ...

This pull request has now been integrated.

Changeset: c4f1714d
Author:    Christian Hagedorn <chagedorn at openjdk.org>
URL:       https://git.openjdk.org/valhalla/commit/c4f1714d21b61136a2451c1a0d1abe281c9cb8f8
Stats:     450 lines in 7 files changed: 292 ins; 62 del; 96 mod

8345250: [lworld] C2: Array loads and stores on inexact flat arrays cause crashes

Reviewed-by: thartmann

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

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