[PATCH v2] x86: Add tls check in gas

[Jan Beulich]

On 10.09.2024 09:12, Cui, Lili wrote:
>> On 05.09.2024 10:57, Cui, Lili wrote:
>>> @@ -6318,6 +6338,284 @@ static INLINE bool may_need_pass2 (const
>>> +      if (i.tm.mnem_off != MN_lea)
>>> +	return x86_tls_error_insn;
>>> +      else if (i.index_reg)
>>> +	return x86_tls_error_sib;
>>> +      else if (!i.base_reg)
>>> +	return x86_tls_error_no_base_reg;
>>> +      else if (i.base_reg->reg_type.bitfield.instance != RegB)
>>> +	return x86_tls_error_RegB;
>>
>> As mentioned on other occasions before: While I understand this is to a fair
>> degree a matter of style and hence personal preference, I view use of "else if"
>> in such situations as misleading. Ommiting the "else" also leads to shorter
>> code, i.e. (slightly) improved readability. (Note how further down you
>> sometimes omit the "else". Such inconsistency is imo even worse than having
>> the "else" everywhere.)
> 
> Jan, do you mean to use "if " instead "else if"? 

Yes.

>>> +    case BFD_RELOC_386_TLS_LDM:
>>> +      /*  Check transition from 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;
>>> +      else if (i.index_reg)
>>> +	return x86_tls_error_sib;
>>> +      else if (!i.base_reg)
>>> +	return x86_tls_error_no_base_reg;
>>> +      else if (i.base_reg->reg_type.bitfield.instance == Accum)
>>> +	return x86_tls_error_RegA;
>>> +      else if (i.types[1].bitfield.instance != Accum)
>>> +	return x86_tls_error_dest_RegA;
>>> +      break;
>>> +
>>> +    case BFD_RELOC_X86_64_GOTPC32_TLSDESC:
>>> +      /* Check transition from GOTPC32 TLSDESC access model:
>>> +
>>> +	 --- LP64 mode ---
>>> +	 leaq x@tlsdesc(%rip), %rax --> Memory reg must be %rip.
>>> +
>>> +	 --- X32 mode ---
>>> +	 rex leal x@tlsdesc(%rip), %eax --> Memory reg must be %rip.
>>> +
>>> +       */
>>> +      if (i.tm.mnem_off != MN_lea)
>>> +	return x86_tls_error_insn;
>>> +      else if (!i.base_reg)
>>> +	return x86_tls_error_no_base_reg;
>>> +      else if (i.base_reg->reg_num != RegIP)
>>> +	return x86_tls_error_RegIP;
>>> +      break;
>>
>> And no check for the required REX prefix in x32 mode? Or is it gas that
>> automatically adds it (could do with a few words in the comment then)?
> 
> For Egpr gas will automatically add rex2 for it. 

And REX2 is permitted there? If so, imo the comment wants updating to
reflect that.

>>> +    case x86_tls_error_base_reg_name:
>>> +      as_bad (_("@%s operator should not have base register"),
>> gotrel[k].str);
>>> +      return;
>>> +
>>> +    case x86_tls_error_index_RegB:
>>> +      as_bad (_("@%s operator only supports ebx/rbx as index register"),
>>> +	      gotrel[k].str);
>>
>> Please properly print register names: With % prefix when such is required,
>> and only the one register size that's actually permitted.
>>
> Removed "rbx". Considering Intel format, I did not add % prefix here.

I didn't ask to add an unconditional %. Instead I asked to use
register_prefix, like we do in other diagnostics. I merely assumed you
knew what's meant; I'm sorry if that was wrong.

Further you having removed rbx (and the messages being possible outside
of 64-bit mode only), I'd like to ask that the enumerators (i.e. also
the other one concerning EBX) be renamed from having RegB to having EBX.

Thinking of the involved addressing: Is use of any of these in 16-bit
code permitted by ld (i.e. when there's an address size prefix there)?
If not, I expect this also wants diagnosing.

Jan