[Bug binutils/10288] New: "objdump -D --target=binary -m arm7tdmi" shows non-ARM7TDMI instructions

objdump for ARM7TDMI binaries shows instructions for newer ARM architectures.

It doesn't return "undefined" like it should for these instructions.

--
           Summary: "objdump -D --target=binary -m arm7tdmi" shows non-
                    ARM7TDMI instructions
           Product: binutils
           Version: 2.19
            Status: NEW
          Severity: normal
          Priority: P2
         Component: binutils
        AssignedTo: unassigned at sources dot redhat dot com
        ReportedBy: chris at seberino dot org
                CC: bug-binutils at gnu dot org


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-17 12:22 -------
Created an attachment (id=4003)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4003&action=view)
Use -m to restrict ARM disassembly


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-17 12:24 -------
Hi Chris,

  Thanks for reporting this problem.  Please could you try out the uploaded
patch and let me know how you get on with it.  (Note - you will need to remove
the earlier version of this patch sent to you via the mailing list).

  This version should handle the coprocessor instructions as well as the base
architecture instructions and it now uses the ARM_FEATURE macros to chose the
core architecture and coprocessor extensions.

Cheers
  Nick


--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-18 03:34 -------
You were certainly correct to remove certain coprocessor instructions like ldc2
that only belong on later architectures.

I'm not sure we're allowed to remove *all* coprocessor instructions.

Even though many (most?) fielded ARM7TDMIs don't utilize coprocessors or virtual
floating point instructions, I think they are still recognized by ARM7TDMI.

What was your rationale for remove *all* coprocessor instructions?

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2009-06-18 10:31 -------
Subject: Bug 10288

CVSROOT: /cvs/src
Module name: src
Changes by: nickc@... 2009-06-18 10:31:21

Modified files:
        include        : ChangeLog dis-asm.h
        gas/testsuite  : ChangeLog
        gas/testsuite/gas/arm: align.s copro.d
        opcodes        : ChangeLog arm-dis.c
        binutils       : ChangeLog objdump.c
        binutils/doc   : binutils.texi

Log message:
        PR 10288
        * arm-dis.c (print_insn_coprocessor): Check that a user specified
        ARM architecture supports the matched instruction.
        (print_insn_arm): Likewise.
        (select_arm_features): New function.  Fills in the fields of an
        arm_feature_set structure based on a given arm machine number.
        (print_insn): Initialise an arm_feature_set structure.
       
        * objdump.c (disassemble_bytes): Set the
        USER_SPECIFIED_MACHINE_TYPE flag in the disassemble_info structure
        if the user has invoked the -m switch.
        * doc/binutils.texi: Document the additional behaviour of
        objdump's -m switch for ARM targets.
       
        * dis-asm.h (USER_SPECIFIED_MACHINE_TYPE): New value for the flags
        field of struct disassemble_info.
       
        * gas/arm/align.s: Add labels so that COFF based targets can
        correctly locate THUMB code.
        * gas/arm/copro.d: Do not pass --architecture switch to objdump.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/include/ChangeLog.diff?cvsroot=src&r1=1.450&r2=1.451
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/include/dis-asm.h.diff?cvsroot=src&r1=1.73&r2=1.74
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1492&r2=1.1493
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/align.s.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.7&r2=1.8
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1402&r2=1.1403
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.97&r2=1.98
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/binutils/ChangeLog.diff?cvsroot=src&r1=1.1485&r2=1.1486
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/binutils/objdump.c.diff?cvsroot=src&r1=1.158&r2=1.159
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/binutils/doc/binutils.texi.diff?cvsroot=src&r1=1.147&r2=1.148



--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-18 10:38 -------
Hi Chris,

  Right, first of all, I have checked in the patch along with the changelog
entries below.

  Secondly you asked: "What was your rationale for remove *all* coprocessor
instructions?"  The answer is that I did not.  I just restricted the coprocessor
instruction display to those that are guaranteed to be supported by the given
architecture, as determined by the macros in the include/opcode/arm.h file.  If
the user wants to see all the coprocessor instructions they can invoke the
disassembler with the "-marm" switch.

Cheers
  Nick

include/ChangeLog
2009-06-18  Nick Clifton  <nickc@...>

        * dis-asm.h (USER_SPECIFIED_MACHINE_TYPE): New value for the flags
        field of struct disassemble_info.

opcodes/ChangeLog
2009-06-18  Nick Clifton  <nickc@...>

        PR 10288
        * arm-dis.c (print_insn_coprocessor): Check that a user specified
        ARM architecture supports the matched instruction.
        (print_insn_arm): Likewise.
        (select_arm_features): New function.  Fills in the fields of an
        arm_feature_set structure based on a given arm machine number.
        (print_insn): Initialise an arm_feature_set structure.

binutils/ChangeLog
2009-06-18  Nick Clifton  <nickc@...>

        PR 10288
        * objdump.c (disassemble_bytes): Set the
        USER_SPECIFIED_MACHINE_TYPE flag in the disassemble_info structure
        if the user has invoked the -m switch.
        * doc/binutils.texi: Document the additional behaviour of
        objdump's -m switch for ARM targets.

gas/testsuite/ChangeLog
2009-06-18  Nick Clifton  <nickc@...>

        PR 10288
        * gas/arm/align.s: Add labels so that COFF based targets can
        correctly locate THUMB code.
        * gas/arm/copro.d: Do not pass --architecture switch to objdump.




--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-18 18:33 -------
I think your patch may have done more than you think.  It not only removed
coprocessor instructions that are not supported by ARM7TDMI, but also removed
coprocessor instructions that *are* supported by ARM7TDMI.  For example, I don't
see ldc anymore which *is* supported by ARM7TDMI.  Not it is reported as undefined.

