Bug 10297 - inconsistencies in objdump's presentation of undefined's and comments
Summary: inconsistencies in objdump's presentation of undefined's and comments
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.19
: P2 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-18 18:47 UTC by Chris Seberino
Modified: 2009-06-29 08:33 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Use a consistent descriptive string for undefined instructions (954 bytes, patch)
2009-06-19 13:53 UTC, Nick Clifton
Details | Diff
Add comments for immediate values outside of the range +-32. (21.18 KB, patch)
2009-06-23 12:19 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Seberino 2009-06-18 18:47:59 UTC
Here is a Python script that creates a raw binary and runs objdump on it to
reproduce the problem with the output following...

# ========================================================================
import commands
raw_binary = open("raw_binary", "w")
raw_binary.write(chr(0x1f) + chr(0x19) + chr(0x42) + chr(0xc3))
raw_binary.write(chr(0x6d) + chr(0xe8) + chr(0x6e) + chr(0x50))
raw_binary.write(chr(0x17) + chr(0x23) + chr(0xa9) + chr(0xb7))
raw_binary.write(chr(0x77) + chr(0xce) + chr(0x1b) + chr(0xf9))
raw_binary.close()
print commands.getoutput("objdump -D --target=binary -m arm7tdmi raw_binary")
# ========================================================================

% python foo.py 

raw_binary:     file format binary


Disassembly of section .data:

00000000 <.data>:
   0:	c342191f 	cmpgt	r2, #507904	; 0x7c000
   4:	506ee86d 	rsbpl	lr, lr, sp, ror #16
   8:	b7a92317 	undefined
   c:	f91bce77 	undefined instruction 0xf91bce77

-----

Notice that the first 2 instructions have a immediate value but only the first
once has a comment with the hex equivalent.  

Also, notice the last 2 instructions are both undefined's yet only the last one
repeats the hex representation of the instruction.
Comment 1 Nick Clifton 2009-06-19 13:53:08 UTC
Created attachment 4008 [details]
Use a consistent descriptive string for undefined instructions
Comment 2 Nick Clifton 2009-06-19 13:56:57 UTC
Hi Chris,

  The first 'discrepancy' is deliberate.  The immediate value in a shifted
operand can only be in the range 0 - 31 so there is no benefit in providing a
hexadecimal equivalent.  When an immediate value is the entire operand however
it can have a much wider range and so it is helpful to display the hexadecimal
equivalent of the decimal number.

  The second discrepancy is just that.  Please could you try out the uploaded
patch which should fix the problem, giving the disassembler a consistent message
for undefined instructions.

Cheers
  Nick
Comment 3 Chris Seberino 2009-06-19 17:50:46 UTC
I can't apply patch from bug #10288 and bug #10297 at same time.
They crash into each other when you try to apply both of them.

Can you make a patch that includes both fixes?

chris
Comment 4 Chris Seberino 2009-06-22 22:50:04 UTC
You said that the hex equivalent should be displayed for all immediate values
that are greater than 5 bits.  I found some cases where larger immediate values
do not have the hex equivalent value shown. 

