On Tue, 24 Mar 2026 12:29:15 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
> `Parse::do_call` and
> detect the broken phi and call `InlineTypeNode::make_from_oop` on the phi
> input...
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
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2256#discussion_r3010097825
