Request for reviews (S): 7125136: SIGILL on linux amd64 in gc/ArrayJuggle/Juggle29

[Azeem Jiva]

Nope that's a 5 byte NOP.

Azeem Jiva
@javawithjiva


On 02/14/2012 11:28 AM, Vladimir Kozlov wrote:
> I found that we have already such function fat_nop() but I am not sure 
> it is correct. I am worried that it is 2 nops in 64-bit VM:
>
>   emit_byte(0x66);
>   emit_byte(0x66);
>   emit_byte(0x90);
>   emit_byte(0x66);
>   emit_byte(0x90);
>
> Thanks,
> Vladimir
>
> Vladimir Kozlov wrote:
>> Christian Thalinger wrote:
>>> On Feb 14, 2012, at 4:56 PM, Vladimir Kozlov wrote:
>>>
>>>> Thank you, Christian
>>>>
>>>> We can't use address nop on old cpus (amd before ss2 and intel 
>>>> before p6, see vm_version_x86.cpp).
>>>
>>> Right, I missed that.  It would be a good idea to have an assert or 
>>> guarantee checking UseAddressNop in these methods.
>>
>> I'm thinking now to add new asm function nop_for_patching() which 
>> will generated either addr_nop_5 or stack_bang based on UseAddressNop 
>> flag.
>>
>> I will add asserts into addr_nop_*().
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> -- Chris
>>>
>>>> I will look on masm definition in MachProlog.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 2/14/12 1:08 AM, Christian Thalinger wrote:
>>>>> On Feb 14, 2012, at 5:02 AM, Vladimir Kozlov wrote:
>>>>>
>>>>>> http://cr.openjdk.java.net/~kvn/7125136/webrev
>>>>>>
>>>>>> 7125136: SIGILL on linux amd64 in gc/ArrayJuggle/Juggle29
>>>>>>
>>>>>> Saving RBP register on nmethod's entry broke nmethod's verified 
>>>>>> entry patching when it become non-entrant. There is big comment 
>>>>>> in MachPrologNode::emit() about first instruction which should be 
>>>>>> at lest 5 bytes long. And push(rbp) is one byte instruction. 
>>>>>> VerifyFPU code also broken (first instruction is pushf). The same 
>>>>>> with C1 generated code with VerifyFPU and C1Breakpoint.
>>>>>>
>>>>>> The only reason we did not noticed this until now is stack bang 
>>>>>> instruction is usually generated first and it is big (store to 
>>>>>> stack with big offset). But C2 does not generated it if compiled 
>>>>>> stack frame is small and no calls in compiled method.
>>>>>>
>>>>>> For C2 moved saving EBP after ESP adjustment. And other cleanup 
>>>>>> in prolog code.
>>>>>> For C1 generated stack bang with small offset (-256) first if 
>>>>>> needed.
>>>>> Why are we using bang_stack_with_offset in this case?  How about 
>>>>> addr_nop_5?
>>>>>
>>>>> It would be better if you would define masm at the beginning of 
>>>>> MachPrologNode::emit instead of doing it multiple times in if 
>>>>> clauses.
>>>>>
>>>>> Otherwise this looks good.
>>>>>
>>>>> -- Chris
>>>>>
>>>>>> Verified by examining generated code.
>>>>>>
>>>>>> SPARC code is safe since we generate 1 instruction (load from 0) 
>>>>>> for patching and use signal handler.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120214/60f0c5f5/attachment-0001.html