RFR: 8316746: Top of lock-stack does not match the unlocked object [v3]

[Martin Doerr]

On Thu, 28 Sep 2023 10:38:52 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> I think we need to support "Top of lock-stack does not match the unlocked object" and take the slow path. Reason: see JBS issue.
>> Currently only PPC64, x86_64 and aarch64 code. I'd like to get feedback before implementing it for other platforms (needed for all platforms).
>
> Martin Doerr has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Pass may_be_unordered information to lightweight_unlock.
>  - Merge remote-tracking branch 'origin' into 8316746_lock_stack
>  - Add x86_64 and aarch64 implementation.
>  - 8316746: Top of lock-stack does not match the unlocked object

Valid point. Thanks! I've got it to work:

diff --git a/src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp b/src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp
index 78361a305ae..4571036477e 100644
--- a/src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp
+++ b/src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp
@@ -135,6 +135,8 @@ void C1_MacroAssembler::unlock_object(Register hdr, Register obj, Register disp_
 
   if (LockingMode == LM_LIGHTWEIGHT) {
     movptr(disp_hdr, Address(obj, hdr_offset));
+    testptr(disp_hdr, markWord::monitor_value);
+    jcc(Assembler::notEqual, slow_case);
     andptr(disp_hdr, ~(int32_t)markWord::lock_mask_in_place);
     lightweight_unlock(obj, disp_hdr, hdr, slow_case);
   } else if (LockingMode == LM_LEGACY) {
diff --git a/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp b/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
index 2154601f2f2..3666d1490fc 100644
--- a/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
+++ b/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp
@@ -863,7 +863,7 @@ void C2_MacroAssembler::fast_unlock(Register objReg, Register boxReg, Register t
   jccb  (Assembler::notZero, CheckSucc);
   // Without cast to int32_t this style of movptr will destroy r10 which is typically obj.
   movptr(Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)), NULL_WORD);
-  jmpb  (DONE_LABEL);
+  jmp  (DONE_LABEL);
 
   // Try to avoid passing control into the slow_path ...
   bind  (CheckSucc);
diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
index 26135c65418..0b3af526b32 100644
--- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp
+++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp
@@ -9836,6 +9836,15 @@ void MacroAssembler::lightweight_unlock(Register obj, Register hdr, Register tmp
   assert(hdr == rax, "header must be in rax for cmpxchg");
   assert_different_registers(obj, hdr, tmp);
 
+  {
+    Label tos_ok;
+    movl(tmp, Address(r15_thread, JavaThread::lock_stack_top_offset()));
+    cmpptr(obj, Address(r15_thread, tmp, Address::times_1, -oopSize));
+    jcc(Assembler::equal, tos_ok);
+    STOP("Top of lock-stack does not match the unlocked object");
+    bind(tos_ok);
+  }
+
   // Mark-word must be lock_mask now, try to swing it back to unlocked_value.
   movptr(tmp, hdr); // The expected old value
   orptr(tmp, markWord::unlocked_value);

The nsk/jdi/StepEvent tests have passed on my x86_64 machine. I couldn't reproduce the issue on that platform.

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

PR Comment: https://git.openjdk.org/jdk/pull/15903#issuecomment-1752936786