From 0f2da83f79bdccff6dfb823238f971d6eb495ab8 Mon Sep 17 00:00:00 2001 From: Martin Doerr Date: Wed, 22 Nov 2023 11:59:23 +0000 Subject: [PATCH] 8316746: Top of lock-stack does not match the unlocked object Reviewed-by: rrich Backport-of: 7d8adfa855e51a90c2f125fc20a06f9a488e6248 --- src/hotspot/cpu/ppc/frame_ppc.inline.hpp | 1 - src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp | 2 +- src/hotspot/cpu/ppc/templateTable_ppc_64.cpp | 140 ++++++++---------- .../jtreg/compiler/locks/TestUnlockOSR.java | 56 +++++++ 4 files changed, 117 insertions(+), 82 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java diff --git a/src/hotspot/cpu/ppc/frame_ppc.inline.hpp b/src/hotspot/cpu/ppc/frame_ppc.inline.hpp index 239db8224c0..876ad046a0c 100644 --- a/src/hotspot/cpu/ppc/frame_ppc.inline.hpp +++ b/src/hotspot/cpu/ppc/frame_ppc.inline.hpp @@ -143,7 +143,6 @@ inline intptr_t* frame::interpreter_frame_mdp_addr() const { return (intptr_t*) &(get_ijava_state()->mdx); } -// Pointer beyond the "oldest/deepest" BasicObjectLock on stack. inline BasicObjectLock* frame::interpreter_frame_monitor_end() const { return (BasicObjectLock*) get_ijava_state()->monitors; } diff --git a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp index 70466fdf3c3..fb07b3f4465 100644 --- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp +++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp @@ -1933,7 +1933,7 @@ void InterpreterMacroAssembler::profile_parameters_type(Register tmp1, Register } } -// Add a InterpMonitorElem to stack (see frame_sparc.hpp). +// Add a monitor (see frame_ppc.hpp). void InterpreterMacroAssembler::add_monitor_to_stack(bool stack_is_empty, Register Rtemp1, Register Rtemp2) { // Very-local scratch registers. diff --git a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp index 2210e05a410..13c13fe843d 100644 --- a/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp +++ b/src/hotspot/cpu/ppc/templateTable_ppc_64.cpp @@ -4045,90 +4045,78 @@ void TemplateTable::athrow() { // at next monitor exit. void TemplateTable::monitorenter() { transition(atos, vtos); - __ verify_oop(R17_tos); - Register Rcurrent_monitor = R11_scratch1, - Rcurrent_obj = R12_scratch2, + Register Rcurrent_monitor = R3_ARG1, + Rcurrent_obj = R4_ARG2, Robj_to_lock = R17_tos, - Rscratch1 = R3_ARG1, - Rscratch2 = R4_ARG2, - Rscratch3 = R5_ARG3, - Rcurrent_obj_addr = R6_ARG4; + Rscratch1 = R11_scratch1, + Rscratch2 = R12_scratch2, + Rbot = R5_ARG3, + Rfree_slot = R6_ARG4; + + Label Lfound, Lallocate_new; + + __ ld(Rscratch1, _abi0(callers_sp), R1_SP); // load FP + __ li(Rfree_slot, 0); // Points to free slot or null. + + // Set up search loop - start with topmost monitor. + __ mr(Rcurrent_monitor, R26_monitor); + __ addi(Rbot, Rscratch1, -frame::ijava_state_size); // ------------------------------------------------------------------------------ // Null pointer exception. - __ null_check_throw(Robj_to_lock, -1, R11_scratch1); + __ null_check_throw(Robj_to_lock, -1, Rscratch1); - // Try to acquire a lock on the object. - // Repeat until succeeded (i.e., until monitorenter returns true). + // Check if any slot is present => short cut to allocation if not. + __ cmpld(CCR0, Rcurrent_monitor, Rbot); + __ beq(CCR0, Lallocate_new); // ------------------------------------------------------------------------------ // Find a free slot in the monitor block. - Label Lfound, Lexit, Lallocate_new; - ConditionRegister found_free_slot = CCR0, - found_same_obj = CCR1, - reached_limit = CCR6; + // Note: The order of the monitors is important for C2 OSR which derives the + // unlock order from it (see comments for interpreter_frame_monitor_*). { - Label Lloop; - Register Rlimit = Rcurrent_monitor; - - // Set up search loop - start with topmost monitor. - __ add(Rcurrent_obj_addr, BasicObjectLock::obj_offset_in_bytes(), R26_monitor); + Label Lloop, LnotFree, Lexit; - __ ld(Rlimit, 0, R1_SP); - __ addi(Rlimit, Rlimit, - (frame::ijava_state_size + frame::interpreter_frame_monitor_size_in_bytes() - BasicObjectLock::obj_offset_in_bytes())); // Monitor base + __ bind(Lloop); + __ ld(Rcurrent_obj, BasicObjectLock::obj_offset_in_bytes(), Rcurrent_monitor); + // Exit if current entry is for same object; this guarantees, that new monitor + // used for recursive lock is above the older one. + __ cmpd(CCR0, Rcurrent_obj, Robj_to_lock); + __ beq(CCR0, Lexit); // recursive locking - // Check if any slot is present => short cut to allocation if not. - __ cmpld(reached_limit, Rcurrent_obj_addr, Rlimit); - __ bgt(reached_limit, Lallocate_new); + __ cmpdi(CCR0, Rcurrent_obj, 0); + __ bne(CCR0, LnotFree); + __ mr(Rfree_slot, Rcurrent_monitor); // remember free slot closest to the bottom + __ bind(LnotFree); - // Pre-load topmost slot. - __ ld(Rcurrent_obj, 0, Rcurrent_obj_addr); - __ addi(Rcurrent_obj_addr, Rcurrent_obj_addr, frame::interpreter_frame_monitor_size() * wordSize); - // The search loop. - __ bind(Lloop); - // Found free slot? - __ cmpdi(found_free_slot, Rcurrent_obj, 0); - // Is this entry for same obj? If so, stop the search and take the found - // free slot or allocate a new one to enable recursive locking. - __ cmpd(found_same_obj, Rcurrent_obj, Robj_to_lock); - __ cmpld(reached_limit, Rcurrent_obj_addr, Rlimit); - __ beq(found_free_slot, Lexit); - __ beq(found_same_obj, Lallocate_new); - __ bgt(reached_limit, Lallocate_new); - // Check if last allocated BasicLockObj reached. - __ ld(Rcurrent_obj, 0, Rcurrent_obj_addr); - __ addi(Rcurrent_obj_addr, Rcurrent_obj_addr, frame::interpreter_frame_monitor_size() * wordSize); - // Next iteration if unchecked BasicObjectLocks exist on the stack. - __ b(Lloop); + __ addi(Rcurrent_monitor, Rcurrent_monitor, frame::interpreter_frame_monitor_size_in_bytes()); + __ cmpld(CCR0, Rcurrent_monitor, Rbot); + __ bne(CCR0, Lloop); + __ bind(Lexit); } // ------------------------------------------------------------------------------ // Check if we found a free slot. - __ bind(Lexit); - - __ addi(Rcurrent_monitor, Rcurrent_obj_addr, -(frame::interpreter_frame_monitor_size() * wordSize) - BasicObjectLock::obj_offset_in_bytes()); - __ addi(Rcurrent_obj_addr, Rcurrent_obj_addr, - frame::interpreter_frame_monitor_size() * wordSize); - __ b(Lfound); + __ cmpdi(CCR0, Rfree_slot, 0); + __ bne(CCR0, Lfound); // We didn't find a free BasicObjLock => allocate one. - __ align(32, 12); __ bind(Lallocate_new); __ add_monitor_to_stack(false, Rscratch1, Rscratch2); - __ mr(Rcurrent_monitor, R26_monitor); - __ addi(Rcurrent_obj_addr, R26_monitor, BasicObjectLock::obj_offset_in_bytes()); + __ mr(Rfree_slot, R26_monitor); // ------------------------------------------------------------------------------ // We now have a slot to lock. __ bind(Lfound); // Increment bcp to point to the next bytecode, so exception handling for async. exceptions work correctly. - // The object has already been poped from the stack, so the expression stack looks correct. + // The object has already been popped from the stack, so the expression stack looks correct. __ addi(R14_bcp, R14_bcp, 1); - __ std(Robj_to_lock, 0, Rcurrent_obj_addr); - __ lock_object(Rcurrent_monitor, Robj_to_lock); + __ std(Robj_to_lock, BasicObjectLock::obj_offset_in_bytes(), Rfree_slot); + __ lock_object(Rfree_slot, Robj_to_lock); // Check if there's enough space on the stack for the monitors after locking. // This emits a single store. @@ -4142,46 +4130,40 @@ void TemplateTable::monitorexit() { transition(atos, vtos); __ verify_oop(R17_tos); - Register Rcurrent_monitor = R11_scratch1, - Rcurrent_obj = R12_scratch2, + Register Rcurrent_monitor = R3_ARG1, + Rcurrent_obj = R4_ARG2, Robj_to_lock = R17_tos, - Rcurrent_obj_addr = R3_ARG1, - Rlimit = R4_ARG2; + Rscratch = R11_scratch1, + Rbot = R12_scratch2; + Label Lfound, Lillegal_monitor_state; - // Check corner case: unbalanced monitorEnter / Exit. - __ ld(Rlimit, 0, R1_SP); - __ addi(Rlimit, Rlimit, - (frame::ijava_state_size + frame::interpreter_frame_monitor_size_in_bytes())); // Monitor base + __ ld(Rscratch, _abi0(callers_sp), R1_SP); // load FP + + // Set up search loop - start with topmost monitor. + __ mr(Rcurrent_monitor, R26_monitor); + __ addi(Rbot, Rscratch, -frame::ijava_state_size); // Null pointer check. - __ null_check_throw(Robj_to_lock, -1, R11_scratch1); + __ null_check_throw(Robj_to_lock, -1, Rscratch); - __ cmpld(CCR0, R26_monitor, Rlimit); - __ bgt(CCR0, Lillegal_monitor_state); + // Check corner case: unbalanced monitorEnter / Exit. + __ cmpld(CCR0, Rcurrent_monitor, Rbot); + __ beq(CCR0, Lillegal_monitor_state); // Find the corresponding slot in the monitors stack section. { Label Lloop; - // Start with topmost monitor. - __ addi(Rcurrent_obj_addr, R26_monitor, BasicObjectLock::obj_offset_in_bytes()); - __ addi(Rlimit, Rlimit, BasicObjectLock::obj_offset_in_bytes()); - __ ld(Rcurrent_obj, 0, Rcurrent_obj_addr); - __ addi(Rcurrent_obj_addr, Rcurrent_obj_addr, frame::interpreter_frame_monitor_size() * wordSize); - __ bind(Lloop); + __ ld(Rcurrent_obj, BasicObjectLock::obj_offset_in_bytes(), Rcurrent_monitor); // Is this entry for same obj? __ cmpd(CCR0, Rcurrent_obj, Robj_to_lock); __ beq(CCR0, Lfound); - // Check if last allocated BasicLockObj reached. - - __ ld(Rcurrent_obj, 0, Rcurrent_obj_addr); - __ cmpld(CCR0, Rcurrent_obj_addr, Rlimit); - __ addi(Rcurrent_obj_addr, Rcurrent_obj_addr, frame::interpreter_frame_monitor_size() * wordSize); - - // Next iteration if unchecked BasicObjectLocks exist on the stack. - __ ble(CCR0, Lloop); + __ addi(Rcurrent_monitor, Rcurrent_monitor, frame::interpreter_frame_monitor_size_in_bytes()); + __ cmpld(CCR0, Rcurrent_monitor, Rbot); + __ bne(CCR0, Lloop); } // Fell through without finding the basic obj lock => throw up! @@ -4191,8 +4173,6 @@ void TemplateTable::monitorexit() { __ align(32, 12); __ bind(Lfound); - __ addi(Rcurrent_monitor, Rcurrent_obj_addr, - -(frame::interpreter_frame_monitor_size() * wordSize) - BasicObjectLock::obj_offset_in_bytes()); __ unlock_object(Rcurrent_monitor); } diff --git a/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java b/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java new file mode 100644 index 00000000000..f2133b49658 --- /dev/null +++ b/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2023 SAP SE. 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. + * + */ + +/* + * @test + * @bug 8316746 + * @summary During OSR, locks get transferred from interpreter frame. + * Check that unlocking 2 such locks works in the OSR compiled nmethod. + * Some platforms verify that the unlocking happens in the corrent order. + * + * @run main/othervm -Xbatch TestUnlockOSR + */ + +public class TestUnlockOSR { + static void test_method(Object a, Object b, int limit) { + synchronized(a) { // allocate space for monitors + synchronized(b) { + } + } // free space to test allocation in reused space + synchronized(a) { // reuse the space + synchronized(b) { + for (int i = 0; i < limit; i++) {} + } + } + } + + public static void main(String[] args) { + Object a = new TestUnlockOSR(), + b = new TestUnlockOSR(); + // avoid uncommon trap before last unlocks + for (int i = 0; i < 100; i++) { test_method(a, b, 0); } + // trigger OSR + test_method(a, b, 100000); + } +}