Bug 10924 - Bug in objdump when disassembling raw armv4t binaries
Summary: Bug in objdump when disassembling raw armv4t binaries
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.21
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-08 23:33 UTC by Chris Seberino
Modified: 2022-07-28 08:28 UTC (History)
2 users (show)

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


Attachments
Add UNPREDICTABLE warning for insns which use Addressing Mode 3 and P == 0 and W == 1 (3.59 KB, patch)
2009-11-10 10:29 UTC, Nick Clifton
Details | Diff
More fixes (3.41 KB, patch)
2009-11-17 17:22 UTC, Nick Clifton
Details | Diff
Do not prnt zero offset for Immediate Offset addressing (2.08 KB, patch)
2009-11-19 14:01 UTC, Nick Clifton
Details | Diff
Cathc PC used in post-indexed addressing (545 bytes, patch)
2009-12-08 17:25 UTC, Nick Clifton
Details | Diff
More tests for unpredictable instructions (3.12 KB, patch)
2009-12-14 16:39 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-11-08 23:33:12 UTC
Instruction 0x002000b0 looks like a strheq instruction but it should be
unpredictable since the P bit is zero and W bit is 1.

Another reason it should be unpredictable is that when W is 1 you can have Rd ==
Rn.  

Perhaps returning undefined for all unpredictables would be a good idea?

That is my suggestion.  

Thoughts?
Comment 1 Nick Clifton 2009-11-10 10:29:23 UTC
Created attachment 4377 [details]
Add UNPREDICTABLE warning for insns which use Addressing Mode 3 and P == 0 and W == 1
Comment 2 Nick Clifton 2009-11-10 10:32:25 UTC
Hi Chris,

  I am planning on applying the uploaded patch to address this issue, but I
would like your feedback on the new behaviour.  With the patch applied your
testcase will disassemble as:

  00000000 <.text>:
     0:   002000b0        strheq  r0, [r0], -r0 <UNPREDICTABLE>

I feel that displaying the disassembled instruction is correct, even though it
is unpredictable, since it is the behaviour of the instruction that cannot be
specified rather than the entire instruction being undefined.

What do you think ?

Cheers
  Nick
Comment 3 Chris Seberino 2009-11-10 17:31:37 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Nov 10, 2009 at 10:32:26AM -0000, nickc at redhat dot com wrote:
>   I am planning on applying the uploaded patch to address this issue, but I
> would like your feedback on the new behaviour.  With the patch applied your
> testcase will disassemble as:
>
>   00000000 <.text>:
>      0:   002000b0        strheq  r0, [r0], -r0 <UNPREDICTABLE>
>
> I feel that displaying the disassembled instruction is correct, even though it
> is unpredictable, since it is the behaviour of the instruction that cannot be
> specified rather than the entire instruction being undefined.

Sounds good.  I would just suggest making the warning a comment so that
the output of objdump still can be run through gas.

e.g.

   00000000 <.text>:
       0:   002000b0        strheq  r0, [r0], -r0 ; UNPREDICTABLE

On another note, do you have any links explaining the hows and whys of
UNPREDICTABLEs?  How did you know "it is the behaviour of the instruction that
cannot be specified rather than the entire instruction being undefined" ?

cs
Comment 4 cvs-commit@gcc.gnu.org 2009-11-11 09:45:09 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-11-11 09:44:46

Modified files:
	opcodes        : ChangeLog arm-dis.c 

Log message:
	PR binutils/10924
	* arm-dis.c (UNPREDICTABLE_INSTRUCTION): New macro.
	(print_insn_arm): Extend %s format control code to check for
	unpredictable addressing modes.  Add support for %S format control
	code which suppresses this check.
	(W_BIT, I_BIT, U_BIT, P_BIT): New macros.
	(WRITEBACK_BIT_SET, IMMEDIATE_BIT_SET, NEGATIVE_BIT_SET,
	PRE_BIT_SET): New macros.
	(print_insn_coprocessor): Use the new macros instead of magic
	constants.
	(print_arm_address): Likewise.
	(pirnt_insn_arm): Likewise.
	(print_insn_thumb32): Likewise.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1492&r2=1.1493
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.109&r2=1.110

Comment 5 Nick Clifton 2009-11-11 09:54:44 UTC
Hi Chris,

> I would just suggest making the warning a comment so that
> the output of objdump still can be run through gas.

Good point - I will make that change.

> On another note, do you have any links explaining the hows and whys 
> of UNPREDICTABLEs? 

