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

[Christian Hagedorn]

On Wed, 11 Dec 2024 13:16:49 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 ...
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review comments

Thanks a lot for your review!

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

PR Review: https://git.openjdk.org/valhalla/pull/1314#pullrequestreview-2495629480