[PATCH V5] x86: Add tls check in gas

[Cui, Lili]

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


Thanks,
Lili.




> Jan