Bug 3134 - objdump incompletely decodes mov disp:32 instruction
Summary: objdump incompletely decodes mov disp:32 instruction
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.17
: P3 minor
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-25 17:13 UTC by Markus Gyger
Modified: 2008-02-27 12:39 UTC (History)
1 user (show)

See Also:
Host: i386-pc-solaris2.10
Target: h8300-elf
Build: i386-pc-solaris2.10
Last reconfirmed:


Attachments
Assembler example to create both documented mov disp:32 encodings (69 bytes, text/plain)
2006-08-25 17:14 UTC, Markus Gyger
Details
Add extra encoding + new gas test (875 bytes, patch)
2008-02-27 12:37 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Gyger 2006-08-25 17:13:03 UTC
objdump doesn't correctly disassemble the alternate coding of
MOV.L ERs, @(d:32, ERd)

From the Notes of Table 2.2 in the 2.4 Instruction Code
section of the Renesas H8S/2600 Series Programming Manual
http://documentation.renesas.com/eng/products/mpumcu/rej09b0139_h8s2600.pdf

"2. Bit 7 of the 4th byte of the MOV.L ERs, @(d:32,ERd) instruction
can be either 1 or 0."

To reproduce:
h8300-elf-as mov.s
h8300-elf-objdump -wd a.out
[...]
   0:   01 00 78 80 6b a0 00 00 00 00   mov.l   er0,@(0x0:32,er0)
   a:   01 00 78 80 6b a0 00 00 00 00   mov.l   er0,@(0x0:32,er0)
  14:   01 00                           .word   H'1,H'0
  16:   78 00 6b a0 00 00 00 00         mov.w   r0,@(0x0:32,er0)

Content of mov.s:
.h8300s
mov er0, @(0:32,er0)
.byte 1, 0, 0x78, 0x80, 0x6b, 0xa0, 0, 0, 0, 0
.byte 1, 0, 0x78, 0x00, 0x6b, 0xa0, 0, 0, 0, 0
Comment 1 Markus Gyger 2006-08-25 17:14:26 UTC
Created attachment 1251 [details]
Assembler example to create both documented mov disp:32 encodings
Comment 2 Denis Vlasenko 2008-02-22 15:04:40 UTC
According to the .pdf, MOV.L ERs,@(d:32,ERd) instruction has this encoding:

[01] [00] [78] [0erd 0] [6B] [A 0ers] [disp:32]
  or
[01] [00] [78] [1erd 0] [6B] [A 0ers] [disp:32]

where "[]" are bytes (or bigger: disp is 4 bytes).

I think it corresponds to this line in include/opcode/h8300.h:

{O (O_MOV, SL), AV_H8H,  6, "mov.l", {{DISP32SRC, RD32, E}},
   {{PREFIX_0100, 0x7, 0x8, B30 | DISPREG, 0x0, 0x6, 0xb, 0x2, RD32, SRC |
DISP32LIST, E}}},

"PREFIX_0100, 0x7, 0x8, B30 | DISPREG..." part directly corresponds to
"[01] [00] [78] [0erd 0]...."

B30 constant seems to mean "bit 3 in the nibble must be low":
B30 =         0x20000000,             /* Bit 3 must be low.   */

So, just removing it (replacing "B30 | DISPREG" by "DISPREG" in that line)
should do the trick. Markus, can you try it?
Comment 3 Markus Gyger 2008-02-22 17:41:35 UTC
Close -- 18 lines up removing B31 works. Thanks, Denis!

--- binutils-2.16.1/include/opcode/h8300.h.orig	Thu Mar  3 11:58:09 2005
+++ binutils-2.16.1/include/opcode/h8300.h	Fri Feb 22 17:31:04 2008
@@ -1518,7 +1518,7 @@ struct h8_opcode h8_opcodes[] = 
   {O (O_MOV, SL), AV_H8SX, 0, "mov.l", {{RS32, DISP2DST,  E}}, {{PREFIX_010, 
B30 | B20 | DISP2DST, 0x6, 0x9, B31 | DSTDISPREG,  RS32, E}}}, 
   {O (O_MOV, SL), AV_H8H,  6, "mov.l", {{RS32, DISP16DST, E}}, {{PREFIX_0100, 
                     0x6, 0xf, B31 | DSTDISPREG,  RS32, DSTDISP16LIST, E}}}, 
   {O (O_MOV, SL), AV_H8SX, 6, "mov.l", {{RS32, DISP32DST, E}}, {{0x7, 0x8, B31
| DSTDISPREG, 0x0,   0x6, 0xb, 0xa,               RS32, DSTDISP32LIST, E}}},
-  {O (O_MOV, SL), AV_H8H,  6, "mov.l", {{RS32, DISP32DST, E}}, {{PREFIX_0100, 
                     0x7, 0x8, B31 | DSTDISPREG, 0x0,   0x6, 0xb, 0xa,         
     RS32, DSTDISP32LIST, E}}},
+  {O (O_MOV, SL), AV_H8H,  6, "mov.l", {{RS32, DISP32DST, E}}, {{PREFIX_0100, 
                     0x7, 0x8, DSTDISPREG, 0x0,   0x6, 0xb, 0xa,              
RS32, DSTDISP32LIST, E}}},
   {O (O_MOV, SL), AV_H8SX, 0, "mov.l", {{RS32, INDEXB16D, E}}, {{PREFIX_0101, 
                     0x6, 0xf, B31 | DSTDISPREG,  RS32, DSTDISP16LIST, E}}}, 
   {O (O_MOV, SL), AV_H8SX, 0, "mov.l", {{RS32, INDEXW16D, E}}, {{PREFIX_0102, 
                     0x6, 0xf, B31 | DSTDISPREG,  RS32, DSTDISP16LIST, E}}}, 
   {O (O_MOV, SL), AV_H8SX, 0, "mov.l", {{RS32, INDEXL16D, E}}, {{PREFIX_0103, 
                     0x6, 0xf, B31 | DSTDISPREG,  RS32, DSTDISP16LIST, E}}}, 
Comment 4 Nick Clifton 2008-02-27 12:37:02 UTC
Created attachment 2294 [details]
Add extra encoding + new gas test
Comment 5 Nick Clifton 2008-02-27 12:39:18 UTC
Hi Markus,

  Thanks for the patch - it works, but it introduces new failures into the gas
h8300 specific tests because it changes the default encoding for that
instruction.  (To the version without the top bit set in the 4th byte).  A
simpler fix is to just add a new line to the h8_opcodes table to accept the
encoding without the top bit set, but leaving in the current (top bit is set)
line.  That way gas does not change its behaviour, but objdump can disassemble
both versions.

  The uploaded patch does this, and it adds a new test to the gas testsuite in
order to make sure that the bug does not reoccur.  I will check this patch in
along with the following changelog entry.

Cheers
  Nick

include/opcode/ChangeLog
2008-02-27  Markus Gyger  <markus+sw@gyger.org>
	    Nick Clifton  <nickc@redhat.com>

	PR 3134
	* h8300.h (h8_opcodes): Add an encoding for a mov.l instruction
	with a 32-bit displacement but without the top bit of the 4th byte
	set.	

gas/testsuite/ChangeLog
2008-02-27  Nick Clifton  <nickc@redhat.com>

	PR 3134
	* gas/h8300/pr3134.s: New test.
	* gas/h8300/pr3134.d: Expected disassembly
	* gas/h8300/h8300.exp: Run the new test.