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

[Stefan Karlsson]

On Mon, 2 Oct 2023 13:53: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.
>
> Nick Gasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review suggestions

Hi, while poking around in the locking code we also found this usage of rscratch1, which seems to be problematic:

  cmpxchg(tmp, zr, rthread, Assembler::xword, /*acquire*/ true,
          /*release*/ true, /*weak*/ false, rscratch1); // Sets flags for result

  if (LockingMode != LM_LIGHTWEIGHT) {
    // Store a non-null value into the box to avoid looking like a re-entrant
    // lock. The fast-path monitor unlock code checks for
    // markWord::monitor_value so use markWord::unused_mark which has the
    // relevant bit set, and also matches ObjectSynchronizer::enter.
    mov(tmp, (address)markWord::unused_mark().value());
    str(tmp, Address(box, BasicLock::displaced_header_offset_in_bytes()));
  }
  br(Assembler::EQ, cont); // CAS success means locking succeeded

  cmp(rscratch1, rthread);
  br(Assembler::NE, cont); // Check for recursive locking


I think we can use the new `tmp3Reg` instead of `rscratch1` here.

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

PR Comment: https://git.openjdk.org/jdk/pull/15978#issuecomment-1745100334