On Wed, 15 Feb 2023 09:01:29 GMT, Johannes Bechberger <d...@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

Reply via email to