Re: [PATCH v2] x86: fix "REP RET" with -madd-bnd-prefix

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

On Tue, Jul 10, 2018 at 6:32 AM, Jan Beulich <JBeulich@suse.com> wrote:
> Just like any other branches, RET should gain a BND prefix also when
> already prefixed in source by REP.
>
> gas/
> 2018-07-10  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (md_assemble): Also replace an already
>         present REP prefix.
>         * testsuite/gas/i386/mpx-add-bnd-prefix.s,
>         testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.s: Test RET with
>         all REP flavors.
>         * testsuite/gas/i386/mpx-add-bnd-prefix.d,
>         testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.d: Adjust
>         expectations.
>         * testsuite/gas/i386/mpx-add-bnd-prefix.e,
>         testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.e: New.
>
> ---
> v2: Warn when replacing REP/REPE with BND.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -4112,10 +4112,16 @@ md_assemble (char *line)
>      }
>
>    /* Insert BND prefix.  */
> -  if (add_bnd_prefix
> -      && i.tm.opcode_modifier.bndprefixok
> -      && !i.prefix[BND_PREFIX])
> -    add_prefix (BND_PREFIX_OPCODE);
> +  if (add_bnd_prefix && i.tm.opcode_modifier.bndprefixok)
> +    {
> +      if (!i.prefix[BND_PREFIX])
> +       add_prefix (BND_PREFIX_OPCODE);
> +      else if (i.prefix[BND_PREFIX] != BND_PREFIX_OPCODE)
> +       {
> +         as_warn (_("replacing `rep'/`repe' prefix by `bnd'"));
> +         i.prefix[BND_PREFIX] = BND_PREFIX_OPCODE;
> +       }
> +    }
>
>    /* Check string instruction segment overrides.  */
>    if (i.tm.opcode_modifier.isstring && i.mem_operands != 0)
> --- a/gas/testsuite/gas/i386/mpx-add-bnd-prefix.d
> +++ b/gas/testsuite/gas/i386/mpx-add-bnd-prefix.d
> @@ -1,4 +1,5 @@
>  #as: -madd-bnd-prefix
> +#stderr: mpx-add-bnd-prefix.e
>  #objdump: -drw
>  #name: Check -madd-bnd-prefix
>
> @@ -18,7 +19,9 @@ Disassembly of section .text:
>  0+14 <foo>:
>  [      ]*[a-f0-9]+:    f2 c3                   bnd ret
>  [      ]*[a-f0-9]+:    f2 c3                   bnd ret
> -[      ]*[a-f0-9]+:    f2 e8 f6 ff ff ff       bnd call 14 <foo>
> +[      ]*[a-f0-9]+:    f2 c3                   bnd ret
> +[      ]*[a-f0-9]+:    f2 c3                   bnd ret
> +[      ]*[a-f0-9]+:    f2 e8 f2 ff ff ff       bnd call 14 <foo>
>  [      ]*[a-f0-9]+:    01 c3                   add    %eax,%ebx
> -[      ]*[a-f0-9]+:    e2 f2                   loop   14 <foo>
> +[      ]*[a-f0-9]+:    e2 ee                   loop   14 <foo>
>  #pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/mpx-add-bnd-prefix.e
> @@ -0,0 +1,3 @@
> +.*: Assembler messages:
> +.*:12: Warning: .*rep.*bnd.*
> +.*:13: Warning: .*rep.*bnd.*
> --- a/gas/testsuite/gas/i386/mpx-add-bnd-prefix.s
> +++ b/gas/testsuite/gas/i386/mpx-add-bnd-prefix.s
> @@ -8,7 +8,9 @@
>         jmp     *(%ebx)
>         ret
>  foo:
> -       # Use of REPNE prefix - we shouldn't get any error

Please keep this comment.

> +       # Use of any REP prefix - we shouldn't get any error

Please add a comment to indicate that REP is replaced
by BND with a warning.  OK with these changes.

Thanks.

H.J.

> +       rep ret
> +       repe ret
>         repne ret
>         # BND prefix already exists - we shouldn't get any error here
>         bnd ret
> --- a/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.d
> +++ b/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.d
> @@ -1,4 +1,5 @@
>  #as: -madd-bnd-prefix
> +#stderr: x86-64-mpx-add-bnd-prefix.e
>  #objdump: -drw
>  #name: Check -madd-bnd-prefix (x86-64)
>
> @@ -18,7 +19,9 @@ Disassembly of section .text:
>  0+14 <foo>:
>  [      ]*[a-f0-9]+:    f2 c3                   bnd retq
>  [      ]*[a-f0-9]+:    f2 c3                   bnd retq
> -[      ]*[a-f0-9]+:    f2 e8 f6 ff ff ff       bnd callq 14 <foo>
> +[      ]*[a-f0-9]+:    f2 c3                   bnd retq
> +[      ]*[a-f0-9]+:    f2 c3                   bnd retq
> +[      ]*[a-f0-9]+:    f2 e8 f2 ff ff ff       bnd callq 14 <foo>
>  [      ]*[a-f0-9]+:    48 01 c3                add    %rax,%rbx
> -[      ]*[a-f0-9]+:    e2 f1                   loop   14 <foo>
> +[      ]*[a-f0-9]+:    e2 ed                   loop   14 <foo>
>  #pass
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.e
> @@ -0,0 +1,3 @@
> +.*: Assembler messages:
> +.*:12: Warning: .*rep.*bnd.*
> +.*:13: Warning: .*rep.*bnd.*
> --- a/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.s
> +++ b/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.s
> @@ -8,7 +8,9 @@
>         jmp     *(%rbx)
>         ret
>  foo:
> -       # Use of REPNE prefix - we shouldn't get any error
> +       # Use of any REP prefix - we shouldn't get any error
> +       rep ret
> +       repe ret
>         repne ret
>         # BND prefix already exists - we shouldn't get any error here
>         bnd ret
>
>
>
>



-- 
H.J.