RFR: 8320308: C2 compilation crashes in LibraryCallKit::inline_unsafe_access [v6]

[Vladimir Kozlov]

On Wed, 25 Sep 2024 08:01:53 GMT, Tobias Holenstein <tholenstein at openjdk.org> wrote:

>> We failed in `LibraryCallKit::inline_unsafe_access()` while trying to inline `Unsafe::getShortUnaligned`. 
>> https://github.com/openjdk/jdk/blob/34c6e0deac567c0f4ed08aa2824671551d843e95/test/hotspot/jtreg/compiler/parsing/TestUnsafeArrayAccessWithNullBase.java#L86
>> The reason is that base (the array) is `ConP #null` hidden behind two `CheckCastPP` with `speculative=byte[int:>=0]`
>> 
>> We call `Node* adr = make_unsafe_address(base, offset, type, kind == Relaxed);`
>> https://github.com/openjdk/jdk/blob/34c6e0deac567c0f4ed08aa2824671551d843e95/src/hotspot/share/opto/library_call.cpp#L2361
>> - with **base** = `147  CheckCastPP` 
>> -  `118  ConP  === 0  [[[ 106 101 71 ] #null`
>> <img width="470" alt="type" src="https://github.com/user-attachments/assets/cbe5497e-cb0c-4f8e-a5d5-7a6ee1157778">
>> 
>> Depending on the **offset** we go two different paths in `LibraryCallKit::make_unsafe_address` which both lead to the same error in the end. 
>> 1. For `UNSAFE.getShortUnaligned(array, 1_049_000)` we get kind = `Type::AnyPtr` because `offset >= os::vm_page_size()`.  Since we assume base can't be null we insert an assert:
>> https://github.com/openjdk/jdk/blob/34c6e0deac567c0f4ed08aa2824671551d843e95/src/hotspot/share/opto/library_call.cpp#L2111
>> 
>> 2. whereas for `UNSAFE.getShortUnaligned(array, 1)` we get kind = `Type:: OopPtr`
>> https://github.com/openjdk/jdk/blob/c17fa910cf3bad48547a3f0d68a30795ec3194e6/src/hotspot/share/opto/library_call.cpp#L2078
>> and insert a null check
>> https://github.com/openjdk/jdk/blob/34c6e0deac567c0f4ed08aa2824671551d843e95/src/hotspot/share/opto/library_call.cpp#L2090
>> In both cases we return call `basic_plus_adr(..)` on a base being `top()` which returns **adr** =  `1  Con  === 0  [[ ]]  #top`
>> 
>> https://github.com/openjdk/jdk/blob/3d5d51e228c19aa216451f647023101ae8bdbc79/src/hotspot/share/opto/library_call.cpp#L2386 => `_gvn.type(adr)` is _top_
>> 
>> https://github.com/openjdk/jdk/blob/3d5d51e228c19aa216451f647023101ae8bdbc79/src/hotspot/share/opto/library_call.cpp#L2394 => `adr_type` is _nullptr_
>> 
>> https://github.com/openjdk/jdk/blob/3d5d51e228c19aa216451f647023101ae8bdbc79/src/hotspot/share/opto/library_call.cpp#L2405-L2406 => `BasicType bt` is  _T_ILLEGAL_
>> 
>> https://github.com/openjdk/jdk/blob/3d5d51e228c19aa216451f647023101ae8bdbc79/src/hotspot/share/opto/library_call.cpp#L2424 => we fail here with `SIGSEGV: null pointer dereference` because `alias_type->adr_type()` is _nullptr_
>> 
>> ### Fix (updated on 18th Sep 2024)
>> T...
>
> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test/hotspot/jtreg/compiler/parsing/TestUnsafeArrayAccessWithNullBase.java
>   
>   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>

src/hotspot/share/opto/library_call.cpp line 2367:

> 2365:   assert(!stopped(), "Inlining of unsafe access failed: address construction stopped unexpectedly");
> 2366: 
> 2367:   if (_gvn.type(base)->isa_ptr() == TypePtr::NULL_PTR) {

Why not `uncast` here?

test/hotspot/jtreg/compiler/parsing/TestUnsafeArrayAccessWithNullBase.java line 34:

> 32:  *      -XX:CompileCommand=compileonly,compiler.parsing.TestUnsafeArrayAccessWithNullBase::test*
> 33:  *      -XX:-TieredCompilation compiler.parsing.TestUnsafeArrayAccessWithNullBase
> 34:  * @run main/othervm compiler.parsing.TestUnsafeArrayAccessWithNullBase

You don't need `/othervm` if no VM's flags are specified.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20033#discussion_r1775637977
PR Review Comment: https://git.openjdk.org/jdk/pull/20033#discussion_r1775636351