# HG changeset patch # User roland # Date 1366796978 -7200 # Node ID 603ca7e5135452658a9f8324e5b88332c93b42be # Parent c07dd9be16e8cc83d4079576cafdbd817b4b52cc 8010460: Interpreter on some platforms loads ConstMethod::_max_stack and misses extra stack slots for JSR 292 Summary: ConstMethod::max_stack() doesn't account for JSR 292 appendix. Reviewed-by: kvn diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/sparc/vm/cppInterpreter_sparc.cpp --- a/src/cpu/sparc/vm/cppInterpreter_sparc.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/sparc/vm/cppInterpreter_sparc.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -1065,7 +1065,7 @@ const int slop_factor = 2*wordSize; const int fixed_size = ((sizeof(BytecodeInterpreter) + slop_factor) >> LogBytesPerWord) + // what is the slop factor? - //6815692//Method::extra_stack_words() + // extra push slots for MH adapters + Method::extra_stack_entries() + // extra stack for jsr 292 frame::memory_parameter_word_sp_offset + // register save area + param window (native ? frame::interpreter_frame_extra_outgoing_argument_words : 0); // JNI, class @@ -1221,9 +1221,7 @@ // Full size expression stack __ ld_ptr(constMethod, O3); __ lduh(O3, in_bytes(ConstMethod::max_stack_offset()), O3); - guarantee(!EnableInvokeDynamic, "no support yet for java.lang.invoke.MethodHandle"); //6815692 - //6815692//if (EnableInvokeDynamic) - //6815692// __ inc(O3, Method::extra_stack_entries()); + __ inc(O3, Method::extra_stack_entries()); __ sll(O3, LogBytesPerWord, O3); __ sub(O2, O3, O3); // __ sub(O3, wordSize, O3); // so prepush doesn't look out of bounds @@ -2084,9 +2082,7 @@ const int fixed_size = sizeof(BytecodeInterpreter)/wordSize + // interpreter state object frame::memory_parameter_word_sp_offset; // register save area + param window - const int extra_stack = 0; //6815692//Method::extra_stack_entries(); return (round_to(max_stack + - extra_stack + slop_factor + fixed_size + monitor_size + @@ -2173,8 +2169,7 @@ // Need +1 here because stack_base points to the word just above the first expr stack entry // and stack_limit is supposed to point to the word just below the last expr stack entry. // See generate_compute_interpreter_state. - int extra_stack = 0; //6815692//Method::extra_stack_entries(); - to_fill->_stack_limit = stack_base - (method->max_stack() + 1 + extra_stack); + to_fill->_stack_limit = stack_base - (method->max_stack() + 1); to_fill->_monitor_base = (BasicObjectLock*) monitor_base; // sparc specific diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/sparc/vm/interp_masm_sparc.cpp --- a/src/cpu/sparc/vm/interp_masm_sparc.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/sparc/vm/interp_masm_sparc.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -521,7 +521,7 @@ // Compute max expression stack+register save area ld_ptr(Lmethod, in_bytes(Method::const_offset()), Gframe_size); lduh(Gframe_size, in_bytes(ConstMethod::max_stack_offset()), Gframe_size); // Load max stack. - add( Gframe_size, frame::memory_parameter_word_sp_offset, Gframe_size ); + add(Gframe_size, frame::memory_parameter_word_sp_offset+Method::extra_stack_entries(), Gframe_size ); // // now set up a stack frame with the size computed above diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/sparc/vm/templateInterpreter_sparc.cpp --- a/src/cpu/sparc/vm/templateInterpreter_sparc.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/sparc/vm/templateInterpreter_sparc.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -507,7 +507,7 @@ const int extra_space = rounded_vm_local_words + // frame local scratch space - //6815692//Method::extra_stack_words() + // extra push slots for MH adapters + Method::extra_stack_entries() + // extra stack for jsr 292 frame::memory_parameter_word_sp_offset + // register save area (native_call ? frame::interpreter_frame_extra_outgoing_argument_words : 0); @@ -1558,7 +1558,6 @@ round_to(callee_extra_locals * Interpreter::stackElementWords, WordsPerLong); const int max_stack_words = max_stack * Interpreter::stackElementWords; return (round_to((max_stack_words - //6815692//+ Method::extra_stack_words() + rounded_vm_local_words + frame::memory_parameter_word_sp_offset), WordsPerLong) // already rounded diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/x86/vm/cppInterpreter_x86.cpp --- a/src/cpu/x86/vm/cppInterpreter_x86.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/x86/vm/cppInterpreter_x86.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -539,12 +539,11 @@ // compute full expression stack limit - const int extra_stack = 0; //6815692//Method::extra_stack_words(); __ movptr(rdx, Address(rbx, Method::const_offset())); __ load_unsigned_short(rdx, Address(rdx, ConstMethod::max_stack_offset())); // get size of expression stack in words __ negptr(rdx); // so we can subtract in next step // Allocate expression stack - __ lea(rsp, Address(rsp, rdx, Address::times_ptr, -extra_stack)); + __ lea(rsp, Address(rsp, rdx, Address::times_ptr, -Method::extra_stack_words())); __ movptr(STATE(_stack_limit), rsp); } @@ -692,10 +691,9 @@ // Always give one monitor to allow us to start interp if sync method. // Any additional monitors need a check when moving the expression stack const int one_monitor = frame::interpreter_frame_monitor_size() * wordSize; - const int extra_stack = 0; //6815692//Method::extra_stack_entries(); __ movptr(rax, Address(rbx, Method::const_offset())); __ load_unsigned_short(rax, Address(rax, ConstMethod::max_stack_offset())); // get size of expression stack in words - __ lea(rax, Address(noreg, rax, Interpreter::stackElementScale(), extra_stack + one_monitor)); + __ lea(rax, Address(noreg, rax, Interpreter::stackElementScale(), one_monitor+Method::extra_stack_words())); __ lea(rax, Address(rax, rdx, Interpreter::stackElementScale(), overhead_size)); #ifdef ASSERT @@ -2265,8 +2263,7 @@ const int overhead_size = sizeof(BytecodeInterpreter)/wordSize + ( frame::sender_sp_offset - frame::link_offset) + 2; - const int extra_stack = 0; //6815692//Method::extra_stack_entries(); - const int method_stack = (method->max_locals() + method->max_stack() + extra_stack) * + const int method_stack = (method->max_locals() + method->max_stack()) * Interpreter::stackElementWords; return overhead_size + method_stack + stub_code; } @@ -2331,8 +2328,7 @@ // Need +1 here because stack_base points to the word just above the first expr stack entry // and stack_limit is supposed to point to the word just below the last expr stack entry. // See generate_compute_interpreter_state. - int extra_stack = 0; //6815692//Method::extra_stack_entries(); - to_fill->_stack_limit = stack_base - (method->max_stack() + extra_stack + 1); + to_fill->_stack_limit = stack_base - (method->max_stack() + 1); to_fill->_monitor_base = (BasicObjectLock*) monitor_base; to_fill->_self_link = to_fill; @@ -2380,8 +2376,7 @@ monitor_size); // Now with full size expression stack - int extra_stack = 0; //6815692//Method::extra_stack_entries(); - int full_frame_size = short_frame_size + (method->max_stack() + extra_stack) * BytesPerWord; + int full_frame_size = short_frame_size + method->max_stack() * BytesPerWord; // and now with only live portion of the expression stack short_frame_size = short_frame_size + tempcount * BytesPerWord; diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/x86/vm/templateInterpreter_x86_32.cpp --- a/src/cpu/x86/vm/templateInterpreter_x86_32.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/x86/vm/templateInterpreter_x86_32.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -1565,8 +1565,7 @@ // be sure to change this if you add/subtract anything to/from the overhead area const int overhead_size = -frame::interpreter_frame_initial_sp_offset; - const int extra_stack = Method::extra_stack_entries(); - const int method_stack = (method->max_locals() + method->max_stack() + extra_stack) * + const int method_stack = (method->max_locals() + method->max_stack()) * Interpreter::stackElementWords; return overhead_size + method_stack + stub_code; } diff -r c07dd9be16e8 -r 603ca7e51354 src/cpu/x86/vm/templateInterpreter_x86_64.cpp --- a/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -1574,8 +1574,7 @@ -(frame::interpreter_frame_initial_sp_offset) + entry_size; const int stub_code = frame::entry_frame_after_call_words; - const int extra_stack = Method::extra_stack_entries(); - const int method_stack = (method->max_locals() + method->max_stack() + extra_stack) * + const int method_stack = (method->max_locals() + method->max_stack()) * Interpreter::stackElementWords; return (overhead_size + method_stack + stub_code); } diff -r c07dd9be16e8 -r 603ca7e51354 src/share/vm/interpreter/bytecodeInterpreter.cpp --- a/src/share/vm/interpreter/bytecodeInterpreter.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/share/vm/interpreter/bytecodeInterpreter.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -468,7 +468,25 @@ #ifdef ASSERT if (istate->_msg != initialize) { - assert(abs(istate->_stack_base - istate->_stack_limit) == (istate->_method->max_stack() + 1), "bad stack limit"); + // We have a problem here if we are running with a pre-hsx24 JDK (for example during bootstrap) + // because in that case, EnableInvokeDynamic is true by default but will be later switched off + // if java_lang_invoke_MethodHandle::compute_offsets() detects that the JDK only has the classes + // for the old JSR292 implementation. + // This leads to a situation where 'istate->_stack_limit' always accounts for + // methodOopDesc::extra_stack_entries() because it is computed in + // CppInterpreterGenerator::generate_compute_interpreter_state() which was generated while + // EnableInvokeDynamic was still true. On the other hand, istate->_method->max_stack() doesn't + // account for extra_stack_entries() anymore because at the time when it is called + // EnableInvokeDynamic was already set to false. + // So we have a second version of the assertion which handles the case where EnableInvokeDynamic was + // switched off because of the wrong classes. + if (EnableInvokeDynamic || FLAG_IS_CMDLINE(EnableInvokeDynamic)) { + assert(abs(istate->_stack_base - istate->_stack_limit) == (istate->_method->max_stack() + 1), "bad stack limit"); + } else { + const int extra_stack_entries = Method::extra_stack_entries_for_indy; + assert(labs(istate->_stack_base - istate->_stack_limit) == (istate->_method->max_stack() + extra_stack_entries + + 1), "bad stack limit"); + } #ifndef SHARK IA32_ONLY(assert(istate->_stack_limit == istate->_thread->last_Java_sp() + 1, "wrong")); #endif // !SHARK diff -r c07dd9be16e8 -r 603ca7e51354 src/share/vm/oops/method.hpp --- a/src/share/vm/oops/method.hpp Fri May 31 06:41:50 2013 +0200 +++ b/src/share/vm/oops/method.hpp Wed Apr 24 11:49:38 2013 +0200 @@ -671,13 +671,15 @@ Symbol* signature, //anything at all TRAPS); static Klass* check_non_bcp_klass(Klass* klass); - // these operate only on invoke methods: + + // How many extra stack entries for invokedynamic when it's enabled + static const int extra_stack_entries_for_jsr292 = 1; + + // this operates only on invoke methods: // presize interpreter frames for extra interpreter stack entries, if needed - // method handles want to be able to push a few extra values (e.g., a bound receiver), and - // invokedynamic sometimes needs to push a bootstrap method, call site, and arglist, - // all without checking for a stack overflow - static int extra_stack_entries() { return EnableInvokeDynamic ? 2 : 0; } - static int extra_stack_words(); // = extra_stack_entries() * Interpreter::stackElementSize() + // Account for the extra appendix argument for invokehandle/invokedynamic + static int extra_stack_entries() { return EnableInvokeDynamic ? extra_stack_entries_for_jsr292 : 0; } + static int extra_stack_words(); // = extra_stack_entries() * Interpreter::stackElementSize // RedefineClasses() support: bool is_old() const { return access_flags().is_old(); } diff -r c07dd9be16e8 -r 603ca7e51354 src/share/vm/opto/matcher.cpp --- a/src/share/vm/opto/matcher.cpp Fri May 31 06:41:50 2013 +0200 +++ b/src/share/vm/opto/matcher.cpp Wed Apr 24 11:49:38 2013 +0200 @@ -1282,16 +1282,6 @@ mcall->_argsize = out_arg_limit_per_call - begin_out_arg_area; } - if (is_method_handle_invoke) { - // Kill some extra stack space in case method handles want to do - // a little in-place argument insertion. - // FIXME: Is this still necessary? - int regs_per_word = NOT_LP64(1) LP64_ONLY(2); // %%% make a global const! - out_arg_limit_per_call += Method::extra_stack_entries() * regs_per_word; - // Do not update mcall->_argsize because (a) the extra space is not - // pushed as arguments and (b) _argsize is dead (not used anywhere). - } - // Compute the max stack slot killed by any call. These will not be // available for debug info, and will be used to adjust FIRST_STACK_mask // after all call sites have been visited.