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

[Zoltán Majó]

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
>>>>
>>