RFR (XS): 8062950: Bug in locking code when UseOptoBiasInlining is disabled: assert(dmw->is_neutral()) failed: invariant

[David Holmes]

On 6/11/2014 9:13 AM, Christian Thalinger wrote:
> I’m not exactly sure who is our biased locking expert these days but I guess it’s someone from the runtime team.  CC’ing them.

The fact I am responding does not imply I am, or consider myself, such 
an expert. ;-) I think we need to hear from Vladimir and Roland 
concerning the original fix for:

8033805: Move Fast_Lock/Fast_Unlock code from .ad files to macroassembler

Looking at that changeset:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/5292439ef895

it seems that in x86_32.ad we had:

if (UseBiasedLocking) {
   masm.biased_locking_enter(boxReg, objReg, tmpReg, scrReg, false, 
DONE_LABEL, NULL, _counters);
}

which passes "false", but in x86_64.ad we had:

if (UseBiasedLocking && !UseOptoBiasInlining) {
   masm.biased_locking_enter(boxReg, objReg, tmpReg, scrReg, true, 
DONE_LABEL, NULL, _counters);
   masm.movptr(tmpReg, Address(objReg, 0)) ;        // [FETCH]
}

which passes "true" because there was a prior load of the markword into 
tmpReg.

The new code then has the 64-bit version:

if (UseBiasedLocking && !UseOptoBiasInlining) {
   biased_locking_enter(boxReg, objReg, tmpReg, scrReg, true, 
DONE_LABEL, NULL, counters);
}

but not the prior load and hence is incorrect.

So I concur with Martin's suggested fix.

Cheers,
David

>> On Nov 5, 2014, at 7:38 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
>>
>> Hi,
>>
>> we found a bug in MacroAssembler::fast_lock on x86 which shows up when UseOptoBiasInlining is switched off.
>> The problem is that biased_locking_enter is used with swap_reg_contains_mark==true, which is no longer correct after biased_locking_enter was put in front of check for IsInflated.


Thanks,
David

>>
>> Please review
>> http://cr.openjdk.java.net/~goetz/webrevs/8062950-lockBug/webrev.00/ <http://cr.openjdk.java.net/~goetz/webrevs/8062950-lockBug/webrev.00/>
>>
>> Best regards,
>> Martin
>