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

[Fangrui Song <maskray@google.com>]

 On Fri, Jan 6, 2023 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 6, 2023 at 1:44 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On Fri, Jan 6, 2023 at 1:27 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Fri, Jan 6, 2023 at 1:25 PM Fangrui Song <maskray@google.com> wrote:
> > > >
> > > >  On Fri, Jan 6, 2023 at 1:14 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > On Fri, Jan 6, 2023 at 10:48 AM Fangrui Song <maskray@google.com> wrote:
> > > > > >
> > > > > > On Fri, Jan 6, 2023 at 9:04 AM H.J. Lu <hjl.tools@gmail.com> 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.
> > > > > > >
> > > > > > > --
> > > > > > > H.J.
> > > > > >
> > > > > > I have multiple arguments (albeit no single one is very strong) that
> > > > > > this 1-deletion-1-addition change provides benefits for users (IMHO
> > > > > > with no burden to binutils at all).
> > > > > >
> > > > > > Some projects may add -Wa,-mrelax-relocations=no to work around older
> > > > > > GNU ld. Then the project's toolchain requirement may increase and no
> > > > > > longer need to work around older GNU ld.
> > > > > > But a distribution may for some reason use a global -fno-plt (e.g.
> > > > > > Arch Linux) and then run into this TLS GD/LD->IE/LE optimization
> > > > > > issue.
> > > > > >
> > > > > > rust src/ci/docker/host-x86_64/*musl/Dockerfile
> > > > > > openjdk/jdk19u make/autoconf/flags-cflags.m4 (this file appears to be
> > > > > > copied into quite a few projects)
> > > > > > Linux kernel arch/x86/boot/compressed/Makefile (not a good example as
> > > > > > it doesn't use TLS AFAICT)
> > > > > >
> > > > > > R_X86_64_GOTPCREL isn't purely usefull. It may help linker design: for
> > > > > > R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX, the linker can make a
> > > > > > decision upfront whether a GOT entry is needed
> > > > > > (this affects the size of .got, which may affect section layout and
> > > > > > whether other relocations may overflow).
> > > > > > This may increase risk of 32-bit relocation overflow.
> > > > > > R_X86_64_GOTPCREL can mitigate the risk while being aware to the user.
> > > > > >
> > > > > > rustc somehow disables x86 relaxed relocations and defaults to `-Z
> > > > >
> > > > > Why is that?
> > > >
> > > > It's assuredly a rust's problem and I am trying to fix that in
> > > > https://github.com/rust-lang/rust/pull/106511
> > > >
> > > > The  -Wa,-mrelax-relocations=no problem may affect more packages.
> > >
> > > -mrelax-relocations=no should be a workaround for the older linker.   It
> > > shouldn't be used with the current linker.
> >
> > A project may choose to work with many linker versions.
> > For simplicity, before it decides to drop compatibility with GNU
> > ld<2.26 (AIUI GOTPCRELX was supported in 2.26),
> > it may unconditionally add -Wa,-mrelax-relocations=no, instead of
>
> -mrelax-relocations=no is only supported with the newer binutils.

The relocatable object file producer and the consumer may be on
different machines and use different binutils versions.
https://github.com/rust-lang/rust/commit/305aca86f9d8d132650b495f610f9abe5239fec6
added -Wa,-mrelax-relocations=no so that the relocatable object files
can be used on a user machine with an old ld.
https://github.com/IHaskell/IHaskell/issues/636 and
https://github.com/dcos/dcos/commit/facda25019e07051f501b39720b4e71049bd0030
likely use the same argument.

In other cases, the project may use -Wa,-mrelax-relocations=no with
Clang (where they assume a not-too-old version), but need to work with
system ld (which may be old).

> > doing configure work to check linker support.
>
> The TLS sequence from -fno-plt doesn't work for the older linker.
> The older linker support should be dropped for -fno-plt.
>
> > Now a user may use -fno-plt (Arch Linux, rustc, maybe Alpine) and run
> > into the aforementioned TLS problem.
> >
> > This 1-deletion-1-addition change can address this issue with no
> > maintenance burden on binutils side in my opinion,
> > so I made this patch.
> >
> > The linker design I described is true as well. Whether GOTPCRELX leads
> > to a GOT entry can be decided at relocation scanning time, before the
> > section layout is decided.
> > Users may make a conscious decision to use GOTPCREL to avoid potential
> > relocation overflow risk.
> >
> > GOTPCREL isn't really dead. It can be used with Intel LAM and tagged
> > global variables (with non-zero high address bits)
> > https://reviews.llvm.org/D111343
> > GOTPCREL instead of GOTPCRELX makes it clear an instruction
> > referencing the variable isn't supposed to be relaxed.
>
> The address of the local symbol, foo, in
>
> movq foo@GOTPCREL(%rip), %rax
>
> is assigned by the linker.  I am not sure how the tag is involved here.
> Besides, it is the call instruction here.

This is an auxiliary argument. I wanted to emphasize that GOTPCREL
isn't dead and did not intend to use it with this call instruction.
If GOTPCRELX is used and the distance between the current location and
the symbol is larger than 2**31, this will trigger a relocation
overflow.
This happens with tagged globals with non-zero high address bits.
A linker can fix the problem by avoiding relaxation, increasing the
size of .got . This requires that it scans relocations more than once.
If GOTPCRELX is decided upfront whether it needs relaxation or not, on
an arch which doesn't use range extension thunks like x86, technically
relocations can just be scanned once.

> > > > > > plt=no` and now relies on llvm-project to work around the GNU ld
> > > > > > compatibility issue.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > 宋方睿
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > H.J.
> > > >
> > > >
> > > >
> > > > --
> > > > 宋方睿
> > >
> > >
> > >
> > > --
> > > H.J.
> >
> >
> >
> > --
> > 宋方睿
>
>
>
> --
> H.J.



-- 
宋方睿