[PATCH v2] x86: Add tls check in gas
H.J. Lu
hjl.tools@gmail.com
Fri Sep 6 14:06:16 GMT 2024
On Fri, Sep 6, 2024 at 6:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Sep 6, 2024 at 6:07 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Sep 6, 2024 at 5:34 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Thu, Sep 5, 2024 at 1:57 AM Cui, Lili <lili.cui@intel.com> wrote:
> > > >
> > > > Sorry, added v2 to the patch name
> > > >
> > > > ---
> > > > Assembler shouldn't accept invalid TLS instructions, TLS relocations
> > > > can only be used with specific instructions as specified in TLS psABI
> > > > and linker issues an error when TLS relocations are used with wrong
> > > > instructions or format. It is inconvenient for gcc to rely on linker
> > > > to report errors, adding tls check in the assembler stage so that gcc
> > > > can know tls errors earlier.
> > > >
> > > > ---
> > > > V2:
> > > > 1. Add format checks for each operator. And add a function to report errors.
> > > > 2. Replace DEFAULT_TLS_CHECK with DEFAULT_X86_TLS_CHECK, and all related names.
> > > > 3. Add more invalid test cases.
> > > > 4. Remove the handling of "call *x@tlsdesc(%rax)", which is now illegal.
> > > > 5. Add new INSTANCE RegDI
> > > > 6. Use option drw for objdump.
> > > > ---
> > > >
> > > > gas/ChangeLog:
> > > >
> > > > PR gas/32022
> > > > * config.in: Regenerate.
> > > > * config/tc-i386.c (_BFD_MAKE_TABLE_bfd_reloc_code_real): New.
> > > > (x86_check_tls_relocation): Added a new function to check tls relocation.
> > > > (i386_assemble): Handle x86_check_tls_relocation.
> > > > (OPTION_MTLS_CHECK): Added a new option to contrl tls check.
> > > > (struct option): Ditto.
> > > > (md_parse_option): Ditto.
> > > > (md_show_usage): Ditto.
> > > > * configure: Added a new option to check tls relocation by default.
> > > > * configure.ac: Ditto.
> > > > * doc/c-i386.texi: Ditto.
> > > > * testsuite/gas/i386/i386.exp: Added new tests.
> > > > * testsuite/gas/i386/ilp32/ilp32.exp: Ditto.
> > > > * testsuite/gas/i386/ilp32/reloc64.d: Disable tls check for it.
> > > > * testsuite/gas/i386/inval-tls.l: Added more test cases.
> > > > * testsuite/gas/i386/inval-tls.s: Ditto.
> > > > * testsuite/gas/i386/reloc32.d: Disable tls check for it.
> > > > * testsuite/gas/i386/reloc64.d: Ditto.
> > > > * testsuite/gas/i386/x86-64-inval-tls.l: Added more test cases.
> > > > * testsuite/gas/i386/x86-64-inval-tls.s: Ditto.
> > > > * testsuite/gas/i386/x86-64.exp: Added new tests.
> > > > * testsuite/gas/i386/tls.d: New test.
> > > > * testsuite/gas/i386/tls.s: Ditto.
> > > > * testsuite/gas/i386/x86-64-tls.d: Ditto.
> > > > * testsuite/gas/i386/x86-64-tls.s: Ditto.
> > > >
> > > > opcodes/ChangeLog:
> > > >
> > > > PR gas/32022
> > > > * i386-gen.c: Added new INSTANCE RegDI.
> > > > * i386-opc.h (enum operand_instance): Ditto.
> > > > * i386-reg.tbl: Added RegDI to rdi.
> > > >
> > > > ld/ChangeLog:
> > > >
> > > > PR gas/32022
> > > > * testsuite/ld-i386/tlsgdesc1.d: Disable tls check for it.
> > > > * testsuite/ld-i386/tlsgdesc2.d: Ditto.
> > > > * testsuite/ld-i386/tlsie2.d: Ditto.
> > > > * testsuite/ld-i386/tlsie3.d: Ditto.
> > > > * testsuite/ld-i386/tlsie4.d: Ditto.
> > > > * testsuite/ld-i386/tlsie5.d: Ditto.
> > > > * testsuite/ld-i386/tlsgdesc3.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsdesc3.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsdesc4.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsie2.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsie3.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsie5.d: Ditto.
> > > > * testsuite/ld-x86-64/tlsdesc5.d: Ditto.
> > > > ---
> > > > gas/config.in | 3 +
> > > > gas/config/tc-i386.c | 348 ++++++++++++++++++++--
> > > > gas/configure | 26 +-
> > > > gas/configure.ac | 18 ++
> > > > gas/doc/c-i386.texi | 10 +
> > > > gas/testsuite/gas/i386/i386.exp | 3 +-
> > > > gas/testsuite/gas/i386/ilp32/ilp32.exp | 2 +-
> > > > gas/testsuite/gas/i386/ilp32/reloc64.d | 2 +-
> > > > gas/testsuite/gas/i386/inval-tls.l | 28 ++
> > > > gas/testsuite/gas/i386/inval-tls.s | 42 +++
> > > > gas/testsuite/gas/i386/reloc32.d | 2 +-
> > > > gas/testsuite/gas/i386/reloc64.d | 2 +-
> > > > gas/testsuite/gas/i386/tls.d | 25 ++
> > > > gas/testsuite/gas/i386/tls.s | 31 ++
> > > > gas/testsuite/gas/i386/x86-64-inval-tls.l | 17 ++
> > > > gas/testsuite/gas/i386/x86-64-inval-tls.s | 26 ++
> > > > gas/testsuite/gas/i386/x86-64-tls.d | 29 ++
> > > > gas/testsuite/gas/i386/x86-64-tls.s | 31 ++
> > > > gas/testsuite/gas/i386/x86-64.exp | 3 +-
> > > > ld/testsuite/ld-i386/tlsgdesc1.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsgdesc2.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsgdesc3.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsie2.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsie3.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsie4.d | 2 +-
> > > > ld/testsuite/ld-i386/tlsie5.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsdesc3.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsdesc4.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsdesc5.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsie2.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsie3.d | 2 +-
> > > > ld/testsuite/ld-x86-64/tlsie5.d | 2 +-
> > > > opcodes/i386-gen.c | 1 +
> > > > opcodes/i386-opc.h | 1 +
> > > > opcodes/i386-reg.tbl | 2 +-
> > > > 35 files changed, 634 insertions(+), 44 deletions(-)
> > > > create mode 100644 gas/testsuite/gas/i386/tls.d
> > > > create mode 100644 gas/testsuite/gas/i386/tls.s
> > > > create mode 100644 gas/testsuite/gas/i386/x86-64-tls.d
> > > > create mode 100644 gas/testsuite/gas/i386/x86-64-tls.s
> > > >
> > > > diff --git a/gas/config.in b/gas/config.in
> > > > index a1f83499332..3f199d5b4a6 100644
> > > > --- a/gas/config.in
> > > > +++ b/gas/config.in
> > > > @@ -60,6 +60,9 @@
> > > > /* Define default value for RISC-V -mpriv-spec */
> > > > #undef DEFAULT_RISCV_PRIV_SPEC
> > > >
> > > > +/* Define to 1 if you want to check tls relocation by default. */
> > > > +#undef DEFAULT_X86_TLS_CHECK
> > > > +
> > > > /* Define to 1 if you want to generate GNU x86 used ISA and feature properties
> > > > by default. */
> > > > #undef DEFAULT_X86_USED_NOTE
> > > > diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> > > > index a9d3536429b..a0222580106 100644
> > > > --- a/gas/config/tc-i386.c
> > > > +++ b/gas/config/tc-i386.c
> > > > @@ -274,6 +274,23 @@ enum i386_error
> > > > internal_error,
> > > > };
> > > >
> > > > +enum x86_tls_error_type
> > > > +{
> > > > + x86_tls_error_none,
>
> Add x86_tls_error_continue.
>
> > > > + x86_tls_error_insn,
> > > > + x86_tls_error_sib,
> > > > + x86_tls_error_no_base_reg,
> > > > + x86_tls_error_require_no_base_index_reg,
> > > > + x86_tls_error_base_reg_name,
> > > > + x86_tls_error_index_RegB,
> > > > + x86_tls_error_RegA,
> > > > + x86_tls_error_RegB,
> > > > + x86_tls_error_RegIP,
> > > > + x86_tls_error_dest_RegA,
> > > > + x86_tls_error_dest_RegDI,
> > > > + x86_tls_error_scale_factor
> > > > +};
> > > > +
> > > > struct _i386_insn
> > > > {
>
> Add
>
> bool has_gotrel;
>
> and set it to true for relocation from the gotrel array.
>
> > > > /* TM holds the template for the insn were currently assembling. */
> > > > @@ -717,6 +734,9 @@ lfence_before_ret;
> > > > static int generate_relax_relocations
> > > > = DEFAULT_GENERATE_X86_RELAX_RELOCATIONS;
> > > >
> > > > +/* 1 if the assembler should check tls relocation. */
> > > > +static bool tls_check = DEFAULT_X86_TLS_CHECK;
> > > > +
> > > > static enum check_kind
> > > > {
> > > > check_none = 0,
> > > > @@ -6318,6 +6338,284 @@ static INLINE bool may_need_pass2 (const insn_template *t)
> > > > && (t->base_opcode | 8) == 0x2c);
> > > > }
> > > >
> > > > +static enum x86_tls_error_type
> > > > +x86_check_tls_relocation (unsigned int r_type)
> >
> > Please use enum bfd_reloc_code_real r_type.
> >
> > > > +{
> > > > + switch (r_type)
> > > > + {
> > > > + case BFD_RELOC_386_TLS_GOTDESC:
> > > > + /* Check transition from GDesc access model:
> > > > +
> > > > + leal x@tlsdesc(%ebx), %reg --> Memory reg must be %ebx 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 != RegB)
> > > > + return x86_tls_error_RegB;
> > > > + break;
> > > > +
> > > > + case BFD_RELOC_386_TLS_GD:
> > > > + /* Check transition from GD access model:
> > > > +
> > > > + leal foo@tlsgd(,%ebx,1), %eax --> Only this fixed format is supported.
> > > > + leal foo@tlsgd(%reg), %eax --> Dest reg must be '%eax'
> > > > + Memory reg can't be %eax.
> > > > + */
> > > > + if (i.tm.mnem_off != MN_lea)
> > > > + return x86_tls_error_insn;
> > > > + else if (i.index_reg)
> > > > + {
> > > > + if (i.base_reg)
> > > > + return x86_tls_error_base_reg_name;
> > > > + if (i.index_reg->reg_type.bitfield.instance != RegB)
> > > > + return x86_tls_error_index_RegB;
> > > > + else if (i.types[1].bitfield.instance != Accum)
> > > > + return x86_tls_error_dest_RegA;
> > > > + else if (i.log2_scale_factor)
> > > > + return x86_tls_error_scale_factor;
> > > > + }
> > > > + else
> > > > + {
> > > > + if (!i.base_reg)
> > > > + return x86_tls_error_no_base_reg;
> > > > + 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_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;
> > > > +
> > > > + case BFD_RELOC_X86_64_TLSGD:
> > > > + /* Check transition from GD access model:
> > > > +
> > > > + leaq foo@tlsgd(%rip), %rdi --> Only this fixed format is supported.
> > > > + */
> > > > + case BFD_RELOC_X86_64_TLSLD:
> > > > + /* Check transition from LD access model:
> > > > +
> > > > + leaq foo@tlsld(%rip), %rdi --> Only this fixed format is supported.
> > > > + */
> > > > + 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;
> > > > + else if (i.types[1].bitfield.instance != RegDI)
> >
> > Use
> >
> > else if (i.types[1].bitfield.class != Reg
> > || !i.op[1].regs->reg_type.bitfield.qword
> > || i.op[1].regs->reg_num != EDI_REG_NUM)
> > return x86_tls_error_dest_RegDI;
> >
> > > > + return x86_tls_error_dest_RegDI;
> > > > + break;
> > > > +
> > > > + case BFD_RELOC_386_TLS_GOTIE:
> > > > + /* Check transition from 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 transition from 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;
> > > > + else if (!i.base_reg)
> > > > + return x86_tls_error_no_base_reg;
> > > > + else if (i.index_reg)
> > > > + return x86_tls_error_sib;
> > > > + break;
> > > > +
> > > > + case BFD_RELOC_386_TLS_IE:
> > > > + /* Check transition from IE access model:
> > > > +
> > > > + movl foo@indntpoff, %reg --> Mod == 00 && r/m == 5
> > > > + addl foo@indntpoff, %reg --> Mod == 00 && r/m == 5
> > > > + */
> > > > + if (i.tm.mnem_off != MN_add && i.tm.mnem_off != MN_mov)
> > > > + return x86_tls_error_insn;
> > > > + else if (i.base_reg || i.index_reg)
> > > > + return x86_tls_error_require_no_base_index_reg;
> > > > + break;
> > > > +
> > > > + case BFD_RELOC_X86_64_GOTTPOFF:
> > > > + /* Check transition from GOTTPOFF access model:
> > > > +
> > > > + mov foo@gottpoff(%rip), %reg --> Memory Reg must be %rip.
> > > > + add foo@gottpoff(%rip), %reg --> Memory Reg must be %rip.
> > > > + */
> > > > + if (i.tm.mnem_off != MN_add && i.tm.mnem_off != MN_mov)
> > > > + 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;
> > > > +
> > > > + case BFD_RELOC_386_TLS_DESC_CALL:
> > > > + /* Check transition from GDesc access model:
> > > > +
> > > > + call *x@tlscall(%eax) --> Memory reg must be %eax and
> > > > + SIB is not supported.
> > > > + */
> > > > + if (i.tm.mnem_off != MN_call)
> > > > + 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;
> > > > + break;
> > > > +
> > > > + case BFD_RELOC_X86_64_TLSDESC_CALL:
> > > > + /* Check transition from GDesc access model:
> > > > +
> > > > + call *x@tlscall(%rax) <--- LP64 mode.
> > > > + call *x@tlscall(%eax) <--- X32 mode.
> > > > +
> > > > + Only these fixed formats are supported.
> > > > + */
> > > > + if (i.tm.mnem_off != MN_call)
> > > > + 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;
> > > > + break;
> > > > +
>
> case BFD_RELOC_NONE:
> return x86_tls_error_continue;
>
> > > > + default:
> > > > + return x86_tls_error_none;
> break;
> > > > + }
> > > > +
> > > > + return x86_tls_error_none;
> > > > +}
> > > > +
> > > > +static void
> > > > +x86_report_tls_error (enum x86_tls_error_type tls_error, unsigned int r_type)
>
> Use enum bfd_reloc_code_real r_type
>
> > > > +{
> > > > + unsigned int k;
> > > > + for (k = 0; k < ARRAY_SIZE (gotrel); k++)
> > > > + {
> > > > + if (gotrel[k].rel[object_64bit] == r_type)
> > > > + break;
> > > > + }
>
> Drop {}.
>
> > > > +
> > > > + switch (tls_error)
> > > > + {
> > > > + case x86_tls_error_insn:
> > > > + as_bad (_("@%s operator cannot be used with `%s'"),
> > > > + gotrel[k].str, insn_name (&i.tm));
> > > > + return;
> > > > +
> > > > + case x86_tls_error_sib:
> > > > + as_bad (_("@%s operator do not support SIB"), gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_no_base_reg:
> > > > + as_bad (_("@%s operator must have base regster"), gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_require_no_base_index_reg:
> > > > + as_bad (_("@%s operator should not have base/index register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + 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);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_RegA:
> > > > + as_bad (_("@%s operator only supports eax/rax as base register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_RegB:
> > > > + as_bad (_("@%s operator only supports ebx/rbx as base register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_RegIP:
> > > > + as_bad (_("@%s operator only supports rip as base register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_dest_RegA:
> > > > + as_bad (_("@%s operator only supports eax/rax as dest register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_dest_RegDI:
> > > > + as_bad (_("@%s operator only supports eax/rax as dest register"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + case x86_tls_error_scale_factor:
> > > > + as_bad (_("@%s operator only supports scale factor of 00"),
> > > > + gotrel[k].str);
> > > > + return;
> > > > +
> > > > + default:
> > > > + abort ();
> > > > + }
> > > > +}
> > > > +
> > > > /* This is the guts of the machine-dependent assembler. LINE points to a
> > > > machine dependent instruction. This function is supposed to emit
> > > > the frags/bytes it assembles to. */
> > > > @@ -6656,6 +6954,14 @@ i386_assemble (char *line)
> > > > i.prefix[LOCK_PREFIX] = 0;
> > > > }
> > > >
> > > > + if (tls_check)
>
> if (i.has_gotrel && tls_check)
>
> > > > + {
> > > > + enum x86_tls_error_type tls_error;
> > > > + for (j = 0; j < i.operands; ++j)
> > > > + if ((tls_error = x86_check_tls_relocation (i.reloc[j])) != x86_tls_error_none)
> > > > + x86_report_tls_error (tls_error, i.reloc[j]);
>
> {
> tls_error = x86_check_tls_relocation (i.reloc[j]);
> if (tls_error == x86_tls_error_continue)
> continue;
>
> if (tls_error != x86_tls_error_none)
> x86_report_tls_error (tls_error, i.reloc[j]);
> break;
> }
>
This patch is on top of your patch.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Improve-x86-TLS-relocation-check.patch
Type: text/x-patch
Size: 5301 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20240906/9ac80235/attachment-0001.bin>
More information about the Binutils
mailing list