RFR: 8000489: older builds of hsdis don't work anymore after 6879063

[Krystal Mo]

Hi Yumin,

I ran PrintInterpreter on JDK7u5, JDK7u9 and JDK8b59 on Ubuntu 
12.04/AMD64 and it looked weird: the newline at the end of each 
disassembly block seems missing.
Am I missing something? I'm sure the new build is fresh, just made a 
fresh clone from hsx/hotspot-rt and applied your patch to make the build.

I've uploaded the output I'm seeing with old/new hsdis on JDK7u5 and 
JDK8b59 here for reference:
https://gist.github.com/3975216

And, I agree with John that all 4 combinations should work.

A nitpick:
There seems to be inconsistent usage of booleans in this change, and in 
the original hsdis. Could you make it more consistent?
Side note: Using the int type for the new_line argument is good to me. 
Using bool instead might cause some weird issues crossing the C and C++ 
boundary.

hsdis.c:

line 113 and 135:
   Lower-cased true/false is used here

line 529, init_disassemble_info_from_bfd()
   Capitalized FALSE is used here

P.S. Running "make all" used to work out-of-the-box for me on AMD64, but 
now I have to specify LP64=1 to make it work. Is this expected?

Thanks,
Kris

On 10/30/2012 12:19 AM, Yumin Qi wrote:
> Serguei and Krys
>
>   If you are OK with the second version, I will do integration into 
> hotspot_rt.
> http://cr.openjdk.java.net/~minqi/8000489
>
> Thanks
> Yumin
>
> On 10/24/2012 2:50 PM, serguei.spitsyn at oracle.com wrote:
>>
>> || Yumin,
>>
>> *src/share/tools/hsdis/hsdis.h:
>> *
>>
>> A typo in the comment:
>>
>>    62 /* This is the compatability interface for older version of hotspot */
>>      =>
>>    62 /* This is the compatibility interface for older versions of hotspot */
>>
>> ||*|src|/share/tools/hsdis/hsdis-demo.c:*
>>
>> Would it make sense to change this to avoid using literal name? :
>>
>> 219   printf("Decoding from %p to %p...with decode_instructions_virtual\n", from, to);
>>    =>
>> 219   printf("Decoding from %p to %p...with %s\n", from, to,DECODE_INSTRUCTIONS_NAME);
>>
>>    and
>>
>> 238   printf("Decoding from %p to %p...with old decode_instructions\n", from, to);
>>    =>
>> 238   printf("Decoding from %p to %p...with old %s\n", from, to,DECODE_INSTRUCTIONS);
>>
>> The (raw && xml) is a special case of the (raw) so that the 4 lines:
>>
>>   225   if (raw && xml) {
>>   226     res = (*decode_instructions_v)(from, to, (unsigned char*)from, to - from, simple_handle_event, stdout, NULL, stdout, options);
>>   227   } else if (raw) {
>>   228     res = (*decode_instructions_v)(from, to, (unsigned char*)from, to - from, simple_handle_event, stdout, NULL, stdout, options);
>> can be replaced with just 2 lines:
>>   225   if (raw) {
>>   226     res = (*decode_instructions_v)(from, to, (unsigned char*)from, to - from, simple_handle_event, stdout, NULL, stdout, options);
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 10/24/12 2:15 PM, yumin.qi at oracle.com wrote:
>>> Krystal,
>>>
>>>   Thanks. Updated in same webrev.
>>>   I checked b10, the output looks OK
>>>
>>> /Yumin
>>>
>>> On 10/24/2012 12:24 PM, Krystal Mok wrote:
>>>> Hi Yumin,
>>>>
>>>> minor typos:
>>>> s/instrurctions/instructions/
>>>>
>>>> By the way, I tried to use a build of hsdis plugin after 6879063 in 
>>>> an earlier JDK build (e.g. JDK8b59) and it was working strangely. A 
>>>> rough look at the output looks like it's missing some newlines, I 
>>>> didn't go into the details. Could you please check if this fix 
>>>> makes the hsdis plugin work properly on earlier JDK builds as well?
>>>>
>>>> Thanks,
>>>> Kris
>>>>
>>>> On Thu, Oct 25, 2012 at 1:01 AM, <yumin.qi at oracle.com 
>>>> <mailto:yumin.qi at oracle.com>> wrote:
>>>>
>>>>      Hi, all
>>>>
>>>>       Can I have your codereview of 8000489: older builds of hsdis
>>>>     don't work anymore after 6879063
>>>>       It caused old build broke the disassembler.
>>>>
>>>>       Webrev: http://cr.openjdk.java.net/~minqi/8000489
>>>>     <http://cr.openjdk.java.net/%7Eminqi/8000489>
>>>>
>>>>     Thanks
>>>>     Yumin
>>>>
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20121030/8baf5cbf/attachment-0001.html