Re: [PATCH] ld: Allow R_X86_64_GOTPCREL for call *__tls_get_addr@GOTPCREL(%rip)

[Jan Beulich <jbeulich@suse.com>]

On 10.01.2023 22:02, Fangrui Song wrote:
> On Tue, Jan 10, 2023 at 12:40 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jan 10, 2023 at 1:16 AM Jan Beulich <jbeulich@suse.com> wrote:
>>> On 09.01.2023 22:14, H.J. Lu wrote:
>>>> On Mon, Jan 9, 2023 at 12:15 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 06.01.2023 18:03, H.J. Lu via Binutils wrote:
>>>>>> On Thu, Jan 5, 2023 at 1:06 PM Fangrui Song via Binutils
>>>>>> <binutils@sourceware.org> wrote:
>>>>>>>
>>>>>>> _Thread_local int a;
>>>>>>> int main() { return a; }
>>>>>>>
>>>>>>> % gcc -fno-plt -fpic a.c -fuse-ld=bfd -Wa,-mrelax-relocations=no
>>>>>>> /usr/bin/ld.bfd: /tmp/ccSSBgrg.o: TLS transition from R_X86_64_TLSGD to R_X86_64_GOTTPOFF against `a' at 0xd in section `.text' failed
>>>>>>> /usr/bin/ld.bfd: failed to set dynamic section sizes: bad value
>>>>>>> collect2: error: ld returned 1 exit status
>>>>>>>
>>>>>>> This commit fixes the issue.
>>>>>>>
>>>>>>>     PR ld/24784
>>>>>>>     * bfd/elf64-x86-64.c (elf_x86_64_check_tls_transition): Allow
>>>>>>>       R_X86_64_GOTPCREL.
>>>>>>> ---
>>>>>>>  bfd/elf64-x86-64.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
>>>>>>> index 914f82d0151..095fe2e0fe6 100644
>>>>>>> --- a/bfd/elf64-x86-64.c
>>>>>>> +++ b/bfd/elf64-x86-64.c
>>>>>>> @@ -1241,7 +1241,7 @@ elf_x86_64_check_tls_transition (bfd *abfd,
>>>>>>>           if (largepic)
>>>>>>>             return r_type == R_X86_64_PLTOFF64;
>>>>>>>           else if (indirect_call)
>>>>>>> -           return r_type == R_X86_64_GOTPCRELX;
>>>>>>> +           return (r_type == R_X86_64_GOTPCRELX || r_type == R_X86_64_GOTPCREL);
>>>>>>>           else
>>>>>>>             return (r_type == R_X86_64_PC32 || r_type == R_X86_64_PLT32);
>>>>>>>         }
>>>>>>> --
>>>>>>> 2.39.0.314.g84b9a713c41-goog
>>>>>>>
>>>>>>
>>>>>> Since the new TLS sequence was added after R_X86_64_GOTPCRELX was
>>>>>> required for call, R_X86_64_GOTPCREL should be invalid in this TLS sequence.
>>>>>
>>>>> While this may well be, would you mind pointing out (more to Fangrui than to
>>>>> me) what bad his proposed change would do?
>>>>
>>>> The problem is caused by the combination of -fno-plt and
>>>> -Wa,-mrelax-relocations=no.
>>>> -Wa,-mrelax-relocations=no was added to generate object files to be
>>>> consumed by the
>>>> older linkers.   On the other hand, -fno-plt requires newer linkers.
>>>> As the result,
>>>>  -fno-plt -Wa,-mrelax-relocations=no generates object files which
>>>> aren't compatible
>>>> with neither older linkers nor newer linkers.
>>>> -Wa,-mrelax-relocations shouldn't be used
>>>> together with -fno-plt.
>>>
>>> Imo use of such option combinations should either be disallowed (warned
>>> about at the very least) or produce sensible output. I guess only the
>>> latter would help Fangrui ...
>>>
>>
>> This isn't a supported combination.  I believe -Wa,-mrelax-relocations=no
>> should be removed.
> 
> Removing -Wa,-mrelax-relocations=no implies that R_X86_64_GOTPCREL is
> completely useless and -Wl,--no-relax is not useful for x86.
> 
> As my earliest replies mentioned, a relocation type indication no
> relaxation is useful in some cases: hwasan (Intel LAM) references to
> global variables, one-pass relocation scanning design in a linker,
> even if we disregard the relocatable-file-producer with
> old-linker-consumer compatibility scenarios.
> 
> If the linker can decide upfront whether GOTPCREL{,X} needs a GOT
> entry, the relocation scanning pass be one-pass and be completely
> moved before synthetic sections (.got, .plt, .got.plt, etc), instead
> of interleaving relocation scanning, synthetic section size decision,
> and section layout.

I agree with Fangrui's view here. I further think that the original change
was (once again) overly limiting applicability (why ADC and SBB but not
e.g. ADCX and ADOX) or doing things in an overly relaxed fashion (the
subsequently addressed 16-bit operand size issue being just one aspect;
I wonder whether 32-bit vs 64-bit operand size is actually handled
correctly there in 64-bit and/or x32 mode).

Jan