RFR: 6879063: SA should use hsdis for disassembly

[Yumin Qi]

Thanks.

I will modify like that.
By the way, missed one file, will add and resend out the modified version.
make/solaris/makefiles/sa.make

Thanks
Yumin

On 2012/8/27 14:43, Christian Thalinger wrote:
> On Aug 27, 2012, at 2:07 PM, Yumin Qi<yumin.qi at oracle.com>  wrote:
>
>> Hi, all
>>
>>    Can I have you code review of
>>    6879063: SA should use hsdis for disassembly
>>
>>    http://cr.openjdk.java.net/~minqi/6879063
>>
>>    The SA has Java based disassemblers for x86 and sparc but amd64.  Instead of porting to amd64 we should switch over to using hsdis for it like the JVM does.  This requires a new entry point into hsdis, decode_instructions_virtual, which separates the address of the code being disassembled from the buffer containing the code.  The existing uses of decode_instructions have been updated to use the new interface and SA Disassembler has Java native methods that call into hsdis and call back up to Java to perform the disassembly. Also changed makefile for hsdis build for both(i386/amd64).
>>
>>    All the old disassembler logic was deleted since it's incompatible with the new disassembly interface. Also deleted are dbx based SA interface and few other dead files.
>>
>>    Tested by dumping full assembly from core files.
>>
>>    Reviewed-by:
>>    Contributed-by: Tom R (never)
> src/share/tools/hsdis/hsdis.c:
>
> Maybe decode_instructions should call decode_instructions_virtual.
>
> agent/src/share/classes/sun/jvm/hotspot/asm/Disassembler.java:
>
> +            String cpu = VM.getVM().getCPU();
> +            if (cpu.equals("sparc")) {
> +               if (VM.getVM().isLP64()) {
> +                  libname = "hsdis-sparcv9";
> +                  options = "v9only";
> +               } else {
> +                  libname = "hsdis-sparc";
> +               }
> +            } else if (cpu.equals("x86")) {
> +               libname = "hsdis-i386";
> +            } else if (cpu.equals("amd64")) {
> +               libname = "hsdis-amd64";
> +            } else if (cpu.equals("ia64")) {
> +               libname = "hsdis-ia64";
> +            }
>
> Should we use some default libname here like:
>
>    libname = "hsdis-" + cpu;
>
> so we cover other architectures too (and only special-case sparc, x86, ...)?
>
> Otherwise it looks good.
>
> -- Chris
>
>>    Thanks
>>    Yumin Qi
>>