RFR: 8316880: AArch64: "stop: Header is not fast-locked" with -XX:-UseLSE since JDK-8315880

[Andrew Haley]

On Fri, 29 Sep 2023 08:12:06 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

> Building a fastdebug image on a machine without LSE (e.g. A72) or explicitly disabling LSE results in:
> 
> 
>   #
>   # A fatal error has been detected by the Java Runtime Environment:
>   #
>   # Internal Error (0xe0000000), pid=64585, tid=64619
>   # stop: Header is not fast-locked
>   #
>   # JRE version: OpenJDK Runtime Environment (22.0) (fastdebug build 22-internal-git-a2391a92c)
>   # Java VM: OpenJDK 64-Bit Server VM (fastdebug 22-internal-git-a2391a92c, mixed mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-aarch64)
>   # Problematic frame:
>   # J 1373 c2 sun.nio.ch.NativeThreadSet.add()I java.base (155 bytes) @ 0x0000ffff7ccdf110 [0x0000ffff7ccdef80+0x0000000000000190]
>   #
> 
> 
> When UseLSE is false `MacroAssembler::cmpxchg()` uses rscratch1 as a temporary to store the result of the store-exclusive instruction. However rscratch1 may also be one of the registers passed as t1 or t2 to `MacroAssembler::lightweight_lock()` and holding a live value which is then clobbered.  Fixed by ensuring rscratch1 is never passed as one of these temporaries.

People unfamiliar with the platform conventions are going to keep getting this wrong. The scratch registers are used in macros: that's what they are for.

Please add:

diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index e3df32ed602..4dbbedc123e 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -2735,6 +2735,10 @@ void MacroAssembler::cmpxchg(Register addr, Register expected,
     mov(result, expected);
     lse_cas(result, new_val, addr, size, acquire, release, /*not_pair*/ true);
     compare_eq(result, expected, size);
+#ifdef ASSERT
+    // Poison rscratch1
+    mov(rscratch1, 0x1f1f1f1f1f1f1f1f);
+#endif
   } else {
     Label retry_load, done;
     prfm(Address(addr), PSTL1STRM);

diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index 51faab3d73b..c72a478949c 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -6315,7 +6315,7 @@ void MacroAssembler::double_move(VMRegPair src, VMRegPair dst, Register tmp) {
 //  - t1, t2: temporary registers, will be destroyed
 void MacroAssembler::lightweight_lock(Register obj, Register hdr, Register t1, Register t2, Label& slow) {
   assert(LockingMode == LM_LIGHTWEIGHT, "only used with new lightweight locking");
-  assert_different_registers(obj, hdr, t1, t2);
+  assert_different_registers(obj, hdr, t1, t2, rscratch1, rscratch2);
 
   // Check if we would have space on lock-stack for the object.
   ldrw(t1, Address(rthread, JavaThread::lock_stack_top_offset()));

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 107:

> 105:   } else {
> 106:     assert(LockingMode == LM_LIGHTWEIGHT, "must be");
> 107:     lightweight_lock(oop, disp_hdr, tmp, rscratch2, no_count);

Please use an allocated scratch register.

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 904:

> 902:       ldr(header_reg, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
> 903:       tbnz(header_reg, exact_log2(markWord::monitor_value), slow_case);
> 904:       lightweight_unlock(obj_reg, header_reg, swap_reg, rscratch2, slow_case);

And here.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 1816:

> 1814:       assert(LockingMode == LM_LIGHTWEIGHT, "must be");
> 1815:       __ ldr(swap_reg, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
> 1816:       __ lightweight_lock(obj_reg, swap_reg, tmp, rscratch2, slow_path_lock);

And here.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 1957:

> 1955:       __ ldr(old_hdr, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
> 1956:       __ tbnz(old_hdr, exact_log2(markWord::monitor_value), slow_path_unlock);
> 1957:       __ lightweight_unlock(obj_reg, old_hdr, swap_reg, rscratch2, slow_path_unlock);

And here.

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

PR Comment: https://git.openjdk.org/jdk/pull/15978#issuecomment-1740830043
PR Comment: https://git.openjdk.org/jdk/pull/15978#issuecomment-1740847447
PR Review Comment: https://git.openjdk.org/jdk/pull/15978#discussion_r1341316596
PR Review Comment: https://git.openjdk.org/jdk/pull/15978#discussion_r1341316893
PR Review Comment: https://git.openjdk.org/jdk/pull/15978#discussion_r1341317694
PR Review Comment: https://git.openjdk.org/jdk/pull/15978#discussion_r1341317842