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

[Nils Eliasson]

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
>