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

[Jorn Vernee]

On Wed, 15 Feb 2023 09:01:29 GMT, Johannes Bechberger <duke at openjdk.org> wrote:

>> Are you referring to the test in test/hotspot/jtreg/serviceability/AsyncGetCallTrace/ ?
>
> 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, 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...).

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. 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:


 __ subptr(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)

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);

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

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