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

[Fangrui Song <maskray@google.com>]

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

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.