For example, loads and stores use addressing mode 2 which has 12 bit immediate
values. Here are 2 stores (str) that don't have hex equivalents...

   8:	14a4ff06 	strtne	pc, [r4], #3846
  18:	858028dd 	strhi	r2, [r0, #2269]

Here are 2 coprocessor loads and stores (addressing mode 5) with 8 bit
immediates that don't have hex equivalents...

  3c:	4d7e6b9f 	ldclmi	11, cr6, [lr, #-636]!
 174:	0ca40a81 	stceq	10, cr0, [r4], #516
 17c:	9d01082f 	stcls	8, cr0, [r1, #-188]

cs
Comment 5 Nick Clifton 2009-06-23 12:19:24 UTC
Created attachment 4016 [details]
Add comments for immediate values outside of the range +-32.
Comment 6 Nick Clifton 2009-06-23 12:20:29 UTC
Hi Chris,

   OK, please try out this new patch.  (You will need to remove the old one
first).  The patch also includes the (current set of) changes for 10288.

Cheers
  Nick
Comment 7 Chris Seberino 2009-06-23 17:43:54 UTC
It looks like a file named gas/testsuite/gas/arm/arm-it-auto-2.d is missing from
binutils-2.19.51 so I can't apply the patch.  What version did you apply this
patch against?

cs
Comment 8 Nick Clifton 2009-06-24 14:04:05 UTC
Subject: Re:  inconsistencies in objdump's presentation
 of	undefined's and comments

Hi Chris,

> It looks like a file named gas/testsuite/gas/arm/arm-it-auto-2.d is missing from
> binutils-2.19.51 so I can't apply the patch.  What version did you apply this
> patch against?

My fault - I failed to check that file in when I was applying another 
patch.  This should be corrected now, so just update your sources.

Cheers
   Nick


Comment 9 Sourceware Commits 2009-06-29 08:08:32 UTC
Subject: Bug 10297

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-06-29 08:08:15

Modified files:
	opcodes        : ChangeLog arm-dis.c 
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: arch6zk.d arch7.d arm-it-auto-2.d 
	                       arm-it-auto.d copro.d float.d fpa-mem.d 
	                       group-reloc-ldc.d group-reloc-ldr.d 
	                       iwmmxt.d maverick.d neon-omit.d svc.d 
	                       thumb-eabi.d thumb.d thumb1_unified.d 
	                       thumb2_add.d thumb2_relax.d thumb32.d 
	                       vfp-neon-syntax.d vfp-neon-syntax_t2.d 
	                       vfp1xD.d vfp1xD_t2.d vfpv3-const-conv.d 
	                       xscale.d 
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-arm: arm-app-abs32.d arm-app.d arm-lib-plt32.d 
	                     arm-lib.d arm-pic-veneer.d armthumb-lib.d 
	                     farcall-mixed-app-v5.d farcall-mixed-app.d 
	                     farcall-mixed-lib.d group-relocs.d 
	                     mixed-app-v5.d mixed-app.d mixed-lib.d 
	                     thumb2-bl-undefweak.d 

Log message:
	PR 10288
	* arm-dis.c (enum opcode_sentinels): New:  Used to mark the
	boundary between variaant and generic coprocessor instuctions.
	(coprocessor): Use it.
	Fix architecture version of MCRR and MRRC instructions.
	(arm_opcdes): Fix patterns for STRB and STRH instructions.
	(print_insn_coprocessor): Check architecture and extension masks.
	Print a hexadecimal version of any decimal constant that is
	outside of the range of -16 to +32.
	(print_arm_address): Add a return value of the offset used in the
	adress, if it is worth printing a hexadecimal version of it.
	(print_insn_neon): Print a hexadecimal version of any decimal
	constant that is outside of the range of -16 to +32.
	(print_insn_arm): Likewise.
	(print_insn_thumb16): Likewise.
	(print_insn_thumb32): Likewise.
	
	PR 10297
	* arm-dis.c (UNDEFINED_INSTRUCTION): New macro for a description
	of an undefined instruction.
	(arm_opcodes): Use it.
	(thumb_opcod): Use it.
	(thumb32_opc): Use it.
	
	Update expected disassembly regrexps in GAS and LD testsuites.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1409&r2=1.1410
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.98&r2=1.99
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1496&r2=1.1497
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch6zk.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch7.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arm-it-auto-2.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arm-it-auto.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/copro.d.diff?cvsroot=src&r1=1.8&r2=1.9
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/float.d.diff?cvsroot=src&r1=1.2&r2=1.3
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/fpa-mem.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/group-reloc-ldc.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/group-reloc-ldr.d.diff?cvsroot=src&r1=1.3&r2=1.4
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/iwmmxt.d.diff?cvsroot=src&r1=1.7&r2=1.8
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/maverick.d.diff?cvsroot=src&r1=1.7&r2=1.8
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/neon-omit.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/svc.d.diff?cvsroot=src&r1=1.3&r2=1.4
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb-eabi.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb.d.diff?cvsroot=src&r1=1.11&r2=1.12
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb1_unified.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb2_add.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb2_relax.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/thumb32.d.diff?cvsroot=src&r1=1.30&r2=1.31
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfp-neon-syntax.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfp-neon-syntax_t2.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfp1xD.d.diff?cvsroot=src&r1=1.8&r2=1.9
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfp1xD_t2.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfpv3-const-conv.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/xscale.d.diff?cvsroot=src&r1=1.8&r2=1.9
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1118&r2=1.1119
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/arm-app-abs32.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/arm-app.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/arm-lib-plt32.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/arm-lib.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/arm-pic-veneer.d.diff?cvsroot=src&r1=1.3&r2=1.4
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/armthumb-lib.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/farcall-mixed-app-v5.d.diff?cvsroot=src&r1=1.2&r2=1.3
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/farcall-mixed-app.d.diff?cvsroot=src&r1=1.2&r2=1.3
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/farcall-mixed-lib.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/group-relocs.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/mixed-app-v5.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/mixed-app.d.diff?cvsroot=src&r1=1.9&r2=1.10
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/mixed-lib.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-arm/thumb2-bl-undefweak.d.diff?cvsroot=src&r1=1.1&r2=1.2

Comment 10 Nick Clifton 2009-06-29 08:33:49 UTC
Hi Chris,

  Right, I have checked in the patch for this issue and I am going to close it
for now. (Keeping track of two competeing PRs is annoying).  If you wish you can
of course re-open this PR, but I hope that will be happy with the current state
of the disassembler.

Cheers
  Nick