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

[Vladimir Kozlov]

Yes, you can push changes are small.

Thanks,
Vladimir

On 1/29/15 1:15 AM, Zoltán Majó wrote:
> 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
>>>>>>
>>>>
>>
>