Well basically it is what ARM have stated in their Architecture Reference
Manual.  In this particular case for example we had a mode 3 address with the P
bit clear (indicating post- addressing) and also the W bit clear (indicating no
writeback), which does not make any sense.  Why compute an address after the
memory access and then ignore it ?

What it really means is that ARM hardware implementers are free to do whatever
they like in UNPREDICTABLE situations.  So for example in this case the hardware
could assume that post- addressing always implies writeback and ignore the W
bit.  Or it could check the W bit and not update the address register.  Both
implementations would conform to the ARM specification but obviously would
confound any assembly code writers or compiler implementers.

> How did you know "it is the behaviour of the instruction that
> cannot be specified rather than the entire instruction being undefined" ?

What I meant was that the instruction itself is still defined.  (So for example
in the test case you supplied the instruction is still a store-half-word).  But
the exact behaviour of the side-effects of the instruction is undefined.  (So
again in the example the r0 register may or may not be updated with the post
computed address).


I have checked the patch in, but I will leave this issue open for reports of
other UNPREDICTABLE bit patterns.

Cheers
  Nick
Comment 6 Chris Seberino 2009-11-14 23:38:53 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Wed, Nov 11, 2009 at 09:54:45AM -0000, nickc at redhat dot com wrote:
> I have checked the patch in, but I will leave this issue open for reports of
> other UNPREDICTABLE bit patterns.

Nick

OK I tried to find all bugs I could in one pass to make your job easier.

Regarding me helping with writing patches, I'll do it if I need to but it is
enough work just to inspect all this output to find the bugs in the first
place.  I'd be afraid of making a mistake.  Is there a specific file you could
point me to where all this parsing takes place?  I'll have a look.

I assume have unit tests you run your patches through so we know we aren't adding
new bugs as we fix existing ones?  And, I assume you are testing what I say
against the ARM manual so that *I* don't introduce a bug?

...

Here is what I found recently.  BTW, when I give you an example of a bug, it is most
likely found in other instructions.  I'm hoping that your fix ends up
eliminating the whole *class* of bugs.  For example, that last undefined bug
regarding P=0 and W=1 was reported for a store.  It also shows up in ldrsb and ldrh.
I hope you patch nailed those too?

Without further ado....

0x004000b0 strheq r0, [r0], #-0  <--- objdump is missing the "#-0" (see ARM-ARM top of A5-45)

0x004f00b1 strheq r0, [pc], #-1  <--- objdump has r0, [pc, #-1]

0x005fffff ldrsheq pc, [pc], #-255 <--- objdump has pc, [pc, #-255]

0x00500090 <-- should be undefined not ldrbeq

0x006fffbf <-- P=0 so can't be right

0x00700090 bit 26 is zero so can't be ldrbeq...I think it is undefined

