From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: [PATCH] x86: correct and simplify NOP disassembly
Date: Wed, 18 May 2022 10:44:52 -0700 [thread overview]
Message-ID: <CAMe9rOohPmtC7+GZM9hcHbAsYD=JmpYhuS5o_9NRSnzKc_RP2w@mail.gmail.com> (raw)
In-Reply-To: <d3486316-64b0-bd8d-38e9-164055713c5c@suse.com>
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.
prev parent reply other threads:[~2022-05-18 17:45 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-11 16:16 Jan Beulich
2022-05-18 17:44 ` H.J. Lu [this message]
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='CAMe9rOohPmtC7+GZM9hcHbAsYD=JmpYhuS5o_9NRSnzKc_RP2w@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=binutils@sourceware.org \
--cc=jbeulich@suse.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).