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

[Johannes Bechberger]

On Wed, 15 Feb 2023 20:24:16 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Yes.
>
>> I think `unextended_sp < sp` is possible on x86 when interpreter entries generated by `MethodHandles::generate_method_handle_interpreter_entry()` are executed. They modify `sp` ([here for example](https://github.com/openjdk/jdk/blob/7f71a1040d9c03f72d082e329ccaf2c4a3c060a6/src/hotspot/cpu/x86/methodHandles_x86.cpp#L310-L314)) but not `sender_sp` (`InterpreterMacroAssembler ::_bcp_register`).
> 
> I think we're talking about a case where an interpreted caller calls an interpreted callee through a MethodHandle. If my understanding is correct (looking at `frame::sender_for_interpreter_frame`), in that case there are 2 sender sps, one that is computed based on the frame pointer of the callee. This just adds 2 words (skipping the return address), and the other is the sp which is saved into `r13` before e.g. a c2i adapter extends the stack [here](https://github.com/openjdk/jdk/blob/50dcc2aec5b16c0826e27d58e49a7f55a5f5ad38/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L631) Which then seems to be saved in the interpreted callee's frame as well (I'm not super familiar with interpreter frame layouts though...). The latter being what ends up as `unextended_sp`.
> 
> 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
> 
> 
> 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);

This is an interesting insight, I'm going to test your fix idea tomorrow.

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

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