On Mon, 30 Mar 2026 14:13:21 GMT, Tobias Hartmann <[email protected]> wrote:

>> This PR prevents a wrong execution / VM segfault resulting from a wrong use
>> of `InlineTypeNode::make_from_oop` when generating a slow and fast path with
>> `PredictedCallGenerator`.
>> 
>> This was initially spotted as a failure with 
>> `compiler/valhalla/inlinetypes/TestUnloadedReturnTypes.java`.
>> 
>> ## Bug Summary
>> 
>> We have an OSR compilation, which matters because `a` is initialized outside 
>> of the loop and thus its type won't be exact.
>> 
>> 
>> A a = new A();
>> 
>> // We want an OSR compilation here
>> for (int i = 0; i < 100_000; i++) {
>>     Asserts.assertEquals(a.virtual(true), new MyValueRetType());
>> }
>> 
>> 
>> We then try to predict the virtual call and generate both fast (inlined) and 
>> slow (dynamic call)
>> paths.
>> 
>> Another important detail here is that the result from the dynamic call is 
>> not scalarized, as it seems there are not enough registers available in this 
>> case (cf `TypeFunc::returns_inline_type_as_fields`).
>> 
>> This means that we end up with a `Phi` that has on one side the result of 
>> the inlined path,
>> which is a non-buffered `InlineTypeNode`, and on the other a `Proj` for the 
>> oop resulting from the call.
>> 
>> (the method and type names match the initial reproducer here)
>> <img width="1746" height="375" alt="image" 
>> src="https://github.com/user-attachments/assets/c895598e-66ea-499e-bb37-e783f835b841";
>>  />
>> 
>> This is wrong already, because the assumption here is that the both branches 
>> contain an oop.
>> In `Parse::do_call`, we end up looking at the return node produced by the 
>> call generator
>> and determine that we should build an `InlineTypeNode` from the oop:
>> 
>> https://github.com/openjdk/valhalla/blob/1257fbdfb3db4bde784c8dab830f2220c2b13544/src/hotspot/share/opto/doCall.cpp#L812-L820
>> 
>> The graph is very clearly broken at this stage, and this leads to wrong 
>> executions or even a segfault
>> as soon as we take the fast path (which always happens here) because the oop 
>> input for the `InlineTypeNode` is effectively null as we are trying to get 
>> rid of the allocation.
>> 
>> ## Fix
>> 
>> In `Parse::do_call`, the assumption when dealing with an inline type as 
>> return value is that we either:
>> - get an `InlineTypeNode` directly (inlining case) and we don't have 
>> anything to do
>> - get an oop, and then we can safely call `InlineTypeNode::make_from_oop`
>> 
>> Here we have something in the middle, and clearly we have to be more 
>> careful. I think there are
>> several options:
>> 
>> The first option would be to slightly modify the existing behavior in `P...
>
> src/hotspot/share/opto/callGenerator.cpp line 1126:
> 
>> 1124:     Node* n = slow_map->in(i);
>> 1125:     if (m != n) {
>> 1126: #ifdef assert
> 
> Suggestion:
> 
> #ifdef ASSERT

Interesting, that this builds šŸ˜„

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2256#discussion_r3010103586

Reply via email to