On Aug 27, 2012, at 2:07 PM, Yumin Qi <yumin...@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
> 

Reply via email to