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

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

On Wed, May 30, 2018 at 12:40 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.7
>
> gas/
> 2018-05-30  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.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -4110,10 +4110,13 @@ 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
> +       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
> @@ -18,7 +18,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
> --- 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
> +       # 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
> --- a/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.d
> +++ b/gas/testsuite/gas/i386/x86-64-mpx-add-bnd-prefix.d
> @@ -18,7 +18,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
> --- 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
>

Assembly code asks for F3 prefix and F2 prefix is generated.
It does't look right to me.

-- 
H.J.