* [PATCH] i386-dis: fix decoding of excess prefixes on push segment register
@ 2012-08-06 21:09 Roland McGrath
2012-08-06 21:19 ` H.J. Lu
0 siblings, 1 reply; 5+ messages in thread
From: Roland McGrath @ 2012-08-06 21:09 UTC (permalink / raw)
To: binutils
objdump produces:
0: 0f a8 pushq %gs
2: 41 0f a8 pushq %bx,%di
The rex.B (0x41) prefix is ignored by the hardware.
The exact failure mode may differ in different builds. The bogon arises
from reading off the end of the names_seg[] array, where what happens to
follow is the index16[] array.
After fixing that, I noticed more bugs in the separate printing of excess
prefixes and fixed those too.
With these fixes, it produces:
0: 0f a8 pushq %gs
2: 41 0f a8 rex.B pushq %gs
5: 48 0f a8 rex.W pushq %gs
8: 66 48 0f a8 data32 rex.W pushq %gs
c: 48 rex.W
d: 41 0f a8 rex.B pushq %gs
10: 66 48 data16 rex.W
12: 41 0f a8 rex.B pushq %gs
I don't really understand what determines whether excess prefixes are
printed as a separate line or before the instruction, and what
distinguishes data32 from data16. But at least the distinction between
what is the actual instruction and what are the unused prefixes is now
correct.
There are no other 'make check' regressions for x86_64-linux-gnu.
Ok for trunk?
Thanks,
Roland
gas/testsuite/
* gas/i386/x86-64-stack.s: Add cases for push segment register.
* gas/i386/x86-64-stack.d: Updated.
* gas/i386/x86-64-stack-suffix.d: Updated.
* gas/i386/x86-64-stack-intel.d: Updated.
* gas/i386/ilp32/x86-64-stack.d: Updated.
* gas/i386/ilp32/x86-64-stack-suffix.d: Updated.
* gas/i386/ilp32/x86-64-stack-intel.d: Updated.
opcodes/
* i386-dis.c (print_insn): Print spaces between multiple excess
prefixes. Return actual number of excess prefixes consumed,
not always one.
* i386-dis.c (OP_REG): Ignore REX_B for segment register cases.
diff --git a/gas/testsuite/gas/i386/ilp32/x86-64-stack-intel.d b/gas/testsuite/gas/i386/ilp32/x86-64-stack-intel.d
index fbcada2..f98b8cd 100644
--- a/gas/testsuite/gas/i386/ilp32/x86-64-stack-intel.d
+++ b/gas/testsuite/gas/i386/ilp32/x86-64-stack-intel.d
@@ -56,5 +56,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 add eax,DWORD PTR \[rax\+rcx\*2\]
[ ]*[a-f0-9]+: 68 01 02 03 04 push 0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W push 0x4030201
+[ ]*[a-f0-9]+: 0f a8 push gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W push gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W push gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/ilp32/x86-64-stack-suffix.d b/gas/testsuite/gas/i386/ilp32/x86-64-stack-suffix.d
index dd2f3da..cecab6d 100644
--- a/gas/testsuite/gas/i386/ilp32/x86-64-stack-suffix.d
+++ b/gas/testsuite/gas/i386/ilp32/x86-64-stack-suffix.d
@@ -56,5 +56,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 addl \(%rax,%rcx,2\),%eax
[ ]*[a-f0-9]+: 68 01 02 03 04 pushq \$0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W pushq \$0x4030201
+[ ]*[a-f0-9]+: 0f a8 pushq %gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw %gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W pushq %gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W pushq %gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw %gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/ilp32/x86-64-stack.d b/gas/testsuite/gas/i386/ilp32/x86-64-stack.d
index 9f4553a..fd649e2 100644
--- a/gas/testsuite/gas/i386/ilp32/x86-64-stack.d
+++ b/gas/testsuite/gas/i386/ilp32/x86-64-stack.d
@@ -56,5 +56,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 add \(%rax,%rcx,2\),%eax
[ ]*[a-f0-9]+: 68 01 02 03 04 pushq \$0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W pushq \$0x4030201
+[ ]*[a-f0-9]+: 0f a8 pushq %gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw %gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W pushq %gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W pushq %gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw %gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-stack-intel.d b/gas/testsuite/gas/i386/x86-64-stack-intel.d
index cb9ee89..1902337 100644
--- a/gas/testsuite/gas/i386/x86-64-stack-intel.d
+++ b/gas/testsuite/gas/i386/x86-64-stack-intel.d
@@ -56,5 +56,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 add eax,DWORD PTR \[rax\+rcx\*2\]
[ ]*[a-f0-9]+: 68 01 02 03 04 push 0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W push 0x4030201
+[ ]*[a-f0-9]+: 0f a8 push gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W push gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W push gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B push gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-stack-suffix.d b/gas/testsuite/gas/i386/x86-64-stack-suffix.d
index a0b94d0..1681d79 100644
--- a/gas/testsuite/gas/i386/x86-64-stack-suffix.d
+++ b/gas/testsuite/gas/i386/x86-64-stack-suffix.d
@@ -56,5 +56,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 addl \(%rax,%rcx,2\),%eax
[ ]*[a-f0-9]+: 68 01 02 03 04 pushq \$0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W pushq \$0x4030201
+[ ]*[a-f0-9]+: 0f a8 pushq %gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw %gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W pushq %gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W pushq %gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw %gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-stack.d b/gas/testsuite/gas/i386/x86-64-stack.d
index 76f7151..760d769 100644
--- a/gas/testsuite/gas/i386/x86-64-stack.d
+++ b/gas/testsuite/gas/i386/x86-64-stack.d
@@ -55,5 +55,15 @@ Disassembly of section .text:
[ ]*[a-f0-9]+: 03 04 48 add \(%rax,%rcx,2\),%eax
[ ]*[a-f0-9]+: 68 01 02 03 04 pushq \$0x4030201
[ ]*[a-f0-9]+: 66 48 68 01 02 03 04 data32 rex.W pushq \$0x4030201
+[ ]*[a-f0-9]+: 0f a8 pushq %gs
+[ ]*[a-f0-9]+: 66 0f a8 pushw %gs
+[ ]*[a-f0-9]+: 48 0f a8 rex.W pushq %gs
+[ ]*[a-f0-9]+: 66 48 0f a8 data32 rex.W pushq %gs
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 41 0f a8 rex.B pushw %gs
+[ ]*[a-f0-9]+: 48 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
+[ ]*[a-f0-9]+: 66 48 data16 rex.W
+[ ]*[a-f0-9]+: 41 0f a8 rex.B pushq %gs
[ ]*[a-f0-9]+: 90 nop
#pass
diff --git a/gas/testsuite/gas/i386/x86-64-stack.s b/gas/testsuite/gas/i386/x86-64-stack.s
index 2da658b..0b8707e 100644
--- a/gas/testsuite/gas/i386/x86-64-stack.s
+++ b/gas/testsuite/gas/i386/x86-64-stack.s
@@ -29,6 +29,11 @@ _start:
# push with a 4-byte immediate
try 0x68, 0x01, 0x02, 0x03, 0x04
+ # push a segment register
+ try 0x0f, 0xa8
+ # with extraneous rex.B
+ try 0x41, 0x0f, 0xa8
+
# This is just to synchronize the disassembly.
# Any new cases must come before this line!
nop
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 43d7ac3..da5ede5 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -11450,9 +11450,10 @@ print_insn (bfd_vma pc, disassemble_info *info)
for (i = 0;
i < (int) ARRAY_SIZE (all_prefixes) && all_prefixes[i];
i++)
- (*info->fprintf_func) (info->stream, "%s",
+ (*info->fprintf_func) (info->stream, "%s%s",
+ i == 0 ? "" : " ",
prefix_name (all_prefixes[i], sizeflag));
- return 1;
+ return i;
}
insn_codep = codep;
@@ -13471,6 +13472,15 @@ OP_REG (int code, int sizeflag)
{
const char *s;
int add;
+
+ switch (code)
+ {
+ case es_reg: case ss_reg: case cs_reg:
+ case ds_reg: case fs_reg: case gs_reg:
+ oappend (names_seg[code - es_reg]);
+ return;
+ }
+
USED_REX (REX_B);
if (rex & REX_B)
add = 8;
@@ -13483,10 +13493,6 @@ OP_REG (int code, int sizeflag)
case sp_reg: case bp_reg: case si_reg: case di_reg:
s = names16[code - ax_reg + add];
break;
- case es_reg: case ss_reg: case cs_reg:
- case ds_reg: case fs_reg: case gs_reg:
- s = names_seg[code - es_reg + add];
- break;
case al_reg: case ah_reg: case cl_reg: case ch_reg:
case dl_reg: case dh_reg: case bl_reg: case bh_reg:
USED_REX (0);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i386-dis: fix decoding of excess prefixes on push segment register
2012-08-06 21:09 [PATCH] i386-dis: fix decoding of excess prefixes on push segment register Roland McGrath
@ 2012-08-06 21:19 ` H.J. Lu
2012-08-06 21:24 ` Roland McGrath
0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2012-08-06 21:19 UTC (permalink / raw)
To: Roland McGrath; +Cc: binutils
On Mon, Aug 6, 2012 at 2:01 PM, Roland McGrath <mcgrathr@google.com> wrote:
> objdump produces:
>
> 0: 0f a8 pushq %gs
> 2: 41 0f a8 pushq %bx,%di
>
> The rex.B (0x41) prefix is ignored by the hardware.
>
> The exact failure mode may differ in different builds. The bogon arises
> from reading off the end of the names_seg[] array, where what happens to
> follow is the index16[] array.
>
> After fixing that, I noticed more bugs in the separate printing of excess
> prefixes and fixed those too.
>
> With these fixes, it produces:
>
> 0: 0f a8 pushq %gs
> 2: 41 0f a8 rex.B pushq %gs
> 5: 48 0f a8 rex.W pushq %gs
> 8: 66 48 0f a8 data32 rex.W pushq %gs
> c: 48 rex.W
> d: 41 0f a8 rex.B pushq %gs
> 10: 66 48 data16 rex.W
> 12: 41 0f a8 rex.B pushq %gs
>
> I don't really understand what determines whether excess prefixes are
> printed as a separate line or before the instruction, and what
> distinguishes data32 from data16. But at least the distinction between
> what is the actual instruction and what are the unused prefixes is now
> correct.
>
> There are no other 'make check' regressions for x86_64-linux-gnu.
>
> Ok for trunk?
>
>
> Thanks,
> Roland
>
...
> opcodes/
> * i386-dis.c (print_insn): Print spaces between multiple excess
> prefixes.
Does your patch contain a testcase for this change?
--
H.J.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i386-dis: fix decoding of excess prefixes on push segment register
2012-08-06 21:19 ` H.J. Lu
@ 2012-08-06 21:24 ` Roland McGrath
2012-08-06 22:21 ` H.J. Lu
0 siblings, 1 reply; 5+ messages in thread
From: Roland McGrath @ 2012-08-06 21:24 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Mon, Aug 6, 2012 at 2:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Does your patch contain a testcase for this change?
That's what the additions to x86-64-stack.s are.
Thanks,
Roland
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i386-dis: fix decoding of excess prefixes on push segment register
2012-08-06 21:24 ` Roland McGrath
@ 2012-08-06 22:21 ` H.J. Lu
2012-08-06 22:36 ` Roland McGrath
0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2012-08-06 22:21 UTC (permalink / raw)
To: Roland McGrath; +Cc: binutils
On Mon, Aug 6, 2012 at 2:18 PM, Roland McGrath <mcgrathr@google.com> wrote:
> On Mon, Aug 6, 2012 at 2:09 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Does your patch contain a testcase for this change?
>
> That's what the additions to x86-64-stack.s are.
>
OK.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i386-dis: fix decoding of excess prefixes on push segment register
2012-08-06 22:21 ` H.J. Lu
@ 2012-08-06 22:36 ` Roland McGrath
0 siblings, 0 replies; 5+ messages in thread
From: Roland McGrath @ 2012-08-06 22:36 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
Committed.
Thanks,
Roland
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-08-06 22:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-06 21:09 [PATCH] i386-dis: fix decoding of excess prefixes on push segment register Roland McGrath
2012-08-06 21:19 ` H.J. Lu
2012-08-06 21:24 ` Roland McGrath
2012-08-06 22:21 ` H.J. Lu
2012-08-06 22:36 ` Roland McGrath
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).