[PATCH v3] x86: Add tls check in gas

[Jan Beulich]

On 11.09.2024 08:08, Cui, Lili wrote:
> @@ -6353,6 +6379,305 @@ static INLINE bool may_need_pass2 (const insn_template *t)
>  	       && (t->base_opcode | 8) == 0x2c);
>  }
>  
> +static enum x86_tls_error_type
> +x86_check_tls_relocation (enum bfd_reloc_code_real r_type)
> +{
> +  switch (r_type)
> +    {
> +    case BFD_RELOC_386_TLS_GOTDESC:
> +      /* Check GDesc access model:
> +
> +	 leal x@tlsdesc(%ebx), %reg --> Memory reg must be %ebx and
> +					SIB is not supported.
> +       */
> +      if (i.tm.mnem_off != MN_lea)
> +	return x86_tls_error_insn;
> +      if (i.index_reg)
> +	return x86_tls_error_sib;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_type.bitfield.instance != RegB)
> +	return x86_tls_error_ebx;
> +      if (!i.op[1].regs->reg_type.bitfield.dword)
> +	return x86_tls_error_dest_reg_size;
> +      break;
> +
> +    case BFD_RELOC_386_TLS_GD:
> +      /* Check GD access model:
> +
> +	 leal foo@tlsgd(,%ebx,1), %eax   --> Only this fixed format is supported.
> +	 leal foo@tlsgd(%reg), %eax      --> Dest reg must be '%eax'
> +					     Memory reg can't be %eax.
> +       */
> +      if (i.tm.mnem_off != MN_lea)
> +	return x86_tls_error_insn;
> +      if (i.op[1].regs->reg_type.bitfield.instance != Accum)
> +	return x86_tls_error_dest_eax;
> +      if (!i.op[1].regs->reg_type.bitfield.dword)
> +	return x86_tls_error_dest_reg_size;
> +      if (i.index_reg)
> +	{
> +	  if (i.base_reg)
> +	    return x86_tls_error_base_reg_name;
> +	  if (i.index_reg->reg_type.bitfield.instance != RegB)
> +	    return x86_tls_error_index_ebx;
> +	  if (i.log2_scale_factor)
> +	    return x86_tls_error_scale_factor;
> +	}
> +      else
> +	{
> +	  if (!i.base_reg)
> +	    return x86_tls_error_no_base_reg;
> +	  if (i.base_reg->reg_type.bitfield.instance == Accum)
> +	    return x86_tls_error_RegA;
> +	}
> +      break;
> +
> +    case BFD_RELOC_386_TLS_LDM:
> +      /*  Check LDM access model:
> +
> +	  leal foo@tlsldm(%reg), %eax --> Dest reg must be '%eax'
> +					  Memory reg can't be %eax and SIB
> +					  is not supported.
> +       */
> +      if (i.tm.mnem_off != MN_lea)
> +	return x86_tls_error_insn;
> +      if (i.index_reg)
> +	return x86_tls_error_sib;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_type.bitfield.instance == Accum)
> +	return x86_tls_error_RegA;
> +      if (i.op[1].regs->reg_type.bitfield.instance != Accum)
> +	return x86_tls_error_dest_eax;
> +      if (!i.op[1].regs->reg_type.bitfield.dword)
> +	return x86_tls_error_dest_reg_size;
> +      break;
> +
> +    case BFD_RELOC_X86_64_GOTPC32_TLSDESC:
> +      /* Check GOTPC32 TLSDESC access model:
> +
> +	 --- LP64 mode ---
> +	 leaq x@tlsdesc(%rip), %reg64 --> Memory reg must be %rip.
> +
> +	 --- X32 mode ---
> +	 rex/rex2 leal x@tlsdesc(%rip), %reg32 --> Memory reg must be %rip.
> +
> +	 In X32 mode, when dest is Egpr, gas will automatically add rex2 to it.
> +	 When dest is Gpr, we need to check whether there is a rex/rex2 prefix.
> +       */
> +      if (i.tm.mnem_off != MN_lea)
> +	return x86_tls_error_insn;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_num != RegIP)
> +	return x86_tls_error_rip;

Just to double check: %eip as "memory reg" is okay?

> +      if (i.op[1].regs->reg_type.bitfield.dword
> +	       && !(i.op[1].regs->reg_flags & RegRex2)

Nit: Indentation looks to be wrong starting from here.

> +	       && (pp.rex2_encoding == false
> +		   && pp.rex_encoding == false
> +		   && i.prefix[REX_PREFIX] != REX_OPCODE))

Why is this last set separately parenthesized? And as mentionbed before:
Please don't ever compare booleans against true or false. Use booleans
as booleans; that's what they are for.

Where's the operand size check for both cases, yielding
x86_tls_error_dest_reg_size when it fails?

