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

[Richard Reingruber]

On Wed, 15 Feb 2023 21:12:19 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>>> I think your conclusion is correct wrt `unextended_sp < sp` in that case. For certain MH linker calls, an additional `MemberName` 'appendix' is passed as the last argument. The MH adapter/trampoline stub pops the appendix argument from the stack for an interpreted caller, and uses it to find out the actual callee. But that means that, the sp saved into `r13` will be one slot too 'large' (small? accounting for one too many arguments). In other words, the MethodHandle adapter for interpreted callers 'consumes' the appendix argument, but doesn't adjust `r13`. I think this could be done though, i.e. just add:
>>> 
>>> ```
>>>  __ addptr(r13, Interpreter::stackElementSize);   // adjust unexteded_sp for callee, for proper stack walking
>>> ```
>> 
>> I don't think you can do that. Modifying the unextended_sp would break accesses to compiled frames (gc, locals). The original unextended_sp is also needed to undo the c2i extension.
>> 
>>> In the linked code in `MethodHandles::generate_method_handle_interpreter_entry` to make this work.
>>> 
>>> (note that we can not just leave the appendix on the stack, since the callee doesn't expect it, so we would break argument oop GC if we left it there)
>>> 
>>> If that works, I think doing that would be preferable to adjusting `safe_for_sender`. That seems like it would uphold the `sp <= unextended_sp` invariant as well (maybe an assert to that effect in the constructor of `frame` wouldn't be a bad idea as well).
>>> 
>>> It seems like PPC is not modifying the stack in this case (?):
>>> 
>>> ```
>>>     Register R19_member = R19_method;  // MemberName ptr; incoming method ptr is dead now
>>>     __ ld(R19_member, RegisterOrConstant((intptr_t)8), R15_argbase);
>>>     __ addi(R15_argbase, R15_argbase, Interpreter::stackElementSize);
>>>     generate_method_handle_dispatch(_masm, iid, tmp_recv, R19_member, not_for_compiler_entry);
>>> ```
>> 
>> Only the expression stack pointer (R15) is modified but not the sp (R1).
>> 
>> unextended_sp < sp is possible on PPC because the expression stack is allocated with its maximum size and it's trimmed to the used part when making a call. The sender sp is the original sp.
>
>> I don't think you can do that. Modifying the unextended_sp would break accesses to compiled frames (gc, locals).
> 
> This is for interpreter entries, so the caller is definitely interpreted. If the callee is compiled, it shouldn't care about the contents of `r13` right? And if the callee is interpreted, it will get the correct `r13`.
> 
>>  The original unextended_sp is also needed to undo the c2i extension.
> 
> Can you explain this? We shouldn't have a c2i adapters in this case. since the caller is interpreted. ~If the callee is compiled, we will go through a c2i adapter for that, but in that case the adapter overwrites `r13` again.~

It's to long since I was working on low level MH stuff.
If an ordinary interpreter entry is used for a call this means the callee will be interpreted. The caller is not necessarily interpreted. It can be compiled.
Is this different here?

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

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