RFR: JDK-8302320: AsyncGetCallTrace obtains too few frames in sanity test [v2]

[Johannes Bechberger]

On Thu, 16 Feb 2023 11:00:14 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>>> (Maybe too) long version:
>> 
>> Not at all :) I don't know much about the interpreter, so this is pretty helpful.
>> 
>> Ok. I think the main point is that a MH linker method doesn't ~have~ use a c2i adapter, it instead has separate versions for interpreted and compiled callers altogether (except `linkToNative` which doesn't have an interpreter version atm)
>> 
>> Longer version:
>> 
>> I think there are 4 theoretical cases (are there more?):
>> 1. interpreted -> MH interpreter entry -> interpreted (the case we want to fix, I think)
>> 2.  interpreted -> MH interpreter entry -> i2c -> compiled
>> 3. compiled -> c2i -> MH interpreter entry -> interpreted
>> 4. compiled -> c2i -> MH interpreter entry -> i2c -> compiled
>> 
>> So... my understanding is that a c2i adapter is used when a callee is interpreted, so its `from_compiled_entry` points to the c2i adapter. A compiled caller calls the `from_compiled_entry`, so it ends up going through the `c2i` adapter of an interpreted callee. However, for method handle linkers, the `from_compiled_entry` never points to a c2i adapter, but to the MH linker compiler entry generated in `gen_special_dispatch` in e.g `sharedRuntime_x86_64`. So, in other words, the 3. and 4. scenarios above are not possible, AFAICS. An MH linker's interpreted/compiled entries simply replace the regular `i2c` and `c2i` adapters for a MH linker method. Though, the actual target method can be either compiled or interpreted, so we can go into either a `c2i` adapter from a MH linker  compiler entry, or a `i2c` adapter from a MH linker interpreter entry (which is scenario 2.).
>> 
>> For scenario 1.: caller sets `r13` before jumping to callee (MH linker), we end up in the MH adapter, pop the last argument, and adjust `r13` for the actual callee. The MH interpreter entry uses `from_interpreted_entry` of the target method, so we don't go through any adapter.
>> 
>> For scenario 2.: mostly the same as 1., but we go through an `i2c` adapter, and the compiled callee doesn't care about what's in `r13` any ways. (there's a comment in the ic2 adapter code about preserving `r13` for the case that we might go back into a c2i adapter if we lose a race where the callee is made non-entrant, but `r13` is also used as a scratch register later in the i2c code, and the c2i code overwrites `r13` any ways. So, I think that is an outdated comment...)
>
>> So... my understanding is that a c2i adapter is used when a callee is
>> interpreted, so its `from_compiled_entry` points to the c2i adapter. A
>> compiled caller calls the `from_compiled_entry`, so it ends up going through
>> the `c2i` adapter of an interpreted callee. However, for method handle
>> linkers, the `from_compiled_entry` never points to a c2i adapter, but to the
>> MH linker compiler entry generated in `gen_special_dispatch` in e.g
>> `sharedRuntime_x86_64`. So, in other words, the 3. and 4. scenarios above are
>> not possible, AFAICS.
> 
> I see. Thanks! That's what I've been missing. It's implemented in `SystemDictionary::find_method_handle_intrinsic()` I think.
> 
> I agree that it will be ok to modify r13 as you suggested. Maybe explaining why we never reach here coming from a c2i adapter: because native wrappers are generated eagerly in `SystemDictionary::find_method_handle_intrinsic()` unless -Xint is given but then the caller cannot be compiled.

I integrated your suggestion, it seems to work now. I'm hopeful that our internal queue does not find a test error.

I'm happy for any commenting suggestions.

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

PR: https://git.openjdk.org/jdk/pull/12535