On Mon, 30 Mar 2026 16:57:26 GMT, Benoît Maillard <[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...
>
> Benoît Maillard has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Update src/hotspot/share/opto/callGenerator.cpp
>
> Co-authored-by: Tobias Hartmann <[email protected]>
Looks good to me but I agree with Quan Anh that a comment should be added.
-------------
Marked as reviewed by thartmann (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/2256#pullrequestreview-4035753027