> +	return x86_tls_error_REX_prefix;
> +      break;
> +
> +    case BFD_RELOC_X86_64_TLSGD:
> +      /* Check GD access model:
> +
> +	 leaq foo@tlsgd(%rip), %rdi --> Only this fixed format is supported.
> +       */
> +    case BFD_RELOC_X86_64_TLSLD:
> +      /* Check LD access model:
> +
> +	 leaq foo@tlsld(%rip), %rdi --> Only this fixed format is supported.
> +       */
> +      if (i.tm.mnem_off != MN_lea)
> +	return x86_tls_error_insn;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_num != RegIP)
> +	return x86_tls_error_rip;
> +      if (!i.op[1].regs->reg_type.bitfield.qword
> +	  || i.op[1].regs->reg_num != EDI_REG_NUM
> +	  || i.op[1].regs->reg_flags)
> +	return x86_tls_error_dest_rdi;
> +      break;
> +
> +    case BFD_RELOC_386_TLS_GOTIE:
> +      /* Check GOTIE access model:
> +
> +	 subl foo@gotntpoff(%reg1), %reg2
> +	 movl foo@gotntpoff(%reg1), %reg2
> +	 addl foo@gotntpoff(%reg1), %reg2
> +
> +	 Memory operand: SIB is not supported.
> +       */
> +    case BFD_RELOC_386_TLS_IE_32:
> +      /* Check IE_32 access model:
> +
> +	 subl foo@gottpoff(%reg1), %reg2
> +	 movl foo@gottpoff(%reg1), %reg2
> +	 addl foo@gottpoff(%reg1), %reg2
> +
> +	 Memory operand: SIB is not supported.
> +       */
> +      if (i.tm.mnem_off != MN_sub
> +	  && i.tm.mnem_off != MN_add
> +	  && i.tm.mnem_off != MN_mov)
> +	return x86_tls_error_insn;

If I'm not mistaken my prior question here wasn't answered: All forms
of these are okay? Without checking, how do you even know ...

> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.index_reg)
> +	return x86_tls_error_sib;
> +      if (!i.op[1].regs->reg_type.bitfield.dword)
> +	return x86_tls_error_dest_reg_size;

... that i.op[1].regs is valid (i.e. that operand is a register)? Just
to take an example.

> +      break;
> +
> +    case BFD_RELOC_386_TLS_IE:
> +      /* Check IE access model:
> +
> +	 movl foo@indntpoff, %reg --> Mod == 00 && r/m == 5
> +	 addl foo@indntpoff, %reg --> Mod == 00 && r/m == 5
> +       */
> +      if (i.tm.mnem_off != MN_add && i.tm.mnem_off != MN_mov)
> +	return x86_tls_error_insn;
> +      if (i.base_reg || i.index_reg)
> +	return x86_tls_error_require_no_base_index_reg;
> +      if (!i.op[1].regs->reg_type.bitfield.dword)
> +	return x86_tls_error_dest_reg_size;

Same issue here.

> +      break;
> +
> +    case BFD_RELOC_X86_64_GOTTPOFF:
> +      /* Check GOTTPOFF access model:
> +
> +	 mov foo@gottpoff(%rip), %reg --> Memory Reg must be %rip.
> +	 add foo@gottpoff(%rip), %reg --> Memory Reg must be %rip.
> +       */
> +      if (i.tm.mnem_off != MN_add && i.tm.mnem_off != MN_mov)
> +	return x86_tls_error_insn;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_num != RegIP)
> +	return x86_tls_error_rip;

And similarly here, even if in this case you don't de-reference a
pointer that may not point to where you want it.

> +      break;
> +
> +    case BFD_RELOC_386_TLS_DESC_CALL:
> +      /* Check GDesc access model:
> +
> +	 call *x@tlscall(%eax) --> Memory reg must be %eax and
> +				   SIB is not supported.
> +       */
> +    case BFD_RELOC_X86_64_TLSDESC_CALL:
> +      /* Check GDesc access model:
> +
> +	 call *x@tlscall(%rax) <--- LP64 mode.
> +	 call *x@tlscall(%eax) <--- X32 mode.
> +
> +	 Only these fixed formats are supported.
> +       */
> +      if (i.tm.mnem_off != MN_call)
> +	return x86_tls_error_insn;
> +      if (i.index_reg)
> +	return x86_tls_error_sib;
> +      if (!i.base_reg)
> +	return x86_tls_error_no_base_reg;
> +      if (i.base_reg->reg_type.bitfield.instance != Accum)
> +	return x86_tls_error_RegA;

No address size check?

> +static void
> +x86_report_tls_error (enum x86_tls_error_type tls_error, enum bfd_reloc_code_real r_type)
> +{
> +  unsigned int k;
> +  for (k = 0; k < ARRAY_SIZE (gotrel); k++)
> +    if (gotrel[k].rel[object_64bit] == r_type)
> +      break;
> +
> +  switch (tls_error)
> +    {
> +    case x86_tls_error_insn:
> +      as_bad (_("@%s operator cannot be used with `%s'"),
> +	      gotrel[k].str, insn_name (&i.tm));
> +      return;
> +
> +    case x86_tls_error_sib:
> +      as_bad (_("@%s operator requires no SIB"), gotrel[k].str);
> +      return;
> +
> +    case x86_tls_error_no_base_reg:
> +      as_bad (_("@%s operator requires base register"), gotrel[k].str);
> +      return;
> +
> +    case x86_tls_error_require_no_base_index_reg:
> +      as_bad (_("@%s operator requires no base/index register"),
> +	      gotrel[k].str);
> +      return;
> +
> +    case x86_tls_error_base_reg_name:
> +      as_bad (_("@%s operator requires no base register"), gotrel[k].str);
> +      return;

What does "name" stand for in the enumerator's identifier?

> +    case x86_tls_error_index_ebx:
> +      as_bad (_("@%s operator requires `%sebx' as index register"),
> +	      gotrel[k].str, register_prefix);
> +      return;
> +
> +    case x86_tls_error_RegA:
> +      as_bad (_("@%s operator requires `%seax/rax' as base register"),
> +	      gotrel[k].str, register_prefix);
> +      return;

This diag is okay only if indeed both registers are okay. I think that
isn't (always) the case. And of course the % prefix needs to appear on
every register name (as long as we're in prefix-enabled mode).

Jan