* [PATCH] x86: correct and simplify NOP disassembly
@ 2022-04-11 16:16 Jan Beulich
2022-05-18 17:44 ` H.J. Lu
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2022-04-11 16:16 UTC (permalink / raw)
To: Binutils
It's not just REX.W which is ignored with opcode 0x90. The same goes for
REX.R and REX.X as well as empty REX. None of these are forms of
"xchg %eax,%eax" (which would mean zero-extending %eax to %rax), so they
also shouldn't be disassembled this way.
While there simplify things: A single hook function suffices, thus
making it unnecessary to keep two expressions in sync. And checking
ins->address_mode for mode_64bit also is unnecessary, as "rex" can be
non-zero only in that case anyway.
--- a/gas/testsuite/gas/i386/ilp32/rex.d
+++ b/gas/testsuite/gas/i386/ilp32/rex.d
@@ -2,46 +2,4 @@
#objdump: -dw
#name: x86-64 (ILP32) manual rex prefix use
#notarget: x86_64-*-elf*
-
-.*: +file format .*
-
-Disassembly of section .text:
-
-0+ <_start>:
-[ ]*[0-9a-f]+:[ ]+40 0f ae 00[ ]+rex fxsave[ ]+\(%rax\)
-[ ]*[0-9a-f]+:[ ]+48 0f ae 00[ ]+fxsave64[ ]+\(%rax\)
-[ ]*[0-9a-f]+:[ ]+41 0f ae 00[ ]+fxsave[ ]+\(%r8\)
-[ ]*[0-9a-f]+:[ ]+49 0f ae 00[ ]+fxsave64[ ]+\(%r8\)
-[ ]*[0-9a-f]+:[ ]+42 0f ae 04 05 00 00 00 00[ ]+fxsave[ ]+(0x0)?\(,%r8(,1)?\)
-[ ]*[0-9a-f]+:[ ]+4a 0f ae 04 05 00 00 00 00[ ]+fxsave64[ ]+(0x0)?\(,%r8(,1)?\)
-[ ]*[0-9a-f]+:[ ]+43 0f ae 04 00[ ]+fxsave[ ]+\(%r8,%r8(,1)?\)
-[ ]*[0-9a-f]+:[ ]+4b 0f ae 04 00[ ]+fxsave64[ ]+\(%r8,%r8(,1)?\)
-[ ]*[0-9a-f]+:[ ]+48 03 04 00[ ]+add[ ]+\(%rax,%rax(,1)?\),%rax
-[ ]*[0-9a-f]+:[ ]+44 03 04 00[ ]+add[ ]+\(%rax,%rax(,1)?\),%r8d
-[ ]*[0-9a-f]+:[ ]+41 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%eax
-[ ]*[0-9a-f]+:[ ]+42 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%eax
-[ ]*[0-9a-f]+:[ ]+49 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%rax
-[ ]*[0-9a-f]+:[ ]+46 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%r8d
-[ ]*[0-9a-f]+:[ ]+45 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%r8d
-[ ]*[0-9a-f]+:[ ]+4a 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%rax
-[ ]*[0-9a-f]+:[ ]+41\s+rex\.B
-[ ]*[0-9a-f]+:[ ]+9b dd 30\s+fsave\s+\(%rax\)
-[ ]*[0-9a-f]+:[ ]+9b 41 dd 30\s+fsave\s+\(%r8\)
-[ ]*[0-9a-f]+:[ ]+40 c5 f9 28 00[ ]+rex vmovapd \(%rax\),%xmm0
-[ ]*[0-9a-f]+:[ ]+40[ ]+rex
-[ ]*[0-9a-f]+:[ ]+41[ ]+rex.B
-[ ]*[0-9a-f]+:[ ]+42[ ]+rex.X
-[ ]*[0-9a-f]+:[ ]+43[ ]+rex.XB
-[ ]*[0-9a-f]+:[ ]+44[ ]+rex.R
-[ ]*[0-9a-f]+:[ ]+45[ ]+rex.RB
-[ ]*[0-9a-f]+:[ ]+46[ ]+rex.RX
-[ ]*[0-9a-f]+:[ ]+47[ ]+rex.RXB
-[ ]*[0-9a-f]+:[ ]+48[ ]+rex.W
-[ ]*[0-9a-f]+:[ ]+49[ ]+rex.WB
-[ ]*[0-9a-f]+:[ ]+4a[ ]+rex.WX
-[ ]*[0-9a-f]+:[ ]+4b[ ]+rex.WXB
-[ ]*[0-9a-f]+:[ ]+4c[ ]+rex.WR
-[ ]*[0-9a-f]+:[ ]+4d[ ]+rex.WRB
-[ ]*[0-9a-f]+:[ ]+4e[ ]+rex.WRX
-[ ]*[0-9a-f]+:[ ]+4f[ ]+rex.WRXB
-#pass
+#dump: ../rex.d
--- a/gas/testsuite/gas/i386/rex.d
+++ b/gas/testsuite/gas/i386/rex.d
@@ -23,6 +23,11 @@ Disassembly of section .text:
[ ]*[0-9a-f]+:[ ]+46 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%r8d
[ ]*[0-9a-f]+:[ ]+45 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%r8d
[ ]*[0-9a-f]+:[ ]+4a 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%rax
+[ ]*[0-9a-f]+:[ ]+40 90[ ]+rex nop
+[ ]*[0-9a-f]+:[ ]+48 90[ ]+rex\.W nop
+[ ]*[0-9a-f]+:[ ]+44 90[ ]+rex\.R nop
+[ ]*[0-9a-f]+:[ ]+42 90[ ]+rex\.X nop
+[ ]*[0-9a-f]+:[ ]+41 90[ ]+xchg[ ]+%eax,%r8d
[ ]*[0-9a-f]+:[ ]+41\s+rex\.B
[ ]*[0-9a-f]+:[ ]+9b dd 30\s+fsave\s+\(%rax\)
[ ]*[0-9a-f]+:[ ]+9b 41 dd 30\s+fsave\s+\(%r8\)
--- a/gas/testsuite/gas/i386/rex.s
+++ b/gas/testsuite/gas/i386/rex.s
@@ -20,6 +20,12 @@ _start:
rex.b add (%rax,%rax), %r8d
rex.x add (%rax,%rax), %rax
+ rex nop
+ rex.w nop
+ rex.r nop
+ rex.x nop
+ rex.b nop
+
.byte 0x41,0x9b,0xdd,0x30
fsave (%r8)
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -98,8 +98,7 @@ static void VPCOM_Fixup (instr_info *, i
static void OP_0f07 (instr_info *, int, int);
static void OP_Monitor (instr_info *, int, int);
static void OP_Mwait (instr_info *, int, int);
-static void NOP_Fixup1 (instr_info *, int, int);
-static void NOP_Fixup2 (instr_info *, int, int);
+static void NOP_Fixup (instr_info *, int, int);
static void OP_3DNowSuffix (instr_info *, int, int);
static void CMP_Fixup (instr_info *, int, int);
static void BadOp (instr_info *);
@@ -2913,9 +2912,9 @@ static const struct dis386 reg_table[][8
static const struct dis386 prefix_table[][4] = {
/* PREFIX_90 */
{
- { "xchgS", { { NOP_Fixup1, eAX_reg }, { NOP_Fixup2, eAX_reg } }, 0 },
+ { "xchgS", { { NOP_Fixup, 0 }, { NOP_Fixup, 1 } }, 0 },
{ "pause", { XX }, 0 },
- { "xchgS", { { NOP_Fixup1, eAX_reg }, { NOP_Fixup2, eAX_reg } }, 0 },
+ { "xchgS", { { NOP_Fixup, 0 }, { NOP_Fixup, 1 } }, 0 },
{ NULL, { { NULL, 0 } }, PREFIX_IGNORED }
},
@@ -12724,25 +12723,14 @@ OP_0f07 (instr_info *ins, int bytemode,
32bit mode and "xchg %rax,%rax" in 64bit mode. */
static void
-NOP_Fixup1 (instr_info *ins, int bytemode, int sizeflag)
+NOP_Fixup (instr_info *ins, int opnd, int sizeflag)
{
- if ((ins->prefixes & PREFIX_DATA) != 0
- || (ins->rex != 0
- && ins->rex != 0x48
- && ins->address_mode == mode_64bit))
- OP_REG (ins, bytemode, sizeflag);
- else
+ if ((ins->prefixes & PREFIX_DATA) == 0 && (ins->rex & REX_B) == 0)
strcpy (ins->obuf, "nop");
-}
-
-static void
-NOP_Fixup2 (instr_info *ins, int bytemode, int sizeflag)
-{
- if ((ins->prefixes & PREFIX_DATA) != 0
- || (ins->rex != 0
- && ins->rex != 0x48
- && ins->address_mode == mode_64bit))
- OP_IMREG (ins, bytemode, sizeflag);
+ else if (opnd == 0)
+ OP_REG (ins, eAX_reg, sizeflag);
+ else
+ OP_IMREG (ins, eAX_reg, sizeflag);
}
static const char *const Suffix3DNow[] = {
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] x86: correct and simplify NOP disassembly
2022-04-11 16:16 [PATCH] x86: correct and simplify NOP disassembly Jan Beulich
@ 2022-05-18 17:44 ` H.J. Lu
0 siblings, 0 replies; 2+ messages in thread
From: H.J. Lu @ 2022-05-18 17:44 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Mon, Apr 11, 2022 at 9:16 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> It's not just REX.W which is ignored with opcode 0x90. The same goes for
> REX.R and REX.X as well as empty REX. None of these are forms of
> "xchg %eax,%eax" (which would mean zero-extending %eax to %rax), so they
> also shouldn't be disassembled this way.
>
> While there simplify things: A single hook function suffices, thus
> making it unnecessary to keep two expressions in sync. And checking
> ins->address_mode for mode_64bit also is unnecessary, as "rex" can be
> non-zero only in that case anyway.
>
> --- a/gas/testsuite/gas/i386/ilp32/rex.d
> +++ b/gas/testsuite/gas/i386/ilp32/rex.d
> @@ -2,46 +2,4 @@
> #objdump: -dw
> #name: x86-64 (ILP32) manual rex prefix use
> #notarget: x86_64-*-elf*
> -
> -.*: +file format .*
> -
> -Disassembly of section .text:
> -
> -0+ <_start>:
> -[ ]*[0-9a-f]+:[ ]+40 0f ae 00[ ]+rex fxsave[ ]+\(%rax\)
> -[ ]*[0-9a-f]+:[ ]+48 0f ae 00[ ]+fxsave64[ ]+\(%rax\)
> -[ ]*[0-9a-f]+:[ ]+41 0f ae 00[ ]+fxsave[ ]+\(%r8\)
> -[ ]*[0-9a-f]+:[ ]+49 0f ae 00[ ]+fxsave64[ ]+\(%r8\)
> -[ ]*[0-9a-f]+:[ ]+42 0f ae 04 05 00 00 00 00[ ]+fxsave[ ]+(0x0)?\(,%r8(,1)?\)
> -[ ]*[0-9a-f]+:[ ]+4a 0f ae 04 05 00 00 00 00[ ]+fxsave64[ ]+(0x0)?\(,%r8(,1)?\)
> -[ ]*[0-9a-f]+:[ ]+43 0f ae 04 00[ ]+fxsave[ ]+\(%r8,%r8(,1)?\)
> -[ ]*[0-9a-f]+:[ ]+4b 0f ae 04 00[ ]+fxsave64[ ]+\(%r8,%r8(,1)?\)
> -[ ]*[0-9a-f]+:[ ]+48 03 04 00[ ]+add[ ]+\(%rax,%rax(,1)?\),%rax
> -[ ]*[0-9a-f]+:[ ]+44 03 04 00[ ]+add[ ]+\(%rax,%rax(,1)?\),%r8d
> -[ ]*[0-9a-f]+:[ ]+41 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%eax
> -[ ]*[0-9a-f]+:[ ]+42 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%eax
> -[ ]*[0-9a-f]+:[ ]+49 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%rax
> -[ ]*[0-9a-f]+:[ ]+46 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%r8d
> -[ ]*[0-9a-f]+:[ ]+45 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%r8d
> -[ ]*[0-9a-f]+:[ ]+4a 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%rax
> -[ ]*[0-9a-f]+:[ ]+41\s+rex\.B
> -[ ]*[0-9a-f]+:[ ]+9b dd 30\s+fsave\s+\(%rax\)
> -[ ]*[0-9a-f]+:[ ]+9b 41 dd 30\s+fsave\s+\(%r8\)
> -[ ]*[0-9a-f]+:[ ]+40 c5 f9 28 00[ ]+rex vmovapd \(%rax\),%xmm0
> -[ ]*[0-9a-f]+:[ ]+40[ ]+rex
> -[ ]*[0-9a-f]+:[ ]+41[ ]+rex.B
> -[ ]*[0-9a-f]+:[ ]+42[ ]+rex.X
> -[ ]*[0-9a-f]+:[ ]+43[ ]+rex.XB
> -[ ]*[0-9a-f]+:[ ]+44[ ]+rex.R
> -[ ]*[0-9a-f]+:[ ]+45[ ]+rex.RB
> -[ ]*[0-9a-f]+:[ ]+46[ ]+rex.RX
> -[ ]*[0-9a-f]+:[ ]+47[ ]+rex.RXB
> -[ ]*[0-9a-f]+:[ ]+48[ ]+rex.W
> -[ ]*[0-9a-f]+:[ ]+49[ ]+rex.WB
> -[ ]*[0-9a-f]+:[ ]+4a[ ]+rex.WX
> -[ ]*[0-9a-f]+:[ ]+4b[ ]+rex.WXB
> -[ ]*[0-9a-f]+:[ ]+4c[ ]+rex.WR
> -[ ]*[0-9a-f]+:[ ]+4d[ ]+rex.WRB
> -[ ]*[0-9a-f]+:[ ]+4e[ ]+rex.WRX
> -[ ]*[0-9a-f]+:[ ]+4f[ ]+rex.WRXB
> -#pass
> +#dump: ../rex.d
> --- a/gas/testsuite/gas/i386/rex.d
> +++ b/gas/testsuite/gas/i386/rex.d
> @@ -23,6 +23,11 @@ Disassembly of section .text:
> [ ]*[0-9a-f]+:[ ]+46 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%r8d
> [ ]*[0-9a-f]+:[ ]+45 03 04 00[ ]+add[ ]+\(%r8,%rax(,1)?\),%r8d
> [ ]*[0-9a-f]+:[ ]+4a 03 04 00[ ]+add[ ]+\(%rax,%r8(,1)?\),%rax
> +[ ]*[0-9a-f]+:[ ]+40 90[ ]+rex nop
> +[ ]*[0-9a-f]+:[ ]+48 90[ ]+rex\.W nop
> +[ ]*[0-9a-f]+:[ ]+44 90[ ]+rex\.R nop
> +[ ]*[0-9a-f]+:[ ]+42 90[ ]+rex\.X nop
> +[ ]*[0-9a-f]+:[ ]+41 90[ ]+xchg[ ]+%eax,%r8d
> [ ]*[0-9a-f]+:[ ]+41\s+rex\.B
> [ ]*[0-9a-f]+:[ ]+9b dd 30\s+fsave\s+\(%rax\)
> [ ]*[0-9a-f]+:[ ]+9b 41 dd 30\s+fsave\s+\(%r8\)
> --- a/gas/testsuite/gas/i386/rex.s
> +++ b/gas/testsuite/gas/i386/rex.s
> @@ -20,6 +20,12 @@ _start:
> rex.b add (%rax,%rax), %r8d
> rex.x add (%rax,%rax), %rax
>
> + rex nop
> + rex.w nop
> + rex.r nop
> + rex.x nop
> + rex.b nop
> +
> .byte 0x41,0x9b,0xdd,0x30
> fsave (%r8)
>
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -98,8 +98,7 @@ static void VPCOM_Fixup (instr_info *, i
> static void OP_0f07 (instr_info *, int, int);
> static void OP_Monitor (instr_info *, int, int);
> static void OP_Mwait (instr_info *, int, int);
> -static void NOP_Fixup1 (instr_info *, int, int);
> -static void NOP_Fixup2 (instr_info *, int, int);
> +static void NOP_Fixup (instr_info *, int, int);
> static void OP_3DNowSuffix (instr_info *, int, int);
> static void CMP_Fixup (instr_info *, int, int);
> static void BadOp (instr_info *);
> @@ -2913,9 +2912,9 @@ static const struct dis386 reg_table[][8
> static const struct dis386 prefix_table[][4] = {
> /* PREFIX_90 */
> {
> - { "xchgS", { { NOP_Fixup1, eAX_reg }, { NOP_Fixup2, eAX_reg } }, 0 },
> + { "xchgS", { { NOP_Fixup, 0 }, { NOP_Fixup, 1 } }, 0 },
> { "pause", { XX }, 0 },
> - { "xchgS", { { NOP_Fixup1, eAX_reg }, { NOP_Fixup2, eAX_reg } }, 0 },
> + { "xchgS", { { NOP_Fixup, 0 }, { NOP_Fixup, 1 } }, 0 },
> { NULL, { { NULL, 0 } }, PREFIX_IGNORED }
> },
>
> @@ -12724,25 +12723,14 @@ OP_0f07 (instr_info *ins, int bytemode,
> 32bit mode and "xchg %rax,%rax" in 64bit mode. */
>
> static void
> -NOP_Fixup1 (instr_info *ins, int bytemode, int sizeflag)
> +NOP_Fixup (instr_info *ins, int opnd, int sizeflag)
> {
> - if ((ins->prefixes & PREFIX_DATA) != 0
> - || (ins->rex != 0
> - && ins->rex != 0x48
> - && ins->address_mode == mode_64bit))
> - OP_REG (ins, bytemode, sizeflag);
> - else
> + if ((ins->prefixes & PREFIX_DATA) == 0 && (ins->rex & REX_B) == 0)
> strcpy (ins->obuf, "nop");
> -}
> -
> -static void
> -NOP_Fixup2 (instr_info *ins, int bytemode, int sizeflag)
> -{
> - if ((ins->prefixes & PREFIX_DATA) != 0
> - || (ins->rex != 0
> - && ins->rex != 0x48
> - && ins->address_mode == mode_64bit))
> - OP_IMREG (ins, bytemode, sizeflag);
> + else if (opnd == 0)
> + OP_REG (ins, eAX_reg, sizeflag);
> + else
> + OP_IMREG (ins, eAX_reg, sizeflag);
> }
>
> static const char *const Suffix3DNow[] = {
>
OK.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-05-18 17:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 16:16 [PATCH] x86: correct and simplify NOP disassembly Jan Beulich
2022-05-18 17:44 ` H.J. Lu
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).