Re: [PATCH v3 7/7] x86: move bad-use-of-TLS-reloc check

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

On Wed, Oct 5, 2022 at 12:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Having it in match_template() is unhelpful. Neither does looking for the
> next template to possibly match make any sense in that case, nor is the
> resulting diagnostic making clear what the problem is.
>
> While moving the check, also generalize it to include all SIMD and VEX-
> encoded insns. This way an existing conditional can be re-used in
> md_assemble(). Note though that this still leaves a lof of insns which
> are also wrong to use with these relocations.

These TLS instruction sequences are generated by compilers.
The check is only meant to check for invalid master registers.

> Further fold the remaining check (BFD_RELOC_386_GOT32) with the XRELEASE
> related one a few lines down. This again allows re-using an existing
> conditional.
> ---
> Is the set of relocations enumerated here actually complete?
> ---
> v2: New.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -5119,14 +5119,30 @@ md_assemble (char *line)
>        return;
>      }
>
> -  /* Check for data size prefix on VEX/XOP/EVEX encoded and SIMD insns.  */
> -  if (i.prefix[DATA_PREFIX]
> -      && (is_any_vex_encoding (&i.tm)
> -         || i.tm.operand_types[i.imm_operands].bitfield.class >= RegMMX
> -         || i.tm.operand_types[i.imm_operands + 1].bitfield.class >= RegMMX))
> +  if (is_any_vex_encoding (&i.tm)
> +      || i.tm.operand_types[i.imm_operands].bitfield.class >= RegMMX
> +      || i.tm.operand_types[i.imm_operands + 1].bitfield.class >= RegMMX)
>      {
> -      as_bad (_("data size prefix invalid with `%s'"), i.tm.name);
> -      return;
> +      /* Check for data size prefix on VEX/XOP/EVEX encoded and SIMD insns.  */
> +      if (i.prefix[DATA_PREFIX])
> +       {
> +         as_bad (_("data size prefix invalid with `%s'"), i.tm.name);
> +         return;
> +       }
> +
> +      /* Don't allow e.g. KMOV in TLS code sequences.  */
> +      for (j = i.imm_operands; j < i.operands; ++j)
> +       switch (i.reloc[j])
> +         {
> +         case BFD_RELOC_386_TLS_GOTIE:
> +         case BFD_RELOC_386_TLS_LE_32:
> +         case BFD_RELOC_X86_64_GOTTPOFF:
> +         case BFD_RELOC_X86_64_TLSLD:
> +           as_bad (_("TLS relocation cannot be used with `%s'"), i.tm.name);
> +           return;
> +         default:
> +           break;
> +         }
>      }
>
>    /* Check if HLE prefix is OK.  */
> @@ -6767,26 +6783,6 @@ match_template (char mnem_suffix)
>             }
>         }
>
> -      switch (i.reloc[0])
> -       {
> -       case BFD_RELOC_386_GOT32:
> -         /* Force 0x8b encoding for "mov foo@GOT, %eax".  */
> -         if (t->base_opcode == 0xa0
> -             && t->opcode_modifier.opcodespace == SPACE_BASE)
> -           continue;
> -         break;
> -       case BFD_RELOC_386_TLS_GOTIE:
> -       case BFD_RELOC_386_TLS_LE_32:
> -       case BFD_RELOC_X86_64_GOTTPOFF:
> -       case BFD_RELOC_X86_64_TLSLD:
> -         /* Don't allow KMOV in TLS code sequences.  */
> -         if (t->opcode_modifier.vex)
> -           continue;
> -         break;
> -       default:
> -         break;
> -       }
> -
>        /* We check register size if needed.  */
>        if (t->opcode_modifier.checkregsize)
>         {
> @@ -6817,12 +6813,19 @@ match_template (char mnem_suffix)
>               && i.types[1].bitfield.instance == Accum
>               && i.types[1].bitfield.dword)
>             continue;
> -         /* xrelease mov %eax, <disp> is another special case. It must not
> -            match the accumulator-only encoding of mov.  */
> -         if (i.hle_prefix
> -             && t->base_opcode == 0xa0
> +
> +         if (t->base_opcode == MOV_AX_DISP32
>               && t->opcode_modifier.opcodespace == SPACE_BASE)
> -           continue;
> +           {
> +             /* Force 0x8b encoding for "mov foo@GOT, %eax".  */
> +             if (i.reloc[0] == BFD_RELOC_386_GOT32)
> +               continue;
> +
> +             /* xrelease mov %eax, <disp> is another special case. It must not
> +                match the accumulator-only encoding of mov.  */
> +             if (i.hle_prefix)
> +               continue;
> +           }
>           /* Fall through.  */
>
>         case 3:
>

OK.

Thanks.

-- 
H.J.