[9] RFR(S): 8071654: disassembler handles embedded OOPs not uniformly

[Zoltán Majó]

Hi Vladimir,


now that Tobias has also reviewed this change, is it OK to push it?

Thank you!

Best regards,


Zoltan

On 01/28/2015 07:37 PM, Zoltán Majó wrote:
> Hi Vladimir,
>
>
> thank you for the feedback.
>
> On 01/28/2015 07:09 PM, Vladimir Kozlov wrote:
>> Good. Thank you for clarifying.
>> I was confused because bugs says it produce different results. But 
>> you are saying that currently it produces the same output and we need 
>> only to cleanup code which is not used anymore. Right?
>
> that is right. Currently the same output is produced and it would be 
> nice to keep it that way.
>
> I think the bug description was not clear enough, sorry for that.
>
> Thank you and best regards,
>
>
> Zoltan
>
>
>>
>> Thanks,
>> Vladimir
>>
>> On 1/28/15 9:59 AM, Zoltán Majó wrote:
>>> Hi Vladimir,
>>>
>>>
>>> thank you for the feedback.
>>>
>>> On 01/28/2015 06:33 PM, Vladimir Kozlov wrote:
>>>> So this is just clean up since Universe::heap()->is_in(obj->klass() 
>>>> is false after PermGen removal.
>>>
>>> That is right.
>>>
>>>> Now you get only hexadecimal value and no comment when binutils 
>>>> does not indicate VM that the instruction has embedded
>>>> oop. Right or I am wrong?
>>>
>>> No, we still get a comment in that case.
>>>
>>> If binutils does not indicate to the VM that an instruction contains 
>>> an embedded OOP, decode_env::print_address() is not
>>> called. Therefore_nm->embeddedOop_at() is not called either. As a 
>>> result, we don't process the relocation information
>>> for _nm and a hexadecimal value is printed.
>>>
>>> But decode_env::end_insn() is always called right after the current 
>>> instruction has been processed. Therefore,
>>> _nm->print_code_comment_on() is called as well and all relocation 
>>> information (including those with type 'oop_type') are
>>> printed.
>>>
>>> I checked by changing binutils to do some extra VM notifications.
>>>
>>> Please let me know if you think I miss/oversee anything.
>>>
>>> Thank you very much!
>>>
>>> Best regards,
>>>
>>>
>>> Zoltan
>>>
>>>> If I am right, can we do something about that? I agree that we 
>>>> should not replace hexadecimal address but can we
>>>> generate comment too because even if binutils does not report VM 
>>>> can see it: _nm->embeddedOop_at(cur_insn())
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 1/28/15 7:51 AM, Zoltán Majó wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>> please review the following small patch.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8071654
>>>>>
>>>>>
>>>>> Problem: The disassembler has two different ways of handling OOPs 
>>>>> embedded into instructions.
>>>>>
>>>>> 1) Embedded OOPs are printed in hexadecimal within a disassembled 
>>>>> instruction. The comment block following the
>>>>> instruction contains the name of the OOP's class. For example:
>>>>>
>>>>> 0x00007f52ed58e3e4: mov    $0x7f52c4e03d78,%rdi ;   {oop(a 
>>>>> 'java/lang/Class' = 'java/lang/invoke/MethodHandle')}
>>>>>
>>>>> 2) Embedded OOPs are replaced in the disassembled instruction by 
>>>>> the OOP's class name (this functionality was available
>>>>> only before PermGen removal, see JDK-8066438 for more details). 
>>>>> The comment block following the disassembled instruction
>>>>> contains the same information *again* (the name of the OOP's 
>>>>> class). For example:
>>>>>
>>>>> 0x01882738: cmp   edx, a 'sun/dyn/ToGeneric$A2';   {oop(a 
>>>>> 'sun/dyn/ToGeneric$A2')}
>>>>>
>>>>> (output taken from 
>>>>> https://wikis.oracle.com/display/HotSpotInternals/PrintAssembly)
>>>>>
>>>>> Whether an OOP is replaced with its class name depends on the 
>>>>> external binutils library. For some types of instructions
>>>>> (e.g., compares, jumps and calls on x86), binutils indicates to 
>>>>> the VM that there are addresses embedded into the
>>>>> instruction (and then the VM applies Way 1). For some instructions 
>>>>> (e.g., movs on x86), binutils does not indicate to
>>>>> the VM that the instruction contains an embedded address and the 
>>>>> VM applies Way 2.
>>>>>
>>>>>
>>>>> Solution: This patch proposes that the disassembler handles OOPs 
>>>>> in a single way (Way 1): An embedded OOP's is printed
>>>>> in hexadecimal within the instruction; the comment following the 
>>>>> disassembled instruction prints the OOP's class. This
>>>>> way both the OOP as raw address and the OOP's class is available 
>>>>> in the disassembly by default.
>>>>>
>>>>> The patch proposes to not print an OOP's class in 
>>>>> decode_env::print_address() anymore; printing should be done only in
>>>>> nmethod::print_code_comment_on(). Thenmethod::embeddedOop_at 
>>>>> method is not needed any more and is therefore removed.
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~zmajo/8071654/webrev.00/
>>>>>
>>>>> Testing: manual testing, built + minimal tests with JPRT on all 
>>>>> supported platforms.
>>>>>
>>>>> Thank you!
>>>>>
>>>>> Best regards,
>>>>>
>>>>>
>>>>> Zoltan
>>>>>
>>>
>