RFR(S): 8031475: Missing oopmap in patching stubs

[Christian Thalinger]

Yes, that looks good.

On May 21, 2014, at 2:40 AM, Nils Eliasson <nils.eliasson at oracle.com> wrote:

> Hi,
> 
> Thanks for the feedback. I found a piece of code that was supposed to make sure the oopmaps for patch stubs was correct but it only matched patching lir_moves. Since lir_checkcast and lir_moves need different checks for if they are patching I added a virtual method and replaced the earlier check.
> 
> Also removed a duplicate method and did a slight renaming for clarity.
> 
> http://cr.openjdk.java.net/~neliasso/8031475/webrev.06/
> 
> Thanks,
> Nils
> 
> On 2014-04-30 15:06, Nils Eliasson wrote:
>> Hi,
>> 
>> I would like some feedback on this change from the c1 experts. It's made in platform dependent code and will be added to the other plattforms as well before submit.
>> 
>> This change fixes a bug that has been observed in testing, and dug up from a core file, but haven't reproduced standalone yet. When patching for checkcast the oop we are casting is not kept in an oopmap during the runtime patching call, a one time chance per run.
>> 
>> The current change is for all the patching stub cases (access_field_id, load_klass_id, load_mirror_id, load_appendix_id) - is that needed? Do you see any potential for breaking anything? It is difficult to trigger a GC at exact this point during a test.
>> 
>> http://cr.openjdk.java.net/~neliasso/8031475/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8031475
>> 
>> Thanks,
>> Nils Eliasson
>> 
>> 
>> Code example:
>> 
>>  0x00007f20c943590c: mov    $0x718d65d38,%rdx  ;   {oop(a 'BeanImpl2')}   // oops to be casted in rdx
>>  0x00007f20c9435916: cmp    $0x0,%rdx
>>  0x00007f20c943591a: je     0x00007f20c9435967   // jump to patching stub
>>  // patch location
>>  0x00007f20c9435920: jmpq   0x00007f20c94359c5  ;   {no_reloc}
>>  0x00007f20c9435925: add    %al,(%rax)
>>  0x00007f20c9435927: add    %al,(%rax)
>>  0x00007f20c9435929: add    %cl,-0x3eb7f786(%rbx)
>>  0x00007f20c943592f: out    %eax,$0x3
>>  // end of patch location
>>  0x00007f20c9435931: cmp    %rbx,%rdi
>>  0x00007f20c9435934: je     0x00007f20c9435967 // A dereference of rdx somewhere here may crash if the oop has moved during a gc
>>  0x00007f20c943593a: mov    0x10(%rbx),%esi
>>  0x00007f20c943593d: cmp    (%rdi,%rsi,1),%rbx
>>  0x00007f20c9435941: je     0x00007f20c9435967
>> 
>>  ...
>> 
>>  ;; PatchingStub slow case
>>  ;;  patch template
>>  0x00007f20c94359b6: mov    $0x0,%rbx          ; {metadata(NULL)}
>>  ;; patch data encoded as movl
>>  0x00007f20c94359c0: mov    $0xa050f00,%eax
>>  ;; patch entry point
>>  0x00007f20c94359c5: callq  0x00007f20c942e3e0  ; OopMap{[32]=Oop off=266}   // rdx not live here, may safepoint on return from runtime call
>>                                                ;*checkcast
>>                                                ; - TestCheckCast::main at 25 (line 24)
>>                                                ;   {runtime_call}
>>  0x00007f20c94359ca: jmpq   0x00007f20c9435920   // back to normal control flow after patching
>> 
>