[PATCH V5] x86: Add tls check in gas

[Jan Beulich]

On 11.10.2024 11:28, Cui, Lili wrote:
>> On 19.09.2024 08:38, Cui, Lili wrote:
>>> +    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.op[1].regs->reg_type.bitfield.class != Reg
>>> +	  || i.op[0].regs->reg_type.bitfield.class
>>> +	  || i.imm_operands)
>>
>> You may not de-reference i.op[0].regs for the intended forms at all. If anything,
>> i.op[0].disps may be de-referenced there.
>>
>> You may also not de-reference i.op[1].regs until you've checked that operand 1
>> actually is a register. There are insn forms after all where operand 1 is a memory
>> one.
>>
>> Which member of the union is valid to de-reference can only be told by
>> inspecting i.types[] and/or i.tm.operand_types[] (when insns are more
>> constrained than mov/add/sub are, going from just
>> i.{reg,imm,mem,disp}_operands may also be possible, just to mention it).
>>
>> I expect you want to check for operand counts first: No immediate one, a single
>> disp one, and a single register one. Then check that it's the destination that's the
>> register one. There may then not be any need to access any i.op[<n>].reg
>> anymore.
>>
> 
> Jan, do you mean to modify it like this?
> 
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index 25ecaa66104..358d76e7bd2 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -6545,9 +6545,10 @@ x86_check_tls_relocation (enum bfd_reloc_code_real r_type)
>           && i.tm.mnem_off != MN_add
>           && i.tm.mnem_off != MN_mov)
>         return x86_tls_error_insn;
> -      if (i.op[1].regs->reg_type.bitfield.class != Reg
> -         || i.op[0].regs->reg_type.bitfield.class
> -         || i.imm_operands)
> +      if (i.imm_operands
> +         || i.disp_operands != 1
> +         || i.reg_operands != 1
> +         || i.types[1].bitfield.class != Reg)
>         return x86_tls_error_opcode;

This goes in the right direction, but still isn't quite enough, I fear.
Knowing there's 1 disp and 1 reg operand still doesn't tell you which one
is which (both forms exist, after all). Plus because of APX there are also
3-operand forms of SUB and ADD (but of course not MOV).

Jan