Re: [PATCH] fix disassembly of spurious REX prefix before FWAIT

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Oct 22, 2012 at 11:52 AM, Roland McGrath <mcgrathr@google.com> wrote:
> The x86 disassembler has explicit logic to recognize prefixes before an
> FWAIT instruction as applying to that instruction rather than to the next
> instruction (into which the FWAIT might be subsumed for disassembly purposes).
> But this is broken in the case of REX prefixes.  This patch fixes it and
> does not introduce any regressions on x86_64-linux-gnu.

Have you verified that REX is ignored in hardware?

> Ok for trunk?
>
>
> Thanks,
> Roland
>
>
> gas/testsuite/
> 2012-10-22  Roland McGrath  <mcgrathr@google.com>
>
>         * gas/i386/x86-64-prefix-fwait.s: New file.
>         * gas/i386/x86-64-prefix-fwait.d: New file.
>         * gas/i386/x86-64-prefix-fwait-intel.d: New file.
>         * gas/i386/i386.exp: Run them.

Please put the new tests in rex.s.

> opcodes/
> 2012-10-22  Roland McGrath  <mcgrathr@google.com>
>
>         * i386-dis.c (ckprefix): When bailing out for fwait with prefixes,
>         set rex_used to rex.
>
>
> diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
> index 4739752..ba431b6 100644
> --- a/gas/testsuite/gas/i386/i386.exp
> +++ b/gas/testsuite/gas/i386/i386.exp
> @@ -480,6 +480,8 @@ if [expr ([istarget "i*86-*-*"] || [istarget
> "x86_64-*-*"]) && [gas_64_check]] t
>      run_dump_test "x86-64-rdseed-intel"
>      run_dump_test "x86-64-prefetch"
>      run_dump_test "x86-64-prefetch-intel"
> +    run_dump_test "x86-64-prefix-fwait"
> +    run_dump_test "x86-64-prefix-fwait-intel"
>
>      if { ![istarget "*-*-aix*"]
>        && ![istarget "*-*-beos*"]
> diff --git a/gas/testsuite/gas/i386/x86-64-prefix-fwait-intel.d
> b/gas/testsuite/gas/i386/x86-64-prefix-fwait-intel.d
> new file mode 100644
> index 0000000..5237654
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-prefix-fwait-intel.d
> @@ -0,0 +1,13 @@
> +#objdump: -dw -Mintel
> +#name: x86-64 prefix before fwait (Intel disassembly)
> +#source: x86-64-prefix-fwait.s
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <foo>:
> +\s*[a-f0-9]+:  41                      rex\.B
> +\s*[a-f0-9]+:  9b dd 30                fsave  \[rax\]
> +\s*[a-f0-9]+:  9b 41 dd 30             fsave  \[r8\]
> +#pass
> diff --git a/gas/testsuite/gas/i386/x86-64-prefix-fwait.d
> b/gas/testsuite/gas/i386/x86-64-prefix-fwait.d
> new file mode 100644
> index 0000000..047b090
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-prefix-fwait.d
> @@ -0,0 +1,12 @@
> +#objdump: -dw
> +#name: x86-64 prefix before fwait
> +
> +.*: +file format .*
> +
> +Disassembly of section .text:
> +
> +0+ <foo>:
> +\s*[a-f0-9]+:  41                      rex\.B
> +\s*[a-f0-9]+:  9b dd 30                fsave  \(%rax\)
> +\s*[a-f0-9]+:  9b 41 dd 30             fsave  \(%r8\)
> +#pass
> diff --git a/gas/testsuite/gas/i386/x86-64-prefix-fwait.s
> b/gas/testsuite/gas/i386/x86-64-prefix-fwait.s
> new file mode 100644
> index 0000000..40d1f0b
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-prefix-fwait.s
> @@ -0,0 +1,7 @@
> +.text
> +foo:
> +.byte 0x41,0x9b,0xdd,0x30
> +.byte 0x9b,0x41,0xdd,0x30

Please use "fsave  (%r8)" instead.

> +# Get a good alignment.
> + .p2align      4,0
> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
> index 15c968a..f8e62aa 100644
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -10756,6 +10756,9 @@ ckprefix (void)
>             {
>               prefixes |= PREFIX_FWAIT;
>               codep++;
> +             /* This ensures that the previous REX prefixes are noticed
> +                as unused prefixes, as in the return case below.  */
> +             rex_used = rex;
>               return 1;
>             }
>           prefixes = PREFIX_FWAIT;

Thanks.

-- 
H.J.