From 5df7089c3eb2e6d7cf6634840a2a21bcaa7e3f4e Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Thu, 8 May 2025 10:56:01 +0000 Subject: [PATCH] 8350398: [s390x] Relativize initial_sp/monitors in interpreter frames Reviewed-by: lucy, aph --- src/hotspot/cpu/s390/frame_s390.hpp | 4 ++-- src/hotspot/cpu/s390/frame_s390.inline.hpp | 15 +++++++----- src/hotspot/cpu/s390/interp_masm_s390.cpp | 23 ++++++++++++++++--- .../templateInterpreterGenerator_s390.cpp | 8 +++++-- src/hotspot/cpu/s390/templateTable_s390.cpp | 9 +++++--- 5 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/hotspot/cpu/s390/frame_s390.hpp b/src/hotspot/cpu/s390/frame_s390.hpp index 1aaabfb1230..ab15e75bc5b 100644 --- a/src/hotspot/cpu/s390/frame_s390.hpp +++ b/src/hotspot/cpu/s390/frame_s390.hpp @@ -498,8 +498,6 @@ inline z_ijava_state* ijava_state() const; - // Where z_ijava_state.monitors is saved. - inline BasicObjectLock** interpreter_frame_monitors_addr() const; // Where z_ijava_state.esp is saved. inline intptr_t** interpreter_frame_esp_addr() const; @@ -517,6 +515,8 @@ // Next two functions read and write z_ijava_state.monitors. private: inline BasicObjectLock* interpreter_frame_monitors() const; + + // Where z_ijava_state.monitors is saved. inline void interpreter_frame_set_monitors(BasicObjectLock* monitors); public: diff --git a/src/hotspot/cpu/s390/frame_s390.inline.hpp b/src/hotspot/cpu/s390/frame_s390.inline.hpp index ede7039ce0a..59e23af7f48 100644 --- a/src/hotspot/cpu/s390/frame_s390.inline.hpp +++ b/src/hotspot/cpu/s390/frame_s390.inline.hpp @@ -109,16 +109,19 @@ inline frame::z_ijava_state* frame::ijava_state() const { return state; } -inline BasicObjectLock** frame::interpreter_frame_monitors_addr() const { - return (BasicObjectLock**) &(ijava_state()->monitors); -} - // The next two functions read and write z_ijava_state.monitors. inline BasicObjectLock* frame::interpreter_frame_monitors() const { - return *interpreter_frame_monitors_addr(); + BasicObjectLock* result = (BasicObjectLock*) at_relative(_z_ijava_idx(monitors)); + // make sure the pointer points inside the frame + assert(sp() <= (intptr_t*) result, "monitor end should be above the stack pointer"); + assert((intptr_t*) result < fp(), "monitor end should be strictly below the frame pointer: result: " INTPTR_FORMAT " fp: " INTPTR_FORMAT, p2i(result), p2i(fp())); + return result; } + inline void frame::interpreter_frame_set_monitors(BasicObjectLock* monitors) { - *interpreter_frame_monitors_addr() = monitors; + assert(is_interpreted_frame(), "interpreted frame expected"); + // set relativized monitors + ijava_state()->monitors = (intptr_t) ((intptr_t*)monitors - fp()); } // Accessors diff --git a/src/hotspot/cpu/s390/interp_masm_s390.cpp b/src/hotspot/cpu/s390/interp_masm_s390.cpp index 9717d260279..aac130ea66e 100644 --- a/src/hotspot/cpu/s390/interp_masm_s390.cpp +++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp @@ -627,7 +627,7 @@ void InterpreterMacroAssembler::verify_esp(Register Resp, Register Rtemp) { // i.e. IJAVA_STATE.monitors > Resp. NearLabel OK; Register Rmonitors = Rtemp; - z_lg(Rmonitors, _z_ijava_state_neg(monitors), Z_fp); + get_monitors(Rmonitors); compareU64_and_branch(Rmonitors, Resp, bcondHigh, OK); reentry = stop_chain_static(reentry, "too many pops: Z_esp points into monitor area"); bind(OK); @@ -676,10 +676,28 @@ void InterpreterMacroAssembler::restore_esp() { void InterpreterMacroAssembler::get_monitors(Register reg) { asm_assert_ijava_state_magic(reg); +#ifdef ASSERT + NearLabel ok; + z_cg(Z_fp, 0, Z_SP); + z_bre(ok); + stop("Z_fp is corrupted"); + bind(ok); +#endif // ASSERT mem2reg_opt(reg, Address(Z_fp, _z_ijava_state_neg(monitors))); + z_slag(reg, reg, Interpreter::logStackElementSize); + z_agr(reg, Z_fp); } void InterpreterMacroAssembler::save_monitors(Register reg) { +#ifdef ASSERT + NearLabel ok; + z_cg(Z_fp, 0, Z_SP); + z_bre(ok); + stop("Z_fp is corrupted"); + bind(ok); +#endif // ASSERT + z_sgr(reg, Z_fp); + z_srag(reg, reg, Interpreter::logStackElementSize); reg2mem_opt(reg, Address(Z_fp, _z_ijava_state_neg(monitors))); } @@ -840,12 +858,11 @@ void InterpreterMacroAssembler::unlock_if_synchronized_method(TosState state, // register for unlock_object to pass to VM directly. Register R_current_monitor = Z_ARG2; Register R_monitor_block_bot = Z_ARG1; - const Address monitor_block_top(Z_fp, _z_ijava_state_neg(monitors)); const Address monitor_block_bot(Z_fp, -frame::z_ijava_state_size); bind(restart); // Starting with top-most entry. - z_lg(R_current_monitor, monitor_block_top); + get_monitors(R_current_monitor); // Points to word before bottom of monitor block. load_address(R_monitor_block_bot, monitor_block_bot); z_bru(entry); diff --git a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp index 04fe574088e..c3c99d7297d 100644 --- a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp +++ b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp @@ -1174,7 +1174,11 @@ void TemplateInterpreterGenerator::generate_fixed_frame(bool native_call) { // z_ijava_state->monitors = fp - frame::z_ijava_state_size - Interpreter::stackElementSize; // z_ijava_state->esp = Z_esp = z_ijava_state->monitors; __ add2reg(Z_esp, -frame::z_ijava_state_size, fp); - __ z_stg(Z_esp, _z_ijava_state_neg(monitors), fp); + + __ z_sgrk(Z_R0, Z_esp, fp); + __ z_srag(Z_R0, Z_R0, Interpreter::logStackElementSize); + __ z_stg(Z_R0, _z_ijava_state_neg(monitors), fp); + __ add2reg(Z_esp, -Interpreter::stackElementSize); __ z_stg(Z_esp, _z_ijava_state_neg(esp), fp); @@ -1633,7 +1637,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) { __ add2reg(Rfirst_monitor, -(frame::z_ijava_state_size + (int)sizeof(BasicObjectLock)), Z_fp); #ifdef ASSERT NearLabel ok; - __ z_lg(Z_R1, _z_ijava_state_neg(monitors), Z_fp); + __ get_monitors(Z_R1); __ compareU64_and_branch(Rfirst_monitor, Z_R1, Assembler::bcondEqual, ok); reentry = __ stop_chain_static(reentry, "native_entry:unlock: inconsistent z_ijava_state.monitors"); __ bind(ok); diff --git a/src/hotspot/cpu/s390/templateTable_s390.cpp b/src/hotspot/cpu/s390/templateTable_s390.cpp index 4262c77ecd7..2b39cc8318c 100644 --- a/src/hotspot/cpu/s390/templateTable_s390.cpp +++ b/src/hotspot/cpu/s390/templateTable_s390.cpp @@ -65,7 +65,8 @@ // The actual size of each block heavily depends on the CPU capabilities and, // of course, on the logic implemented in each block. #ifdef ASSERT - #define BTB_MINSIZE 256 +// With introduced assert in get_monitor() & set_monitor(), required block size is now 322. + #define BTB_MINSIZE 512 #else #define BTB_MINSIZE 64 #endif @@ -91,7 +92,8 @@ if (len > alignment) { \ tty->print_cr("%4d of %4d @ " INTPTR_FORMAT ": Block len for %s", \ len, alignment, e_addr-len, name); \ - guarantee(len <= alignment, "block too large"); \ + guarantee(len <= alignment, "block too large, len = %d, alignment = %d", \ + len, alignment); \ } \ guarantee(len == e_addr-b_addr, "block len mismatch"); \ } @@ -112,7 +114,8 @@ if (len > alignment) { \ tty->print_cr("%4d of %4d @ " INTPTR_FORMAT ": Block len for %s", \ len, alignment, e_addr-len, name); \ - guarantee(len <= alignment, "block too large"); \ + guarantee(len <= alignment, "block too large, len = %d, alignment = %d", \ + len, alignment); \ } \ guarantee(len == e_addr-b_addr, "block len mismatch"); \ }