0x007fffff ldrsheq pc, [pc, #-255]! <-- objdump is missing the "!" since bit 21=1

0x00cf00b0 strheq r0, [pc], #0 <--- objdump has r0, [pc, #0]  (bit24=0)
  (likewise for 0x00df00b0 and 0x00dfffff)

0x00ffffff ldrsheq pc, [pc, #255] <-- can't be right since P=0

0x0100f000 <-- obdjump say this is a tstpeq....What is tstp? No such thing!

0x01100090 <-- Can't be ldrbeq since bit26 is zero.  I think is undefined

0x0120f096 <-- objdump has "<illegal shifter operand>".  That should be fixed.

0x01300090 <--Can't be ldrbeq since bit26 =0.  I think is undefined.

0x01400000 <-- Should be mrseq not cmpeq since bit 20=0

0x016000b0 strheq r0, [r0, #-0]! <-- objdump has r0, [r0]! which is wrong

cs
Comment 7 cvs-commit@gcc.gnu.org 2009-11-17 17:20:49 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-11-17 17:20:26

Modified files:
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: arch4t-eabi.d arch4t.d archv6t2.d arm7t.d 
	                       inst.d vfma1.d xscale.d 
	opcodes        : ChangeLog arm-dis.c 

Log message:
	* gas/arm/vfma1.d: Only run on ELF based targets.
	
	PR binutils/10924
	* gas/arm/arch4t-eabi.d: Update expected disassembly.
	* gas/arm/arch4t.d: Likewise.
	* gas/arm/archv6t2.d: Likewise.
	* gas/arm/arm7t.d: Likewise.
	* gas/arm/inst.d: Likewise.
	* gas/arm/xscale.d: Likewise.
	
	PR binutils/10924
	* arm-dis.c (arm_opcodes): Add patterns to match undefined LDRB
	instruction variants.  Add pattern for MRS variant that was being
	confused with CMP.
	(arm_decode_shift): Place error message in a comment.
	(print_insn_arm): Note that writing back to the PC is
	unpredictable.
	Only print 'p' variants of cmp/cmn/teq/tst instructions if
	decoding for pre-V6 architectures.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1591&r2=1.1592
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch4t-eabi.d.diff?cvsroot=src&r1=1.2&r2=1.3
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch4t.d.diff?cvsroot=src&r1=1.5&r2=1.6
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/archv6t2.d.diff?cvsroot=src&r1=1.4&r2=1.5
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arm7t.d.diff?cvsroot=src&r1=1.16&r2=1.17
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/inst.d.diff?cvsroot=src&r1=1.18&r2=1.19
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/vfma1.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/xscale.d.diff?cvsroot=src&r1=1.10&r2=1.11
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1502&r2=1.1503
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.112&r2=1.113

Comment 8 Nick Clifton 2009-11-17 17:22:38 UTC
Created attachment 4392 [details]
More fixes
Comment 9 Nick Clifton 2009-11-17 17:25:49 UTC
Hi Chris,

  Right - I have checked in the newly uploaded pr10924_arm_dis.c_patch.2 which
handles all of the new test cases you found.

  With regard to the TSTP instruction, it is a real instruction, but an obsolete
one.  The patch takes care of this as well.

  Let me know if you find any more bad instruction patterns.

Cheers
  Nick
Comment 10 Chris Seberino 2009-11-17 17:57:00 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Nov 17, 2009 at 05:25:50PM -0000, nickc at redhat dot com wrote:
>   Right - I have checked in the newly uploaded pr10924_arm_dis.c_patch.2 which
> handles all of the new test cases you found.
>
>   With regard to the TSTP instruction, it is a real instruction, but an obsolete
> one.  The patch takes care of this as well.
>
>   Let me know if you find any more bad instruction patterns.

Thanks will do.  Glad I can help.  On another note, do you have any tricks to
make me feel confident we aren't introducing bugs?  I don't suppose ARM has a
blessed disassembler we can compare against?  Or, do you have rock solid
comprehensive unit tests against gas?

cs
Comment 11 Nick Clifton 2009-11-18 09:14:16 UTC
Hi Chris,

> On another note, do you have any tricks to make me feel confident we aren't
> introducing bugs?

Tricks no.  But the newly patched disassembler does not introduce any
regressions into the GAS testsuite for any of the ARM based toolchains.  (I
checked).

> I don't suppose ARM has a blessed disassembler we can compare against?

They do have a disassembler, if you are prepared to pay for it.  

If anyone does have access to this tool and notices any discrepancies when
comparing the results against objdump then they are free to report them and I
will be happy to investigate.

>  Or, do you have rock solid comprehensive unit tests against gas?

Rock solid, no.  Reasonable yes.  The gas testsuite does not check every
possible legal and illegal ARM binary pattern, but it does cover a reasonable
range of the instructions.

Cheers
  Nick
 
Comment 12 Daniel Jacobowitz 2009-11-18 16:14:20 UTC
Hi Nick,

Thoms Schwinge noticed a failure in group-relocations.d:

    regexp_diff match failure
    regexp "^    8074:      e1c020d0        ldrd    r2, \[r0\]$"
    line   "    8074:       e1c020d0        ldrd    r2, [r0, #0]"

Is this change really needed?  I think Chris's point was that we have to print
"[r0, #-0]" which is a separate instruction from "[r0, #0]".
Comment 13 Nick Clifton 2009-11-18 17:07:13 UTC
Subject: Re:  Bug in objdump when disassembling raw armv4t
 binaries

Hi Daniel,

> Thoms Schwinge noticed a failure in group-relocations.d:
> 
>     regexp_diff match failure
>     regexp "^    8074:      e1c020d0        ldrd    r2, \[r0\]$"
>     line   "    8074:       e1c020d0        ldrd    r2, [r0, #0]"

Oops, I thought that I had caught all of these.

> Is this change really needed?  I think Chris's point was that we have to print
> "[r0, #-0]" which is a separate instruction from "[r0, #0]".

Actually I think that he was referring to the compulsory presence of the 
offset, even if it is zero, in the pre-indexed form of addressing.

You are correct though, the immediate value is not needed when using 
offset addressing.  I'll create a patch to fix this and post it later.

Cheers
   Nick


Comment 14 Chris Seberino 2009-11-18 18:17:58 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Wed, Nov 18, 2009 at 05:07:14PM -0000, nickc at redhat dot com wrote:
> Actually I think that he was referring to the compulsory presence of the
> offset, even if it is zero, in the pre-indexed form of addressing.

Just to make sure we're clear....my understanding of my ARM manual is that we
need to specify #0 just like any other number.

cs
Comment 15 Nick Clifton 2009-11-19 11:15:22 UTC
Hi Chris,

> Just to make sure we're clear....my understanding of my ARM manual is that 
> we need to specify #0 just like any other number.

Not quite.  There is one form of Addressing Mode 3 where the #0 is optional. 
This is Immediate Offset addressing (see page A5-36 of the ARM ARM that you are
using).

Cheers
  Nick
Comment 16 Nick Clifton 2009-11-19 14:01:58 UTC
Created attachment 4396 [details]
Do not prnt zero offset for Immediate Offset addressing
Comment 17 Nick Clifton 2009-11-19 14:03:45 UTC
Hi Guys,

  Right - I have checked in the third patch in this series.  This restores the
default behaviour of not printing a zero offset if it is used in Immediate
Offset addressing mode.  This also resovles the group-relocations.[sd] linker
testsuite regression reported by Daniel.

Cheers
  Nick
Comment 18 cvs-commit@gcc.gnu.org 2009-11-19 14:07:37 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-11-19 14:07:11

Modified files:
	opcodes        : ChangeLog arm-dis.c 
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: arch4t-eabi.d arch4t.d arm7t.d armv1.d 
	                       wince_inst.d xscale.d 

Log message:
	PR binutils/10924
	* gas/arm/arch4t-eabi.d: Restore previous expected dissambly of
	instructions using Immediate Offset addressing with an offset of
	zero.
	* gas/arm/arch4t.d: Likewise.
	* gas/arm/arm7t.d: Likewise.
	* gas/arm/xscale.d: Likewise.
	* gas/arm/wince-inst.d: Remove 'p' suffix from cmp, cmn, teq and
	tst instructions.
	
	PR binutils/10924
	* arm-dis.c (print_insn_arm): Do not print an offset of zero when
	decoding Immediaate Offset addressing.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1506&r2=1.1507
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.113&r2=1.114
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1596&r2=1.1597
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch4t-eabi.d.diff?cvsroot=src&r1=1.3&r2=1.4
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arch4t.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/arm7t.d.diff?cvsroot=src&r1=1.17&r2=1.18
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/armv1.d.diff?cvsroot=src&r1=1.6&r2=1.7
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/wince_inst.d.diff?cvsroot=src&r1=1.7&r2=1.8
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/xscale.d.diff?cvsroot=src&r1=1.11&r2=1.12

Comment 19 Daniel Jacobowitz 2009-11-22 04:43:11 UTC
Thanks.  I don't see why we need to print a positive zero offset (it's not
ambiguous with anything), although [r0, #0]! is a sufficiently odd addressing
mode that it does seem sensible.
Comment 20 Chris Seberino 2009-11-22 17:12:34 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Sun, Nov 22, 2009 at 04:43:12AM -0000, drow at sources dot redhat dot com wrote:
>
> ------- Additional Comments From drow at sources dot redhat dot com  2009-11-22 04:43 -------
> Thanks.  I don't see why we need to print a positive zero offset (it's not
> ambiguous with anything),

I think Nick is right.  I don't think we have to.  My ARM book seems to agree.

although [r0, #0]! is a sufficiently odd addressing
> mode that it does seem sensible.

Not sure about ! situation yet.

cs
Comment 21 Chris Seberino 2009-11-30 04:55:47 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Nov 10, 2009 at 10:32:26AM -0000, nickc at redhat dot com wrote:
>   I am planning on applying the uploaded patch to address this issue, but I
> would like your feedback on the new behaviour.  With the patch applied your
> testcase will disassemble as:
>
>   00000000 <.text>:
>      0:   002000b0        strheq  r0, [r0], -r0 <UNPREDICTABLE>

You ended up adding a comment for the above unpredictable instruction flagging
it as unpredictable.

There are thousands of other loads and stores you still need to flag as
unpredictable.    What they all have in common is they have W=1 and P=0.
(bits #21 and #24) for addressing mode 3.

Please flag all loads and stores with the following format as unpredictable...

0xX02XXXXX
0xX03XXXXX
0xX06XXXXX
0xX07XXXXX
0xX0aXXXXX
0xX0bXXXXX
0xX0eXXXXX
..etc.

(Notice they all have bit 24 unset and bit 21 set.)

cs
Comment 22 Nick Clifton 2009-12-01 12:07:56 UTC
Hi Chris,

> Please flag all loads and stores with the following format as unpredictable...

A checked a variety of the patterns you suggested and they are all flagged as
unpredictable, so I think that the disassembler is working correctly.

Cheers
  Nick


Comment 23 Chris Seberino 2009-12-01 15:20:25 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Dec 01, 2009 at 12:07:56PM -0000, nickc at redhat dot com wrote:
>
> ------- Additional Comments From nickc at redhat dot com  2009-12-01 12:07 -------
> Hi Chris,
>
> > Please flag all loads and stores with the following format as unpredictable...
>
> A checked a variety of the patterns you suggested and they are all flagged as
> unpredictable, so I think that the disassembler is working correctly.

I tried to apply the patch to binutils-2.20.51 and patch said it looked like
the patch was applied already so I did NOT apply it.

Did I blow it?  Must I apply the patch to another version?

cs
Comment 24 Chris Seberino 2009-12-02 06:41:46 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Dec 01, 2009 at 07:20:14AM -0800, chris@seberino.org wrote:
> I tried to apply the patch to binutils-2.20.51 and patch said it looked like
> the patch was applied already so I did NOT apply it.

See here...

% cd binutils-2.20.51

% patch -p0 < pr10924_arm-dis.c_patch.3
patching file opcodes/arm-dis.c
Reversed (or previously applied) patch detected!  Assume -R? [n]

cs
Comment 25 Nick Clifton 2009-12-02 11:22:15 UTC
Hi Chris,

> I tried to apply the patch to binutils-2.20.51 and patch said it looked like
> the patch was applied already so I did NOT apply it.

Good - I have already checked the patch in.  (I was confident that it was correct).

Cheers
  Nick
Comment 26 Chris Seberino 2009-12-02 19:20:08 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Wed, Dec 02, 2009 at 11:22:15AM -0000, nickc at redhat dot com wrote:
>
> ------- Additional Comments From nickc at redhat dot com  2009-12-02 11:22 -------
> Hi Chris,
>
> > I tried to apply the patch to binutils-2.20.51 and patch said it looked like
> > the patch was applied already so I did NOT apply it.
>
> Good - I have already checked the patch in.  (I was confident that it was correct).

Sorry.  That one was my fault.  Indeed objdump now flags loads and stores as
UNPREDICTABLE that should be.  It appears that it also flags some as
UNPREDICTABLE that are not.  See these 3...

% ./objdump -b binary -m armv4t -D aaa23

test:     file format binary


Disassembly of section .data:

00000000 <.data>:
   0:    004f00b0       strheq  r0, [pc], #0    ; <UNPREDICTABLE>
   4:    005f00b0       ldrheq  r0, [pc], #0    ; <UNPREDICTABLE>

Why are those UNPREDICTABLE?

cs
Comment 27 Nick Clifton 2009-12-03 10:54:50 UTC
Hi Chris,

> See these 3...

There were only 2 instructions in your test case...

>   0:    004f00b0       strheq  r0, [pc], #0    ; <UNPREDICTABLE>
>   4:    005f00b0       ldrheq  r0, [pc], #0    ; <UNPREDICTABLE>
>
> Why are those UNPREDICTABLE?

They are unpredictable because they use the program counter as the address
register.  See page A5-45 of your ARM ARM:

  "Specifying R15 as register Rn has UNPREDICTABLE results."

Cheers
  Nick
Comment 28 Chris Seberino 2009-12-04 19:15:52 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Thu, Dec 03, 2009 at 10:54:51AM -0000, nickc at redhat dot com wrote:
> They are unpredictable because they use the program counter as the address
> register.  See page A5-45 of your ARM ARM:
>
>   "Specifying R15 as register Rn has UNPREDICTABLE results."

Nick

Thanks for being patient with my mistakes.  Between the two of us, we'll catch
each other's mistakes and both end up stronger for it.

OK I think I found a bug on an old fix to redeem myself....

0x000000bf returns strheq r0, [r0], -pc (pc = r15)

ARM ARM say for this situation neither Rn nor Rm can be pc (R15).
Hence, we need an "; <UNPREDICTABLE>" for this case too.

cs

P.S. By the way, I'm using ARM ARM version ARM DDI 0100I which has different
page numbers than your version.  I can send you my PDF if you wish.
Comment 29 Nick Clifton 2009-12-08 17:25:00 UTC
Created attachment 4450 [details]
Cathc PC used in post-indexed addressing
Comment 30 Nick Clifton 2009-12-08 17:30:19 UTC
Hi Chris,
 
  Good catch.  I have uploaded a patch to catch this form of unpredictable
addressing.  You may have some difficulty applying it as I created it from my
local sources which have a second, uncommited, patch applied to the same file.

Cheers
  Nick

> P.S. By the way, I'm using ARM ARM version ARM DDI 0100I

Mine is ARM DDI 0100E.  I would not mind having a copy of 0100I PDF, but what I
would really like is a copy of the latest ARM ISA specification.  But that is
only available to registered ARM customers. :-(

Comment 31 Chris Seberino 2009-12-08 18:25:41 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Tue, Dec 08, 2009 at 05:25:00PM -0000, nickc at redhat dot com wrote:
>
> ------- Additional Comments From nickc at redhat dot com  2009-12-08 17:25 -------
> Created an attachment (id=4450)
>  --> (http://sourceware.org/bugzilla/attachment.cgi?id=4450&action=view)
> Cathc PC used in post-indexed addressing

Will the daily snapshot of binutils contain latest patches always?  Like this one?
Last time I didn't need to apply your patch so I'm wondering if I should just
always use latest snapshot "as is" for new testing.

cs
Comment 32 Daniel Jacobowitz 2009-12-08 19:49:05 UTC
Subject: Re:  Bug in objdump when disassembling raw
 armv4t binaries

On Tue, Dec 08, 2009 at 05:30:21PM -0000, nickc at redhat dot com wrote:
> Mine is ARM DDI 0100E.  I would not mind having a copy of 0100I PDF, but what I
> would really like is a copy of the latest ARM ISA specification.  But that is
> only available to registered ARM customers. :-(

FYI, it's still registration-required, but you don't have to be a
customer to get it; just register with their web portal.

I've found I have to be a little careful with the current (DDI0406B)
specification, as it sometimes is ARMv6/ARMv7-centric; to find out
ways that ARMv5 is different you sometimes have to go back to DDI0100I.

Comment 33 cvs-commit@gcc.gnu.org 2009-12-09 08:38:19 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-12-09 08:38:04

Modified files:
	opcodes        : ChangeLog arm-dis.c 

Log message:
	PR 10924
	* arm-dis.c (print_insn_arm): Mark insns that use the PC in
	post-indexed addressing as unpredictable.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1512&r2=1.1513
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.115&r2=1.116

Comment 34 Chris Seberino 2009-12-10 22:47:41 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

From Dec 10 version of binutils....

I can't see why 0x004000bf is marked UNPREDICTABLE.  I think that is incorrect...

   0:    004000bf       strheq  r0, [r0], #-15  ; <UNPREDICTABLE>

Agreed?

cs
Comment 35 Daniel Jacobowitz 2009-12-10 23:12:27 UTC
Subject: Re:  Bug in objdump when disassembling raw
 armv4t binaries

On Thu, Dec 10, 2009 at 10:47:41PM -0000, chris at seberino dot org wrote:
> I can't see why 0x004000bf is marked UNPREDICTABLE.  I think that is incorrect...
> 
>    0:    004000bf       strheq  r0, [r0], #-15  ; <UNPREDICTABLE>
> 
> Agreed?

Writeback is set, and rN == rT.  From my copy of the docs, that's
unpredictable.

Comment 36 Chris Seberino 2009-12-10 23:33:06 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Thu, Dec 10, 2009 at 11:12:27PM -0000, drow at sources dot redhat dot com wrote:
> Writeback is set, and rN == rT.  From my copy of the docs, that's
> unpredictable.

You are right.  I found where this is hidden in my ARM ARM.  Nice catch.

cs
Comment 37 Chris Seberino 2009-12-11 17:12:14 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

I think instructions like these below should have a comment flagging them as
UNPREDICTABLE....strh can't have Rd == R15.


3c2c0:  0000f0b0        strheq  pc, [r0], -r0

3fc2f8: 000ff0be        strheq  pc, [pc], -lr

cs
Comment 38 Chris Seberino 2009-12-11 17:25:24 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries


I don't think the following is UNPREDICTABLE.  Every load and store that has
Rd == Rn isn't UNPREDICTABLE.  That only applies if the W and P bits are set.
Or, if it is one of the "t" variants like strt.

2fc:    004000bf        strheq  r0, [r0], #-15  ; <UNPREDICTABLE>

cs
Comment 39 Chris Seberino 2009-12-11 17:35:27 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

I don't see why this one is marked UNPREDICTABLE...

42fc:   005010bf     ldrheq r1, [r0], #-15      ; <UNPREDICTABLE>


The P and W bits are both zero which is ok.

cs
Comment 40 drow@false.org 2009-12-11 17:48:59 UTC
Subject: Re:  Bug in objdump when disassembling raw
 armv4t binaries

On Fri, Dec 11, 2009 at 05:25:25PM -0000, chris at seberino dot org wrote:
> I don't think the following is UNPREDICTABLE.  Every load and store that has
> Rd == Rn isn't UNPREDICTABLE.  That only applies if the W and P bits are set.
> Or, if it is one of the "t" variants like strt.
>
> 2fc:    004000bf        strheq  r0, [r0], #-15  ; <UNPREDICTABLE>

My copy of the manual doesn't say this is unpredictable - but it
doesn't say what it means, either.  The valid versions all have P == 1
and/or W == 1.  And the post-indexed encoding is the same as strht
encoding.

I'm not quite sure what that adds up to!

Comment 41 Chris Seberino 2009-12-11 17:58:45 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Fri, Dec 11, 2009 at 05:48:59PM -0000, drow at false dot org wrote:
> My copy of the manual doesn't say this is unpredictable - but it
> doesn't say what it means, either.  The valid versions all have P == 1
> and/or W == 1.  And the post-indexed encoding is the same as strht
> encoding.
>
> I'm not quite sure what that adds up to!

Oh no.  I'll check later but an unspecified instruction would be bad for us.

cs
Comment 42 Chris Seberino 2009-12-11 18:17:32 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Fri, Dec 11, 2009 at 05:48:59PM -0000, drow at false dot org wrote:
>
> ------- Additional Comments From drow at false dot org  2009-12-11 17:48 -------
> Subject: Re:  Bug in objdump when disassembling raw
>  armv4t binaries
>
> On Fri, Dec 11, 2009 at 05:25:25PM -0000, chris at seberino dot org wrote:
> > I don't think the following is UNPREDICTABLE.  Every load and store that has
> > Rd == Rn isn't UNPREDICTABLE.  That only applies if the W and P bits are set.
> > Or, if it is one of the "t" variants like strt.
> >
> > 2fc:    004000bf        strheq  r0, [r0], #-15  ; <UNPREDICTABLE>
>
> My copy of the manual doesn't say this is unpredictable - but it
> doesn't say what it means, either.

I think unspecified should be flagged as UNPREDICTABLE since we can't predict
what it will do.

cs
Comment 43 Chris Seberino 2009-12-13 03:46:54 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Fri, Dec 11, 2009 at 05:48:59PM -0000, drow at false dot org wrote:
> > 2fc:    004000bf        strheq  r0, [r0], #-15  ; <UNPREDICTABLE>
>
> My copy of the manual doesn't say this is unpredictable - but it
> doesn't say what it means, either.  The valid versions all have P == 1
> and/or W == 1.  And the post-indexed encoding is the same as strht
> encoding.

If I'm not mistaken, this instruction as well as the P=W=0 case *is* specified
in the ARM ARM.

I can send you a copy of my version ARM DDI 0100I if you wish.

The instruction above involves addressing mode 3 with immediate post-indexed
case.   That case has P=W=0.

Chris
Comment 44 cvs-commit@gcc.gnu.org 2009-12-14 16:38:38 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-12-14 16:38:23

Modified files:
	opcodes        : ChangeLog arm-dis.c 
	gas/testsuite  : ChangeLog 
Added files:
	gas/testsuite/gas/arm: unpredictable.d unpredictable.s 

Log message:
	PR binutils/10924
	* arm-dis.c (arm_opcodes): Specify %R in cases where using r15
	results in unpredictable behaviour.
	(print_insn_arm): Handle %R.
	
	* gas/arm/unpredictable.s: New test case - checks the disassembly
	of instructions with unpredictable behaviour.
	* gas/arm/unpredictable.d: New file - expected disassembly.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1517&r2=1.1518
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.117&r2=1.118
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1603&r2=1.1604
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/unpredictable.d.diff?cvsroot=src&r1=NONE&r2=1.1
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/unpredictable.s.diff?cvsroot=src&r1=NONE&r2=1.1

Comment 45 Nick Clifton 2009-12-14 16:39:10 UTC
Created attachment 4467 [details]
More tests for unpredictable instructions
Comment 46 Nick Clifton 2009-12-14 16:46:12 UTC
Hi Chris,

  I have checked in another patch (which should be in tomorrow's tarball) to fix
the new cases you found and also to correct the snafu when post indexed
addressing is used with an immediate offset 15.  (The code was checking for a
register number of 15, but forgot to see if register-indexed or
immediate-indexed addressing was being used).

  I have also added a new testcase to the gas testsuite for the ARM, to check
the disassembly of unpredictable instructions.  One thing I found whilst doing
this is that there are several instructions that GAS itself will happily
assemble, even though they have unpredictable behaviour.  (Eg "mrs pc, cpsr"). 
So I guess I am going to have to start fixing gas as well.  *sigh*

  There are also several situations where unpredictable behaviour results from
the same register being used twice in an instruction.  (Eg the MLA, MUL, UMULL
etc instructions).  Currently the disassembler does not pick these up.  I am not
sure how important it is to detect them, but let me know what you think.

Cheers
  Nick
Comment 47 Chris Seberino 2009-12-15 02:20:34 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Mon, Dec 14, 2009 at 04:46:12PM -0000, nickc at redhat dot com wrote:
>   There are also several situations where unpredictable behaviour results from
> the same register being used twice in an instruction.  (Eg the MLA, MUL, UMULL
> etc instructions).  Currently the disassembler does not pick these up.  I am not
> sure how important it is to detect them, but let me know what you think.

I'm not sure what you mean.  Are you saying you don't want to flag all
UNPREDICTABLES? I would think we should flag all UNPREDICTABLEs with a comment.
It helps me check objdump against by custom
disassembler....I need something in the output that is predictable.

Did you catch all the load and store situations that are UNPREDICTABLE when the
same register is used twice I alerted you to in a previous email?

Chris
Comment 48 cvs-commit@gcc.gnu.org 2009-12-17 09:52:32 UTC
Subject: Bug 10924

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-12-17 09:52:18

Modified files:
	opcodes        : ChangeLog arm-dis.c 
	gas            : ChangeLog 
	gas/config     : tc-arm.c 
	gas/testsuite  : ChangeLog 
	gas/testsuite/gas/arm: unpredictable.d unpredictable.s 

Log message:
	PR binutils/10924
	* config/tc-arm.c (do_ldstv4): Do not allow r15 as the destination
	register.
	(do_mrs): Likewise.
	(do_mul): Likewise.
	
	* arm-dis.c: Add support for %<>ru and %<>rU formats to enforce
	unique register numbers.  Extend support for %<>R format to
	thumb32 and coprocessor instructions.
	
	* gas/arm/unpredictable.s: Add more unpredictable instructions.
	* gas/arm/unpredictable.d: Add expected disassemblies.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/ChangeLog.diff?cvsroot=src&r1=1.1532&r2=1.1533
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/opcodes/arm-dis.c.diff?cvsroot=src&r1=1.118&r2=1.119
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/ChangeLog.diff?cvsroot=src&r1=1.4048&r2=1.4049
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/config/tc-arm.c.diff?cvsroot=src&r1=1.420&r2=1.421
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1604&r2=1.1605
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/unpredictable.d.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gas/testsuite/gas/arm/unpredictable.s.diff?cvsroot=src&r1=1.1&r2=1.2

Comment 49 Nick Clifton 2009-12-17 10:01:01 UTC
Hi Chris,

> I'm not sure what you mean.  Are you saying you don't want to flag 
> all UNPREDICTABLES?

Don't mind me, I was just whining...

> Did you catch all the load and store situations that are UNPREDICTABLE 
> when the same register is used twice I alerted you to in a previous email?

I think so.  Please try updating your sources in order to get my latest patch
and then see if you still find any undetected unpredictable instructions. 
(Actually I know that we are not currently detecting many unpredictable THUMB
instructions, but I was going to look at them after we are confident that we
have the ARM instruction set covered).

Cheers
  Nick


Comment 50 Chris Seberino 2009-12-17 16:33:32 UTC
Subject: Re:  Bug in objdump when disassembling raw
	armv4t binaries

On Thu, Dec 17, 2009 at 10:01:02AM -0000, nickc at redhat dot com wrote:
> Don't mind me, I was just whining...

I understand.  This is a little tedious.  I commend you for hanging on this
far.  It does take a lot of work on your part to fix other peoples' code.  It
is a big responsibility since so many people use binutils.  You are doing a lot
of hard work.  There is no denying that.  Anything I can do to motivate you to
hang on?

> > Did you catch all the load and store situations that are UNPREDICTABLE
> > when the same register is used twice I alerted you to in a previous email?
>
> I think so.  Please try updating your sources in order to get my latest patch
> and then see if you still find any undetected unpredictable instructions.

I assume if I wait until Friday that it will have the patch included already?

> (Actually I know that we are not currently detecting many unpredictable THUMB
> instructions, but I was going to look at them after we are confident that we
> have the ARM instruction set covered).

Yes me too.  I'll attack Thumb after ARM (32 bit) is all done.  I'm hoping the
loads and stores are the hardest part.   We should be almost done with that.
Hopefully things will go a lot faster now.  *knocking on wood*

cs
Comment 51 Alan Modra 2022-07-28 08:28:23 UTC
.