RFR: 8336042: Caller/callee param size mismatch in deoptimization causes crash

[Richard Reingruber]

On Tue, 11 Feb 2025 07:59:01 GMT, Dean Long <dlong at openjdk.org> wrote:

> When calling a MethodHandle linker, such as linkToStatic, we drop the last argument, which causes a mismatch between what the caller pushed and what the callee received.  In deoptimization, we check for this in several places, but in one place we had outdated code.  See the bug for the gory details.
> 
> In this PR I add asserts and a test to reproduce the problem, plus the necessary fixes in deoptimizations.  There are other inefficiencies in deoptimization that I didn't address, hoping to simplify the fix for backports.
> 
> Some platforms align locals according to the caller during deoptimization, while some align locals according to the callee.  The asserts I added compute locals both ways and check that they are still within the frame.  I attempted this on all platforms, but am only able to test x64 and aarch64.  I need help testing those asserts for arm32, ppc, riscv, and s390.

src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp line 136:

> 134:     // Test caller-aligned placement vs callee-aligned
> 135:     intptr_t* l2 = caller->sp() + method->max_locals() - 1 + (frame::java_abi_size / Interpreter::stackElementSize);
> 136:     assert(l2 >= locals_base, "bad placement");

The assertion at L136 fails on ppc64 (similar to what @offamitkumar reported for s390x).
I don't understand the assertion because it is just a stricter version of the fist one.
On ppc64 the sp of `caller` is aligned down because it needs to be 16 byte aligned. `locals_base` is only 8 byte aligned. But from what I saw the difference was larger then just one word. Maybe `caller` has got an c2i extension? I guess this would be problematic.
On x86_64 `l2` depends on the last expression stack pointer not on the `caller`'s sp. If you try to translate this to ppc64 then you'll get the expression used to initialize `locals_base` at L128.
I think you can remove the 2nd assertion. Even the first one looks redundant.
Besides that I've tested `MHDeoptTest.java` successfully on ppc64.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23557#discussion_r1952565563