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

[Nils Eliasson]

answer inline.

On 2014-04-30 21:17, Christian Thalinger wrote:
>
> On Apr 30, 2014, at 3:06 AM, Nils Eliasson <nils.eliasson at oracle.com 
> <mailto:nils.eliasson at oracle.com>> 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? 
>
> This looks reasonable.  What you are saying is that at:
>
>  0x00007f20c94359c5: callq  0x00007f20c942e3e0  ; OopMap{[32]=Oop off=266}
>
> the OopMap does not contain the object (in this case in rdx) and so is 
> not handled during a GC, correct?

Yes

>
>> Do you see any potential for breaking anything? It is difficult to 
>> trigger a GC at exact this point during a test.
>
> Can’t you trigger a GC inside the runtime call by calling:
>
> Universe::heap()->collect(GCCause::_java_lang_system_gc);
>
> ?

Yes, that did the trick. Now I have a reliable reproducer.

Thank you!

//Nils

>
>>
>> http://cr.openjdk.java.net/~neliasso/8031475/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Eneliasso/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
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140508/d6d44251/attachment.html>