RFR: 8260899: ARM32: SyncOnValueBasedClassTest fails with assert(is_valid()) failed: invalid register

[Aleksey Shipilev]

On Fri, 5 Feb 2021 09:21:24 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> $ CONF=linux-arm-server-fastdebug make run-test TEST=runtime/Monitor/SyncOnValueBasedClassTest.java
>> ...
>> 
>> #  Internal Error (/home/pi/jdk/src/hotspot/cpu/arm/register_arm.hpp:155), pid=3793, tid=3808
>> #  assert(is_valid()) failed: invalid register
>> #
>> # JRE version: OpenJDK Runtime Environment (17.0) (fastdebug build 17-internal+0-adhoc.pi.jdk)
>> # Java VM: OpenJDK Server VM (fastdebug 17-internal+0-adhoc.pi.jdk, compiled mode, emulated-client, g1 gc, linux-arm)
>> # Problematic frame:
>> # V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>> 
>> Current CompileTask:
>> C1:    318    2   !b        java.lang.Class::desiredAssertionStatus (54 bytes)
>> 
>> Stack: [0x72580000,0x72600000],  sp=0x725fe170,  free space=504k
>> Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
>> V  [libjvm.so+0xe1a6a8]  MacroAssembler::load_klass(RegisterImpl*, RegisterImpl*, AsmCondition)+0xa4
>> V  [libjvm.so+0x43b6b4]  C1_MacroAssembler::lock_object(RegisterImpl*, RegisterImpl*, RegisterImpl*, RegisterImpl*, Label&)+0xcf8
>> V  [libjvm.so+0x3d731c]  LIR_Assembler::emit_lock(LIR_OpLock*)+0x160
>> 
>> 
>> The problem is in this code:
>> 
>>   if (DiagnoseSyncOnValueBasedClasses != 0) {
>>     load_klass(tmp1, obj); <--- asserts
>>     ldr_u32(tmp1, Address(tmp1, Klass::access_flags_offset()));
>>     tst(tmp1, JVM_ACC_IS_VALUE_BASED_CLASS);
>>     b(slow_case, ne);
>>   }
>> 
>> `tmp1` is `noreg` when `!BiasedLocking`, because `c1_LIRGenerator_arm.cpp` provides it only when `UseBiasedLocking` is enabled:
>> 
>> void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {
>>   ...
>>   // Need a scratch register for biased locking on arm
>>   LIR_Opr scratch = LIR_OprFact::illegalOpr;
>>   if(UseBiasedLocking) {
>>     scratch = new_pointer_register();
>>   } else {
>>     scratch = atomicLockOpr(); // <--- actually illegalOpr
>>   }
>> 
>>   ...
>> 
>>   monitor_enter(obj.result(), lock, hdr, scratch,
>>                 x->monitor_no(), info_for_exception, info);
>> }
>> 
>> The way out is to use `tmp2`, which is the alias for `Rtemp` and always available.
>> 
>> Additional testing:
>>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:+UseBiasedLocking`
>>  - [x] Linux ARM32 `SyncOnValueBasedClassTest`, `-XX:-UseBiasedLocking`
>
> Hi,
> The change is good! I did not notice this Pull Request, I studied the case and came up with exactly the same solution.
> Thanks for fixing this.
> Boris

Thanks! I need a formal Reviewer to ack this :)

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

PR: https://git.openjdk.java.net/jdk/pull/2349