[PATCH] x86: Report invalid TLS relocation name

[Cui, Lili]

> On 27.08.2024 22:15, H.J. Lu wrote:
> > On Tue, Aug 27, 2024 at 9:57 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> --- a/gas/config/tc-i386.c
> >> +++ b/gas/config/tc-i386.c
> >> @@ -1293,6 +1293,94 @@ static htab_t op_hash;
> >>
> >>  /* Hash table for register lookup.  */  static htab_t reg_hash;
> >> +
> >> +static const struct
> >> +{
> >> +  const char *str;
> >> +  int len;
> 
> As it is being moved, please conver to unsigned int.
> 
> >> +  const enum bfd_reloc_code_real rel[2];
> >> +  const i386_operand_type types64;
> >> +  bool need_GOT_symbol;
> >> +}
> >> +gotrel[] =
> >> +{
> >> +#define OPERAND_TYPE_IMM32_32S_DISP32 { .bitfield = \
> >> +      { .imm32 = 1, .imm32s = 1, .disp32 = 1 } } #define
> >> +OPERAND_TYPE_IMM32_32S_64_DISP32 { .bitfield = \
> >> +      { .imm32 = 1, .imm32s = 1, .imm64 = 1, .disp32 = 1 } } #define
> >> +OPERAND_TYPE_IMM32_32S_64_DISP32_64 { .bitfield = \
> >> +      { .imm32 = 1, .imm32s = 1, .imm64 = 1, .disp32 = 1, .disp64 =
> >> +1 } } #define OPERAND_TYPE_IMM64_DISP64 { .bitfield = \
> >> +      { .imm64 = 1, .disp64 = 1 } }
> >> +
> >> +#ifndef TE_PE
> >> +#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
> >> +    { STRING_COMMA_LEN ("SIZE"),      { BFD_RELOC_SIZE32,
> >> +                                       BFD_RELOC_SIZE32 },
> >> +    { .bitfield = { .imm32 = 1, .imm64 = 1 } }, false }, #endif
> >> +    { STRING_COMMA_LEN ("PLTOFF"),
> { _dummy_first_bfd_reloc_code_real,
> >> +                                      BFD_RELOC_X86_64_PLTOFF64 },
> >> +    { .bitfield = { .imm64 = 1 } }, true },
> >> +    { STRING_COMMA_LEN ("PLT"),      { BFD_RELOC_386_PLT32,
> >> +                                      BFD_RELOC_X86_64_PLT32    },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, false },
> >> +    { STRING_COMMA_LEN ("GOTPLT"),
> { _dummy_first_bfd_reloc_code_real,
> >> +                                      BFD_RELOC_X86_64_GOTPLT64 },
> >> +    OPERAND_TYPE_IMM64_DISP64, true },
> >> +    { STRING_COMMA_LEN ("GOTOFF"),   { BFD_RELOC_386_GOTOFF,
> >> +                                      BFD_RELOC_X86_64_GOTOFF64 },
> >> +    OPERAND_TYPE_IMM64_DISP64, true },
> >> +    { STRING_COMMA_LEN ("GOTPCREL"),
> { _dummy_first_bfd_reloc_code_real,
> >> +                                      BFD_RELOC_X86_64_GOTPCREL },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true },
> >> +    { STRING_COMMA_LEN ("TLSGD"),    { BFD_RELOC_386_TLS_GD,
> >> +                                      BFD_RELOC_X86_64_TLSGD    },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true },
> >> +    { STRING_COMMA_LEN ("TLSLDM"),   { BFD_RELOC_386_TLS_LDM,
> >> +                                      _dummy_first_bfd_reloc_code_real },
> >> +    OPERAND_TYPE_NONE, true },
> >> +    { STRING_COMMA_LEN ("TLSLD"),
> { _dummy_first_bfd_reloc_code_real,
> >> +                                      BFD_RELOC_X86_64_TLSLD    },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true },
> >> +    { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32,
> >> +                                      BFD_RELOC_X86_64_GOTTPOFF },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true },
> >> +    { STRING_COMMA_LEN ("TPOFF"),    { BFD_RELOC_386_TLS_LE_32,
> >> +                                      BFD_RELOC_X86_64_TPOFF32  },
> >> +    OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
> >> +    { STRING_COMMA_LEN ("NTPOFF"),   { BFD_RELOC_386_TLS_LE,
> >> +                                      _dummy_first_bfd_reloc_code_real },
> >> +    OPERAND_TYPE_NONE, true },
> >> +    { STRING_COMMA_LEN ("DTPOFF"),   { BFD_RELOC_386_TLS_LDO_32,
> >> +                                      BFD_RELOC_X86_64_DTPOFF32 },
> >> +    OPERAND_TYPE_IMM32_32S_64_DISP32_64, true },
> >> +    { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE,
> >> +                                      _dummy_first_bfd_reloc_code_real },
> >> +    OPERAND_TYPE_NONE, true },
> >> +    { STRING_COMMA_LEN ("INDNTPOFF"),{ BFD_RELOC_386_TLS_IE,
> >> +                                      _dummy_first_bfd_reloc_code_real },
> >> +    OPERAND_TYPE_NONE, true },
> >> +    { STRING_COMMA_LEN ("GOT"),      { BFD_RELOC_386_GOT32,
> >> +                                      BFD_RELOC_X86_64_GOT32    },
> >> +    OPERAND_TYPE_IMM32_32S_64_DISP32, true },
> >> +    { STRING_COMMA_LEN ("TLSDESC"),
> { BFD_RELOC_386_TLS_GOTDESC,
> >> +                                      BFD_RELOC_X86_64_GOTPC32_TLSDESC },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true },
> >> +    { STRING_COMMA_LEN ("TLSCALL"),
> { BFD_RELOC_386_TLS_DESC_CALL,
> >> +                                      BFD_RELOC_X86_64_TLSDESC_CALL },
> >> +    OPERAND_TYPE_IMM32_32S_DISP32, true }, #else /* TE_PE */
> >> +    { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL,
> >> +                                      BFD_RELOC_32_SECREL },
> >> +    OPERAND_TYPE_IMM32_32S_64_DISP32_64, false }, #endif
> >> +
> >> +#undef OPERAND_TYPE_IMM32_32S_DISP32 #undef
> >> +OPERAND_TYPE_IMM32_32S_64_DISP32 #undef
> >> +OPERAND_TYPE_IMM32_32S_64_DISP32_64
> >> +#undef OPERAND_TYPE_IMM64_DISP64
> >> +};
> >>
> >>    /* Various efficient no-op patterns for aligning code labels.
> >>       Note: Don't try to assemble the instructions in the comments.
> >> @@ -6586,8 +6674,16 @@ i386_assemble (char *line)
> >>           case BFD_RELOC_X86_64_GOTTPOFF:
> >>           case BFD_RELOC_386_TLS_GOTIE:
> >>           case BFD_RELOC_X86_64_TLSLD:
> >> -           as_bad (_("TLS relocation cannot be used with `%s'"), insn_name
> (&i.tm));
> >> -           return;
> >> +           for (unsigned int k = 0; k < ARRAY_SIZE (gotrel); k++)
> >> +             {
> >> +               if (gotrel[k].rel[object_64bit] == i.reloc[j])
> >> +                 {
> >> +                   as_bad (_("%s relocation cannot be used with `%s'"),
> >> +                         gotrel[k].str, insn_name (&i.tm));
> 
> Please can you make this @%s to match what's used in source? That'll at least
> somewhat disambiguate the name space (as "relocation" could also mean
> other things).
> 
> > This is what I am checking in.
> 
> I'm sorry for repeating myself, but can you please, please wait some before
> committing patches? How in the world am I to respond if you send a patch
> after my workday ended and you commit it just a couple of hours later? Just
> mention it once again - I am usually giving it a week for people to comment.
> I'd really expect the same from you.
> 

In our discussion email, the requirements of these two patches were very clear, and my patch depends on these two patches. I think this is the main reason why H.J checked them in yesterday😊.

Lili.
> Jan