Re: [PATCH 0/4] RISC-V: Implement TLS Descriptors.

[Fangrui Song <i@maskray.me>]

On Thu, Aug 17, 2023 at 5:22 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
> Hi,
>
> Before the reviewing, I think you still should have test cases for each
> patch to make sure everything is correct in the binutils, and helps people
> understand what the patches try to resolve what problem or support what
> features.  In general, the regressions in binutils, gcc and glibc are
> seperate.  Besides, each patch should at least have the ChangLogs in the
> commit message.
>
> Thanks
> Nelson

A relocatable object file can contain both TLSDESC and TLS GD
relocations (e.g. relocatable output).
The GOT use cases are different. It will be nice to have such a test.

We need tests for at least (a) -shared and (b) -no-pie or -pie (I
prefer just -no-pie).

Perhaps you have done this (as the glibc patch suggests), but I want
to make sure that the GOT entries are placed in .rela.dyn, not
.rela.plt
(https://sourceware.org/bugzilla/show_bug.cgi?id=28387  ld: Move
R_*_TLSDESC to .rela.dyn)
sysdeps/{aarch64,x86_64}/dl-machine.h unfortunately have to handle
TLSDESC in elf_machine_lazy_rel for compatibility.

> [PATCH 4/4] RISC-V: Initial ld.bfd support for TLSDESC.
> + /* TLSDESC needs one dynamic reloc and four GOT slots. *

Why is there 4? Other ports reserve 2 GOT slots for one symbol.

> On Fri, Aug 18, 2023 at 2:10 AM Tatsuyuki Ishi via Binutils <
> binutils@sourceware.org> wrote:
>
> > This patchset implements TLS Descriptors (TLSDESC) for RISC-V targets, per
> > the specification draft at [1].
> >
> > This patchset is based on top of [2].
> >
> > No regression in binutils and gcc tests for rv64gc, tested alongside the
> > gcc and glibc implementation (will be posted shortly).
> >
> > This contribution is made on behalf of Blue Whale Systems, which has
> > copyright assignment on file with the FSF.
> >
> > [1]: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/373
> > [2]: https://sourceware.org/pipermail/binutils/2023-August/129075.html
> >
> > Tatsuyuki Ishi (4):
> >   RISC-V: Add TLSDESC reloc definitions.
> >   RISC-V: Add assembly support for TLSDESC.
> >   RISC-V: Define and use GOT entry size constants for TLS.
> >   RISC-V: Initial ld.bfd support for TLSDESC.
> >
> >  bfd/bfd-in2.h                              |   4 +
> >  bfd/elfnn-riscv.c                          | 105 ++++++++++++++++++---
> >  bfd/elfxx-riscv.c                          |  75 ++++++++++++++-
> >  bfd/libbfd.h                               |   4 +
> >  bfd/reloc.c                                |   8 ++
> >  gas/config/tc-riscv.c                      |  18 +++-
> >  include/elf/riscv.h                        |   5 +
> >
>
>
> >  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp |   2 +
> >
>
> This doesn't work, seems still not adding any test cases.
>
>
> >  opcodes/riscv-opc.c                        |   1 +
> >  9 files changed, 201 insertions(+), 21 deletions(-)
> >
> > --
> > 2.41.0
> >
> >