On Wed, 15 Feb 2023 20:24:16 GMT, Jorn Vernee <jver...@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
