From: Roland McGrath <mcgrathr@google.com>
To: hjl.tools@gmail.com
Cc: Victor Khimenko <khim@google.com>
Cc: binutils@sourceware.org
Subject: [RFC PATCH] Fix decoding of superfluous data32 prefix before superfluous rex.W prefix before push.
Date: Fri, 03 Aug 2012 18:47:00 -0000 [thread overview]
Message-ID: <x57jr4rnalib.fsf@frobland.mtv.corp.google.com> (raw)
x86-64 hardware ignores a superfluous data32 (0x66) prefix that precedes a
superfluous rex.W (0x48) prefix that precedes a push-immediate (0x68)
instruction. But the disassembler gets confused by this:
0: 48 68 01 02 03 04 rex.W pushq $0x4030201
6: 66 48 68 01 02 data32 pushq $0x201
b: 03 .byte 0x3
c: 04 .byte 0x4
With this change it's at least not confused in the decoding:
0: 48 68 01 02 03 04 rex.W pushq $0x4030201
6: 66 48 68 01 02 03 04 data32 pushq $0x4030201
That's the most important thing, since it prevents it losing track of the
instruction boundaries. But I'm not at all sure this is really the best
way to fix that. The i386-dis.c code is extremely hairy and barely
commented.
As of 2.20, it was not so confused and printed:
0: 66 48 68 01 02 03 04 pushq $0x4030201
So this is a regression of sorts.
What would really be ideal is:
0: 48 68 01 02 03 04 rex.W pushq $0x4030201
6: 66 48 68 01 02 03 04 data32 rex.W pushq $0x4030201
i.e., print both superfluous prefixes rather than ignoring either.
But it's not at all clear to me how to make that happen. I also really
have no idea what other cases might be affected by the same sort of problem.
HJ, do you have any insight into all this?
Thanks,
Roland
binutils/testsuite/
2012-08-03 Roland McGrath <mcgrathr@google.com>
* binutils-all/x86-64/data32-pushq-imm.s: New file.
* binutils-all/x86-64/data32-pushq-imm.d: New file.
opcodes/
2012-08-03 Roland McGrath <mcgrathr@google.com>
Victor Khimenko <khim@google.com>
* i386-dis.c (OP_sI): In v_mode, REX_W trumps DFLAG.
diff --git a/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.d b/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.d
new file mode 100644
index 0000000..211b2ca
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.d
@@ -0,0 +1,13 @@
+# This is really a test of the disassembler in objdump, but run_dump_test
+# wants us to specify some other program to run first after assembling.
+#PROG: objcopy
+#as: --64
+#objdump: -d
+
+.*: +file format .*
+
+
+Disassembly of section \.text:
+
+0+ <foo>:
+\s*0: 66 48 68 01 02 03 04 data32 pushq \$0x4030201
diff --git a/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.s b/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.s
new file mode 100644
index 0000000..6d9d39a
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/data32-pushq-imm.s
@@ -0,0 +1,4 @@
+.text
+foo:
+ .byte 0x66
+ rex.W pushq $0x04030201
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 685e968..5f2eb78 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -1,6 +1,6 @@
/* Print i386 instructions for GDB, the GNU debugger.
Copyright 1988, 1989, 1991, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
- 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011
+ 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
Free Software Foundation, Inc.
This file is part of the GNU opcodes library.
@@ -13730,7 +13730,8 @@ OP_sI (int bytemode, int sizeflag)
}
break;
case v_mode:
- if (sizeflag & DFLAG)
+ /* The operand-size prefix is overridden by a REX prefix. */
+ if ((sizeflag & DFLAG) || (rex & REX_W))
op = get32s ();
else
op = get16 ();
next reply other threads:[~2012-08-03 18:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-03 18:47 Roland McGrath [this message]
2012-08-03 18:54 ` H.J. Lu
2012-08-03 19:52 ` H.J. Lu
2012-08-03 21:55 ` Roland McGrath
2012-08-03 22:10 ` H.J. Lu
2012-08-03 23:24 ` Roland McGrath
2012-08-03 23:36 ` H.J. Lu
2012-08-04 0:30 ` H.J. Lu
2012-08-06 20:02 ` Roland McGrath
2012-08-06 20:20 ` H.J. Lu
2012-08-06 20:34 ` Roland McGrath
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=x57jr4rnalib.fsf@frobland.mtv.corp.google.com \
--to=mcgrathr@google.com \
--cc=hjl.tools@gmail.com \
--cc=khim@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).