The problem with your suggestion of using the "-marm" switch is that is would
return the problem that existed before...namely, showing coprocessors
instructions for newer architectures that shouldn't be there like ldc2.

--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-19 14:51 -------
Created an attachment (id=4009)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4009&action=view)
Fix coprocessor opcode discrimination


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-19 14:51 -------
Hi Chris,

  I take your point.  Please try out the uploaded additional patch which should
fix this.

Cheers
  Nick


--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |WAITING


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-19 17:50 -------
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


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-22 11:45 -------
Created an attachment (id=4011)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4011&action=view)
Combination patch for pr 10297 and 10288


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-22 11:45 -------
Hi Chris,

  Please try out the newly uploaded, combined patch.

Cheers
  Nick


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-22 19:43 -------
The undefined fix is very nice.  I did find some issues and have appended a
Python script to reproduce...


#==========The Python script============================================

import struct
raw_binary = open("raw_binary", "w")
raw_binary.write(struct.pack("L", 0x4c585ee5))
raw_binary.write(struct.pack("L", 0x01a23597))
raw_binary.write(struct.pack("L", 0x3d9da24e))

#==========The Python script output=====================================

% objdump -D --target=binary -m arm7tdmi raw_binary

raw_binary:     file format binary


Disassembly of section .data:

00000000 <.data>:
   0: 4c585ee5 mrrcmi 14, 14, r5, r8, cr5
   4: 01a23597 strbeq r3, [r2, r7]!
   8: 3d9da24e lfmcc f2, 1, [sp, #312]

#==================Comments======================================

1. mrrcmi is an extended DSP instruction that doesn't belong on ARM7TDMI right?

2. According to my ARM ref manual, strb needs to have bits 4-11 zeroed out which
01a23597 doesn't.  Should this be undefined instead?

3. I can't find lfm in my ARM ref manual.  Googling reveal it to be a floating
point multiple load instruction.  This may be right but I'm not sure since it
isn't documented in ARM book I have.

cs


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-22 21:53 -------
I was thinking a little more about the lfm instruction.  It seems there are
standard coprocessor instruction names on ARM: cdp, ldc, stc, mcr and mrc.

And, because ARM defines optional standard coprocessor setups for common
hardware extensions like floating point, ARM has defined additional *ALIASES*
for aforementioned coprocessor instructions like lfm, fstms, fstmd, fstd, fstd, etc.

I would personally prefer that only the standard coprocessor instruction names
be used (cdp, ldc, stc, mcr and mrc) and the other aliases like floating point
aliases be avoided since we can't guarantee existence of that.

cs

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-24 16:40 -------
Created an attachment (id=4019)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4019&action=view)
Fix a few more disasssembly mistakes


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-24 16:45 -------
Hi Chris,

> 1. mrrcmi is an extended DSP instruction that doesn't belong on ARM7TDMI right?

Right.  This was a typo in the arm-dis.c file.

> 2. According to my ARM ref manual, strb needs to have bits 4-11 zeroed out
which 01a23597 doesn't.  Should this be undefined instead?

Yes it should.  The disassembly patterns were being a bit too generous.


> 3. I can't find lfm in my ARM ref manual.

Neither can I.  As found out it appears to be a variant of the LDC instruction,
but it is not a true alias for it - there is actually a difference syntax to the
instruction and a different bit encoding.  (It uses bits 11 and 22 to encode a
'count' value).

The uploaded patch fixes 1 and 2.  I have to assume that the encoding and
decoding for LFM, and presumably SFM, are correct.  I also assume that they are
defined in one of the proprietary versions of the ARM Architecture Reference
Manual that ARM only gives out to its approved, registered customers.

Cheers
  Nick

PS.  The uploaded patch includes the previous patches for this issue and the
patches for issue 10297.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-24 20:14 -------
mrrc is gone with is good.  strb appears to have gotten worse! I think the new
patch introduced new bugs into strb.  See below.  Also, some hex equivalents
appear to be botched.  See below for that too....

New objdump shows following instructions to be strb but I think they should be
all undefined....

46647659 strbmi r7, [r4], #-105
77c1cdb4 strbvc ip, [r1, #212]
e640361f strb r3, [r0], #-111

Notice following have incorrect or missing hex equivalents...

4c585ee5 ldclmi 14, cr5, [r8], {229} ; 0xfffffc6c
d446399e strble r3, [r6], #-158
11d87ed1 ldrsbne r7, [r8, #225]
44afa697 strtmi sl, [pc], #1687 ; 0xb4
d4bf78b4 ldrtle r7, [pc], #2228 ; 0xf4
bc041350 stclt 3, cr1, [r4], {80} ; 0xfffffec0

chris

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From chris at seberino dot org  2009-06-24 22:32 -------
About lfm and sfm.....these are alternative aliases for floating point
coprocessor instructions along with many others in the ARM docs I've seen.

We can't guarantee that every ARM7TDMI will have a floating point coprocessor so
these aliases will not always apply.

Wouldn't it therefore be safer and make more sense to have objdump just display
the standard coprocessor instruction names?... (i.e. ldc, stc, cdp ?)



--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-25 07:16 -------
Created an attachment (id=4020)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4020&action=view)
Updated version of previous patch


--
           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #4003 is|0                           |1
           obsolete|                            |
Attachment #4009 is|0                           |1
           obsolete|                            |
Attachment #4011 is|0                           |1
           obsolete|                            |
Attachment #4019 is|0                           |1
           obsolete|                            |


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From nickc at redhat dot com  2009-06-25 07:17 -------
Hi Chris,

  Oops, please ignore the previous patch.  I had not checked it thoroughly
enough.  I have uploaded a revised version which should be better.

Cheers
  Nick


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10288

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils