Patchwork arm-dis: Include opcode hex when doing disassembly

login
register
mail settings
Submitter Peter Maydell
Date Jan. 10, 2011, 4:16 p.m.
Message ID <1294676186-12518-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/78168/
State New
Headers show

Comments

Peter Maydell - Jan. 10, 2011, 4:16 p.m.
Enhance the ARM disassembler used for debugging so that it includes
the hex dump of the opcode as well as the symbolic disassembly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is based on meego-qemu commit e548a60c with a change suggested
last time that patch was sent to qemu-devel:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg28258.html
http://www.mail-archive.com/qemu-devel@nongnu.org/msg29235.html
    
I have used GNU-style indent conventions in this change because
the rest of this file consistently does so (being from libopcode
originally).

 arm-dis.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)
Aurelien Jarno - Jan. 10, 2011, 4:49 p.m.
On Mon, Jan 10, 2011 at 04:16:26PM +0000, Peter Maydell wrote:
> Enhance the ARM disassembler used for debugging so that it includes
> the hex dump of the opcode as well as the symbolic disassembly.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is based on meego-qemu commit e548a60c with a change suggested
> last time that patch was sent to qemu-devel:
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg28258.html
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg29235.html
>     
> I have used GNU-style indent conventions in this change because
> the rest of this file consistently does so (being from libopcode
> originally).
> 
>  arm-dis.c |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)

Strangely on arm host, the opcode hex is already included, as shown
below:

| OUT: [size=308]
| 0x01001ec0:  e5974004  ldr      r4, [r7, #4]
| 0x01001ec4:  e1a04804  lsl      r4, r4, #16
| 0x01001ec8:  e1a04824  lsr      r4, r4, #16
| 0x01001ecc:  e1a04404  lsl      r4, r4, #8

Maybe there is just an option to enable to allow that?
Peter Maydell - Jan. 10, 2011, 5:09 p.m.
On 10 January 2011 10:49, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Strangely on arm host, the opcode hex is already included, as shown
> below:
>
> | OUT: [size=308]
> | 0x01001ec0:  e5974004  ldr      r4, [r7, #4]
> | 0x01001ec4:  e1a04804  lsl      r4, r4, #16
> | 0x01001ec8:  e1a04824  lsr      r4, r4, #16
> | 0x01001ecc:  e1a04404  lsl      r4, r4, #8
>
> Maybe there is just an option to enable to allow that?

It looks like that's just an ugly #ifdef in disas.c:disas():
#ifdef __arm__
        /* since data is included in the code, it is better to
           display code data too */
        fprintf(out, "%08x  ", (int)bfd_getl32((const bfd_byte *)pc));
#endif

...so I guess if we commit the patch I submitted we should
just delete that #ifdef.

-- PMM
Aurelien Jarno - Jan. 10, 2011, 5:31 p.m.
On Mon, Jan 10, 2011 at 11:09:28AM -0600, Peter Maydell wrote:
> On 10 January 2011 10:49, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > Strangely on arm host, the opcode hex is already included, as shown
> > below:
> >
> > | OUT: [size=308]
> > | 0x01001ec0:  e5974004  ldr      r4, [r7, #4]
> > | 0x01001ec4:  e1a04804  lsl      r4, r4, #16
> > | 0x01001ec8:  e1a04824  lsr      r4, r4, #16
> > | 0x01001ecc:  e1a04404  lsl      r4, r4, #8
> >
> > Maybe there is just an option to enable to allow that?
> 
> It looks like that's just an ugly #ifdef in disas.c:disas():
> #ifdef __arm__
>         /* since data is included in the code, it is better to
>            display code data too */
>         fprintf(out, "%08x  ", (int)bfd_getl32((const bfd_byte *)pc));
> #endif
> 
> ...so I guess if we commit the patch I submitted we should
> just delete that #ifdef.
> 

Agreed.
Aurelien Jarno - Jan. 12, 2011, 2:13 p.m.
On Mon, Jan 10, 2011 at 11:09:28AM -0600, Peter Maydell wrote:
> On 10 January 2011 10:49, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > Strangely on arm host, the opcode hex is already included, as shown
> > below:
> >
> > | OUT: [size=308]
> > | 0x01001ec0:  e5974004  ldr      r4, [r7, #4]
> > | 0x01001ec4:  e1a04804  lsl      r4, r4, #16
> > | 0x01001ec8:  e1a04824  lsr      r4, r4, #16
> > | 0x01001ecc:  e1a04404  lsl      r4, r4, #8
> >
> > Maybe there is just an option to enable to allow that?
> 
> It looks like that's just an ugly #ifdef in disas.c:disas():
> #ifdef __arm__
>         /* since data is included in the code, it is better to
>            display code data too */
>         fprintf(out, "%08x  ", (int)bfd_getl32((const bfd_byte *)pc));
> #endif
> 
> ...so I guess if we commit the patch I submitted we should
> just delete that #ifdef.
> 

I have applied this patch, and committed another one that removes this
hack.

Patch

diff --git a/arm-dis.c b/arm-dis.c
index af21739..3ece02c 100644
--- a/arm-dis.c
+++ b/arm-dis.c
@@ -4101,6 +4101,30 @@  print_insn_arm (bfd_vma pc, struct disassemble_info *info)
        addresses, since the addend is not currently pc-relative.  */
     pc = 0;
 
+  /* We include the hexdump of the instruction. The format here
+     matches that used by objdump and the ARM ARM (in particular,
+     32 bit Thumb instructions are displayed as pairs of halfwords,
+     not as a single word.)  */
+  if (is_thumb)
+    {
+      if (size == 2)
+	{
+	  info->fprintf_func(info->stream, "%04lx       ",
+			     ((unsigned long)given) & 0xffff);
+	}
+      else
+	{
+	  info->fprintf_func(info->stream, "%04lx %04lx  ",
+			     (((unsigned long)given) >> 16) & 0xffff,
+			     ((unsigned long)given) & 0xffff);
+	}
+    }
+  else
+    {
+      info->fprintf_func(info->stream, "%08lx      ",
+			 ((unsigned long)given) & 0xffffffff);
+    }
+
   printer (pc, info, given);
 
   if (is_thumb)