[PATCH V5] x86: Add tls check in gas

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, October 14, 2024 6:22 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: binutils@sourceware.org; hjl.tools@gmail.com
> Subject: Re: [PATCH V5] x86: Add tls check in gas
> 
> On 14.10.2024 12:06, Cui, Lili wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Friday, October 11, 2024 6:04 PM
> >> To: Cui, Lili <lili.cui@intel.com>
> >> Cc: binutils@sourceware.org; hjl.tools@gmail.com
> >> Subject: Re: [PATCH V5] x86: Add tls check in gas
> >>
> >> 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).
> >
> > "i.types[1].bitfield.class != Reg" requires dest to be reg, so source should be
> disp.
> >
> >> Plus because of APX there are also 3-operand forms of SUB and ADD
> >> (but of course not MOV).
> >>
> >
> > For APX the 3-operand form, there are two types,
> >
> > 1 imm + 2 others   :  “i.imm_operands”  can exclude this situation.
> > 1 dis + 2 regs          :  “i.reg_operands != 1” can exclude this situation.
> 
> Oh, you're right. I wasn't properly looking at things, I'm sorry.
> 

NP, thank you for this optimization suggestion, I will send out the patch later.

Lili.

> Jan