diff --git a/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp b/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp index a3c729fdd56..b543c96f3b8 100644 --- a/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/abstractInterpreter_aarch64.cpp @@ -149,7 +149,8 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + assert(locals <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(locals >= interpreter_frame->sender_sp() + max_locals - 1, "bad placement"); } #endif diff --git a/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp b/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp index 075db4736f1..978011491a0 100644 --- a/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp +++ b/src/hotspot/cpu/arm/abstractInterpreter_arm.cpp @@ -131,6 +131,15 @@ void AbstractInterpreter::layout_activation(Method* method, intptr_t* locals = interpreter_frame->sender_sp() + max_locals - 1; +#ifdef ASSERT + if (caller->is_interpreted_frame()) { + // Test exact placement on top of caller args + intptr_t* l2 = caller->interpreter_frame_last_sp() + caller_actual_parameters - 1; + assert(l2 <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(l2 >= locals, "bad placement"); + } +#endif + interpreter_frame->interpreter_frame_set_locals(locals); BasicObjectLock* montop = interpreter_frame->interpreter_frame_monitor_begin(); BasicObjectLock* monbot = montop - moncount; diff --git a/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp b/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp index cc094ad4f99..beadce33637 100644 --- a/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp +++ b/src/hotspot/cpu/ppc/abstractInterpreter_ppc.cpp @@ -128,6 +128,15 @@ void AbstractInterpreter::layout_activation(Method* method, caller->interpreter_frame_esp() + caller_actual_parameters : caller->sp() + method->max_locals() - 1 + (frame::java_abi_size / Interpreter::stackElementSize); +#ifdef ASSERT + if (caller->is_interpreted_frame()) { + assert(locals_base <= caller->interpreter_frame_expression_stack(), "bad placement"); + const int caller_abi_bytesize = (is_bottom_frame ? frame::top_ijava_frame_abi_size : frame::parent_ijava_frame_abi_size); + intptr_t* l2 = caller->sp() + method->max_locals() - 1 + (caller_abi_bytesize / Interpreter::stackElementSize); + assert(locals_base >= l2, "bad placement"); + } +#endif + intptr_t* monitor_base = caller->sp() - frame::ijava_state_size / Interpreter::stackElementSize; intptr_t* monitor = monitor_base - (moncount * frame::interpreter_frame_monitor_size()); intptr_t* esp_base = monitor - 1; diff --git a/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp b/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp index 843a58e28d7..00a6877684a 100644 --- a/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp +++ b/src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp @@ -141,7 +141,8 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + assert(locals <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(locals >= interpreter_frame->sender_sp() + max_locals - 1, "bad placement"); } #endif diff --git a/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp b/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp index 37aa9d9a0e8..e815542a51e 100644 --- a/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp +++ b/src/hotspot/cpu/s390/abstractInterpreter_s390.cpp @@ -182,6 +182,13 @@ void AbstractInterpreter::layout_activation(Method* method, intptr_t* sender_sp; if (caller->is_interpreted_frame()) { sender_sp = caller->interpreter_frame_top_frame_sp(); +#ifdef ASSERT + assert(locals_base <= caller->interpreter_frame_expression_stack(), "bad placement"); + // Test caller-aligned placement vs callee-aligned + intptr_t* l2 = (caller->sp() + method->max_locals() - 1 + + frame::z_parent_ijava_frame_abi_size / Interpreter::stackElementSize); + assert(locals_base >= l2, "bad placement"); +#endif } else if (caller->is_compiled_frame()) { sender_sp = caller->fp() - caller->cb()->frame_size(); // The bottom frame's sender_sp is its caller's unextended_sp. diff --git a/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp b/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp index 68ac5b6ca9a..6680b8c4c03 100644 --- a/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp +++ b/src/hotspot/cpu/x86/abstractInterpreter_x86.cpp @@ -87,7 +87,10 @@ void AbstractInterpreter::layout_activation(Method* method, #ifdef ASSERT if (caller->is_interpreted_frame()) { - assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement"); + // Test exact placement on top of caller args + intptr_t* l2 = caller->interpreter_frame_last_sp() + caller_actual_parameters - 1; + assert(l2 <= caller->interpreter_frame_expression_stack(), "bad placement"); + assert(l2 >= locals, "bad placement"); } #endif diff --git a/src/hotspot/share/interpreter/bytecode.hpp b/src/hotspot/share/interpreter/bytecode.hpp index 870fcb7784e..de83cfe5d71 100644 --- a/src/hotspot/share/interpreter/bytecode.hpp +++ b/src/hotspot/share/interpreter/bytecode.hpp @@ -226,6 +226,8 @@ class Bytecode_invoke: public Bytecode_member_ref { bool has_appendix(); + bool has_member_arg() const; + int size_of_parameters() const; private: diff --git a/src/hotspot/share/interpreter/bytecode.inline.hpp b/src/hotspot/share/interpreter/bytecode.inline.hpp index 43e0cf2991d..139f477d397 100644 --- a/src/hotspot/share/interpreter/bytecode.inline.hpp +++ b/src/hotspot/share/interpreter/bytecode.inline.hpp @@ -28,6 +28,7 @@ #include "interpreter/bytecode.hpp" #include "oops/cpCache.inline.hpp" +#include "prims/methodHandles.hpp" inline bool Bytecode_invoke::has_appendix() { if (invoke_code() == Bytecodes::_invokedynamic) { @@ -37,4 +38,13 @@ inline bool Bytecode_invoke::has_appendix() { } } +inline bool Bytecode_invoke::has_member_arg() const { + // NOTE: We could resolve the call and use the resolved adapter method here, but this function + // is used by deoptimization, where resolving could lead to problems, so we avoid that here + // by doing things symbolically. + // + // invokedynamic instructions don't have a class but obviously don't have a MemberName appendix. + return !is_invokedynamic() && MethodHandles::has_member_arg(klass(), name()); +} + #endif // SHARE_INTERPRETER_BYTECODE_INLINE_HPP diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 4a51e2cbd92..167b3095fa9 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -36,6 +36,7 @@ #include "gc/shared/collectedHeap.hpp" #include "gc/shared/memAllocator.hpp" #include "interpreter/bytecode.hpp" +#include "interpreter/bytecode.inline.hpp" #include "interpreter/bytecodeStream.hpp" #include "interpreter/interpreter.hpp" #include "interpreter/oopMapCache.hpp" @@ -641,11 +642,12 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread bool caller_was_method_handle = false; if (deopt_sender.is_interpreted_frame()) { methodHandle method(current, deopt_sender.interpreter_frame_method()); - Bytecode_invoke cur = Bytecode_invoke_check(method, deopt_sender.interpreter_frame_bci()); - if (cur.is_invokedynamic() || cur.is_invokehandle()) { - // Method handle invokes may involve fairly arbitrary chains of - // calls so it's impossible to know how much actual space the - // caller has for locals. + Bytecode_invoke cur(method, deopt_sender.interpreter_frame_bci()); + if (cur.has_member_arg()) { + // This should cover all real-world cases. One exception is a pathological chain of + // MH.linkToXXX() linker calls, which only trusted code could do anyway. To handle that case, we + // would need to get the size from the resolved method entry. Another exception would + // be an invokedynamic with an adapter that is really a MethodHandle linker. caller_was_method_handle = true; } } @@ -748,9 +750,14 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread } #endif + int caller_actual_parameters = -1; // value not used except for interpreted frames, see below + if (deopt_sender.is_interpreted_frame()) { + caller_actual_parameters = callee_parameters + (caller_was_method_handle ? 1 : 0); + } + UnrollBlock* info = new UnrollBlock(array->frame_size() * BytesPerWord, caller_adjustment * BytesPerWord, - caller_was_method_handle ? 0 : callee_parameters, + caller_actual_parameters, number_of_frames, frame_sizes, frame_pcs, @@ -939,7 +946,7 @@ JRT_LEAF(BasicType, Deoptimization::unpack_frames(JavaThread* thread, int exec_m if (Bytecodes::is_invoke(cur_code)) { Bytecode_invoke invoke(mh, iframe->interpreter_frame_bci()); cur_invoke_parameter_size = invoke.size_of_parameters(); - if (i != 0 && !invoke.is_invokedynamic() && MethodHandles::has_member_arg(invoke.klass(), invoke.name())) { + if (i != 0 && invoke.has_member_arg()) { callee_size_of_parameters++; } } diff --git a/src/hotspot/share/runtime/vframeArray.cpp b/src/hotspot/share/runtime/vframeArray.cpp index 1640b08c479..e17961fc424 100644 --- a/src/hotspot/share/runtime/vframeArray.cpp +++ b/src/hotspot/share/runtime/vframeArray.cpp @@ -25,6 +25,7 @@ #include "classfile/vmSymbols.hpp" #include "code/vmreg.inline.hpp" #include "interpreter/bytecode.hpp" +#include "interpreter/bytecode.inline.hpp" #include "interpreter/interpreter.hpp" #include "memory/allocation.inline.hpp" #include "memory/resourceArea.hpp" @@ -610,10 +611,7 @@ void vframeArray::unpack_to_stack(frame &unpack_frame, int exec_mode, int caller methodHandle caller(current, elem->method()); methodHandle callee(current, element(index - 1)->method()); Bytecode_invoke inv(caller, elem->bci()); - // invokedynamic instructions don't have a class but obviously don't have a MemberName appendix. - // NOTE: Use machinery here that avoids resolving of any kind. - const bool has_member_arg = - !inv.is_invokedynamic() && MethodHandles::has_member_arg(inv.klass(), inv.name()); + const bool has_member_arg = inv.has_member_arg(); callee_parameters = callee->size_of_parameters() + (has_member_arg ? 1 : 0); callee_locals = callee->max_locals(); } diff --git a/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java b/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java new file mode 100644 index 00000000000..61dd330cfdd --- /dev/null +++ b/test/hotspot/jtreg/compiler/jsr292/MHDeoptTest.java @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package compiler.jsr292; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodHandles.Lookup; +import java.lang.invoke.MethodType; +import java.lang.reflect.Method; + +/* + * @test + * @bug 8336042 + * @library /test/lib / + * + * @run main/bootclasspath/othervm -Xbatch -XX:-TieredCompilation compiler.jsr292.MHDeoptTest + * + */ +public class MHDeoptTest { + + static int xx = 0; + + public static void main(String[] args) throws Throwable { + MethodHandle mh1 = MethodHandles.lookup().findStatic(MHDeoptTest.class, "body1", MethodType.methodType(int.class)); + MethodHandle mh2 = MethodHandles.lookup().findStatic(MHDeoptTest.class, "body2", MethodType.methodType(int.class)); + MethodHandle[] arr = new MethodHandle[] {mh2, mh1}; + + for (MethodHandle mh : arr) { + for (int i = 1; i < 50_000; i++) { + xx = i; + mainLink(mh); + } + } + + } + + static int mainLink(MethodHandle mh) throws Throwable { + return (int)mh.invokeExact(); + } + + static int cnt = 1000; + + static int body1() { + int limit = 0x7fff; + // uncommon trap + if (xx == limit) { + // OSR + for (int i = 0; i < 50_000; i++) { + } + ++cnt; + ++xx; + } + if (xx == limit + 1) { + return cnt + 1; + } + return cnt; + } + + static int body2() { + int limit = 0x7fff; + int dummy = 0; + // uncommon trap + if (xx == limit) { + // OSR + for (int i = 0; i < 50_000; i++) { + } + ++cnt; + ++xx; + } + if (xx == limit + 1) { + return cnt + 1; + } + return cnt; + } + +}