[PATCH V5] x86: Add tls check in gas

[Jan Beulich]

On 27.09.2024 01:12, H.J. Lu wrote:
> On Wed, Sep 25, 2024 at 10:29 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 25.09.2024 14:45, Cui, Lili wrote:
>>>> Subject: Re: [PATCH V5] x86: Add tls check in gas
>>>>
>>>> On 25.09.2024 05:32, Cui, Lili wrote:
>>>>>> I've grabbed the newest tls.pdf I could find (still over 10 years
>>>>>> old, from Ulrich Drepper's site as referenced by the Thread-Local
>>>>>> Storage section in gcc14 doc), and that raises questions:
>>>>>
>>>>> Jan,  you can find the latest TLS in ABI chapter 10
>>>>> (https://gitlab.com/x86-psABIs/x86-64-ABI)
>>>>
>>>> So far all my questions were on the i386 side of things, which isn't covered there.
>>>>
>>>>> and some TLS doc linkers there (e.g.
>>>>> http://www.akkadia.org/drepper/tls.pdf)
>>>>
>>>> This is the doc I was referring to.
>>>>
>>>> As an aside - of course it shouldn't really take two docs to get a complete picture.
>>>
>>> At least you can find " GDesc access model " in ABI doc.
>>
>> But not what exactly it means on i386. Are we here dealing with private,
>> undocumented extensions to the TLS spec?
>>
>>>>>> On 19.09.2024 08:38, Cui, Lili wrote:
>>>>>>> @@ -6357,6 +6387,356 @@ 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 (enum bfd_reloc_code_real r_type) {
>>>>>>> +  switch (r_type)
>>>>>>> +    {
>>>>>>> +    case BFD_RELOC_386_TLS_GOTDESC:
>>>>>>> +      /* Check GDesc access model:
>>>>>>
>>>>>> What's "GDesc access model"? I can't find anything named like this,
>>>>>> nor mention of ...
>>>>>>> +  leal x@tlsdesc(%ebx), %reg32 --> Memory reg must be %ebx and
>>>>>>> +                                   SIB is not supported.
>>>>>>> +       */
>>>>>>
>>>>>> ... @tlsdesc.
>>>>>
>>>>> It is defined in ABI doc.
>>>>
>>>> As per above - where? I can see the 64-bit counterpart of this being described in
>>>> the x86-64 psABI, but that's not covering the case here.
>>>>
>>>>>>> +      if (i.tm.mnem_off != MN_lea)
>>>>>>> + return x86_tls_error_insn;
>>>>>>> +      if (i.index_reg)
>>>>>>> + return x86_tls_error_sib;
>>>>>>> +      if (!i.base_reg)
>>>>>>> + return x86_tls_error_no_base_reg;
>>>>>>> +      if (i.base_reg->reg_type.bitfield.instance != RegB)
>>>>>>> + return x86_tls_error_ebx;
>>>>>>> +      if (!i.op[1].regs->reg_type.bitfield.dword)
>>>>>>> + return x86_tls_error_dest_32bit_reg_size;
>>>>>>> +      break;
>>>>>>> +
>>>>>>> +    case BFD_RELOC_386_TLS_GD:
>>>>>>> +      /* Check GD access model:
>>>>>>> +
>>>>>>> +  leal foo@tlsgd(,%ebx,1), %eax   --> Only this fixed format is supported.
>>>>>>> +  leal foo@tlsgd(%reg32), %eax    --> Dest reg must be '%eax'
>>>>>>> +                                      Memory reg can't be %eax.
>>>>>>> +       */
>>>>>>
>>>>>> Where's this 2nd form coming from? The doc says the SIB form must be
>>>>>> used, and the converted-to code sequence would require one more byte
>>>>>> than the non- SIB form provides space for.
>>>>>>
>>>>>> The doc doesn't say that the index reg has to be %ebx, it merely
>>>>>> mentions that in the example, for being the commonly used GOT reg.
>>>>>> This is the same ...
>>>>>
>>>>> It is mentioned in 4.1 of tls.pdf.
>>>>
>>>> Where? Please quote the sentence. I can't spot anything saying that it has to
>>>> be %ebx. And similarly I can't spot anything indicating the non-SIB form would
>>>> be okay to use; to the contrary it says "Note the form of the first operand of leal
>>>> which forces the use of the SIB form ...". While that leaves open the use of a NOP
>>>> (or address size override on the associated CALL, as per the checking the linker
>>>> does) together with the non-SIB form,
>>>
>>> This is the place, tls.pdf is encrypted and cannot be copied. I typed a sentence.
>>>
>>> "To signal that this instruction is for the GNU variant of the access model the syntax @tlsgd(%ebx) is used."
>>>
>>> It mentioned @tlsgd(%ebx),
>>
>> As you say - mentions. It doesn't say that's the only valid form. Instead ...
>>
>>> but I think the details are not clear enough, after all, the documentation has not been updated for 10 years.
>>
>> ... it seems pretty obvious that if any reg other than %eax is fine for the
>> non-SIB form, the same is true for the SIB one. The two forms are solely
>> different encodings of the same EA.
>>
>>> This place needs to be confirmed with the author.
>>
>> First of all I'd like to understand the origin of the non-SIB form, and
>> thus (hopefully) why that permits more registers than the SIB one.
> 
> These assembler checks are the same as the linker checks.
> Unless there is a real workload to show the linker check is
> incorrect, I don't believe we should check the linker.

Here we go again - you apply personal preferences. For those APX insn
promotions you've insisted on us sticking to the spec. Here you say you
want to keep the linker as is, no matter whether it matches the spec
(wherever the missing parts thereof are). I'm afraid I'd like to ask
that you make up your mind in a consistent way: Do specs matter, or can
we take arbitrary liberties? My view is that in both cases we could
permit sensible extensions, as long as those are sufficiently clear to
be in line with the original intentions of the spec. In the case here
that of course would first mean to properly identify what the extension
parts are. And then any extensions of course want to be consistent with
the original spec as well as self-consistent.

Jan