[PATCH v2] x86: Add tls check in gas

[Cui, Lili]

> 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.
> 
Done.

> >>> +    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.
> 
REX2 is permitted , Added. 

> >>> +    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.
> 
Oh, sorry, I misunderstood.  Done.

> 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.
> 
Done.

> 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.
> 
I found in TLS sequences, 0x66 is often used as a placeholder, so data16 should be ok.


Thanks,
Lili.

> Jan