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
inputs that are oops.
This seems a bit hacky, as we allow the broken phi to exist for a while, and
it's hard to know
if we could be missing some cases.
The second option would be to fill in the blanks and add an
`InlineTypeNode::make_from_oop` call
in the call generator, either in `VirtualCallGenerator` or
`PredictedCallGenerator`. I think this
is also hacky, we would need to call `InlineTypeNode::make_from_oop` at several
places.
The third option is to have the `make_from_oop` call closer to the creation of
the return node. This ensures that we do not miss any case
and that we only have to do it in one place. I think
`GraphKit::set_results_for_java_call` fits
these requirements, and there is already some inline type related stuff going
on there. Unfortunately, I noticed after some preliminary testing that we still
have to keep the the `make_from_oop` call in `do_call`, as there are cases
where we rely on it to create an `InlineTypeNode` for the inlined path.
I have also added a check to ensure we never merge a non-buffered `InlineType`
with an oop in `PredictedCallGenerator`.
### Testing
- [x] GitHub Actions
- [x] tier1-4, plus some internal testing
Thank you for reviewing!
-------------
Commit messages:
- Minor changes in test
- Move assert and add ifdef assert
- details
- Add assert
- Add test
- Call make_from_oop in GraphKit::set_results_for_java_call
Changes: https://git.openjdk.org/valhalla/pull/2256/files
Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2256&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8380434
Stats: 90 lines in 3 files changed: 90 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/valhalla/pull/2256.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2256/head:pull/2256
PR: https://git.openjdk.org/valhalla/pull/2256
