[v3] RISC-V: Implement TLS Descriptors.

Message ID 20230913172633.39229-1-ishitatsuyuki@gmail.com
State Superseded
Headers
Series [v3] RISC-V: Implement TLS Descriptors. |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Tatsuyuki Ishi Sept. 13, 2023, 5:26 p.m. UTC
  This is mostly based off AArch64 implementation, with some adaptations
to different TLS DTV offsets and calling conventions.

No regression in binutils and gcc tests for rv64gc, tested alongside the
gcc and binutils implementation (posted at the same time).
---
v2: Fix end-of-file newlines.
v3: Fix segfaulting on the slow path of TLSDESC resolver.
    Fix handling of lazy relocations.

This contribution is made on behalf of Blue Whale Systems, which has
copyright assignment on file with the FSF.

 sysdeps/riscv/Makefile       |   8 ++
 sysdeps/riscv/dl-lookupcfg.h |  27 +++++
 sysdeps/riscv/dl-machine.h   |  48 ++++++++-
 sysdeps/riscv/dl-tlsdesc.S   | 203 +++++++++++++++++++++++++++++++++++
 sysdeps/riscv/dl-tlsdesc.h   |  49 +++++++++
 sysdeps/riscv/linkmap.h      |   1 +
 sysdeps/riscv/tlsdesc.c      |  38 +++++++
 sysdeps/riscv/tlsdesc.sym    |  19 ++++
 8 files changed, 392 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/riscv/dl-lookupcfg.h
 create mode 100644 sysdeps/riscv/dl-tlsdesc.S
 create mode 100644 sysdeps/riscv/dl-tlsdesc.h
 create mode 100644 sysdeps/riscv/tlsdesc.c
 create mode 100644 sysdeps/riscv/tlsdesc.sym
  

Comments

Adhemerval Zanella Netto Sept. 13, 2023, 7:14 p.m. UTC | #1
On 13/09/23 14:26, Tatsuyuki Ishi wrote:
> This is mostly based off AArch64 implementation, with some adaptations
> to different TLS DTV offsets and calling conventions.
> 
> No regression in binutils and gcc tests for rv64gc, tested alongside the
> gcc and binutils implementation (posted at the same time).

How did you actually build glibc? I saw multiple build issues with default
configuration and even with --disable-werror, so I am doubtful that this
patch was really proper tested. Please ensure that have-mtls-dialect-gnu2
is set to 'yes' on config.make so the tests are actually run.

The lack of include guards at dl-tls.h make the sysdeps/riscv/tlsdesc.sym 
generation fail with confling types:

In file included from ../sysdeps/riscv/dl-tlsdesc.h:23,
                 from <stdin>:6:
../sysdeps/riscv/dl-tls.h:25:3: error: conflicting types for ‘tls_index’; have ‘struct <anonymous>’
   25 | } tls_index;
      |   ^~~~~~~~~
In file included from <stdin>:5:
../sysdeps/riscv/dl-tls.h:25:3: note: previous declaration of ‘tls_index’ with type ‘tls_index’
   25 | } tls_index;
      |   ^~~~~~~~~
../sysdeps/riscv/dl-tls.h:42:14: error: conflicting types for ‘__tls_get_addr’; have ‘void *(tls_index *)’
   42 | extern void *__tls_get_addr (tls_index *ti);
      |              ^~~~~~~~~~~~~~
../sysdeps/riscv/dl-tls.h:42:14: note: previous declaration of ‘__tls_get_addr’ with type ‘void *(tls_index *)’
   42 | extern void *__tls_get_addr (tls_index *ti);
      |              ^~~~~~~~~~~~~~

Removing '#include <dl-tls.h>' fixes it, but it fails later trying
to build dl-load. It is better to add proper guards:

diff --git a/sysdeps/riscv/dl-tls.h b/sysdeps/riscv/dl-tls.h
index 67c8ae639c..6c569509bd 100644
--- a/sysdeps/riscv/dl-tls.h
+++ b/sysdeps/riscv/dl-tls.h
@@ -16,6 +16,8 @@
    License along with the GNU C Library.  If not, see
    <https://www.gnu.org/licenses/>.  */

+#ifndef _RISCV_DL_TLS_H
+#define _RISCV_DL_TLS_H

 /* Type used for the representation of TLS information in the GOT.  */
 typedef struct
@@ -46,3 +48,5 @@ extern void *__tls_get_addr (tls_index *ti);

 /* Value used for dtv entries for which the allocation is delayed.  */
 #define TLS_DTV_UNALLOCATED    ((void *) -1l)
+
+#endif /* _RISCV_DL_TLS_H */


There are other build failures as well.

> ---
> v2: Fix end-of-file newlines.
> v3: Fix segfaulting on the slow path of TLSDESC resolver.
>     Fix handling of lazy relocations.
> 
> This contribution is made on behalf of Blue Whale Systems, which has
> copyright assignment on file with the FSF.
> 
>  sysdeps/riscv/Makefile       |   8 ++
>  sysdeps/riscv/dl-lookupcfg.h |  27 +++++
>  sysdeps/riscv/dl-machine.h   |  48 ++++++++-
>  sysdeps/riscv/dl-tlsdesc.S   | 203 +++++++++++++++++++++++++++++++++++
>  sysdeps/riscv/dl-tlsdesc.h   |  49 +++++++++
>  sysdeps/riscv/linkmap.h      |   1 +
>  sysdeps/riscv/tlsdesc.c      |  38 +++++++
>  sysdeps/riscv/tlsdesc.sym    |  19 ++++
>  8 files changed, 392 insertions(+), 1 deletion(-)
>  create mode 100644 sysdeps/riscv/dl-lookupcfg.h
>  create mode 100644 sysdeps/riscv/dl-tlsdesc.S
>  create mode 100644 sysdeps/riscv/dl-tlsdesc.h
>  create mode 100644 sysdeps/riscv/tlsdesc.c
>  create mode 100644 sysdeps/riscv/tlsdesc.sym
> 
> diff --git a/sysdeps/riscv/Makefile b/sysdeps/riscv/Makefile
> index 8fb10b164f..bb4bcd29b2 100644
> --- a/sysdeps/riscv/Makefile
> +++ b/sysdeps/riscv/Makefile
> @@ -2,6 +2,14 @@ ifeq ($(subdir),misc)
>  sysdep_headers += sys/asm.h
>  endif
>  
> +ifeq ($(subdir),elf)
> +sysdep-dl-routines += tlsdesc dl-tlsdesc
> +endif
> +
> +ifeq ($(subdir),csu)
> +gen-as-const-headers += tlsdesc.sym
> +endif
> +


Minor style issue, for new code we are prefering a new line per entry,
alphabetically sorted: 

  ifeq ($(subdir),elf)
  sysdep-dl-routines += \
    dl-tlsdesc
    tlsdesc \
    # routines
  endif

  ifeq ($(subdir),csu)
  gen-as-const-headers += \
    tlsdesc.sym \
    # gen-as-const-headers
  endif

>  # RISC-V's assembler also needs to know about PIC as it changes the definition
>  # of some assembler macros.
>  ASFLAGS-.os += $(pic-ccflag)
> diff --git a/sysdeps/riscv/dl-lookupcfg.h b/sysdeps/riscv/dl-lookupcfg.h
> new file mode 100644
> index 0000000000..d7fe73636b
> --- /dev/null
> +++ b/sysdeps/riscv/dl-lookupcfg.h
> @@ -0,0 +1,27 @@
> +/* Configuration of lookup functions.
> +   Copyright (C) 2006-2023 Free Software Foundation, Inc.

I think it should be only 2023 for new code.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define DL_UNMAP_IS_SPECIAL
> +
> +#include_next <dl-lookupcfg.h>
> +
> +struct link_map;
> +
> +extern void _dl_unmap (struct link_map *map);
> +
> +#define DL_UNMAP(map) _dl_unmap (map)
> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> index c0c9bd93ad..eb0c874e72 100644
> --- a/sysdeps/riscv/dl-machine.h
> +++ b/sysdeps/riscv/dl-machine.h
> @@ -25,6 +25,7 @@
>  #include <elf/elf.h>
>  #include <sys/asm.h>
>  #include <dl-tls.h>
> +#include <dl-tlsdesc.h>
>  #include <dl-irel.h>
>  #include <dl-static-tls.h>
>  #include <dl-machine-rel.h>
> @@ -50,7 +51,8 @@
>       || (__WORDSIZE == 32 && (type) == R_RISCV_TLS_TPREL32)	\
>       || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPREL64)	\
>       || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPMOD64)	\
> -     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64)))	\

This seems wrong, even with a R_RISCV_TLSDESC definition compiler fails
with:

dl-reloc.c: In function ‘resolve_map’:
../sysdeps/riscv/dl-machine.h:48:25: error: ‘*’ in boolean context, suggest ‘&&’ instead [-Werror=int-in-bool-context]
   48 |   ((ELF_RTYPE_CLASS_PLT * ((type) == ELF_MACHINE_JMP_SLOT       \
dl-reloc.c:175:10: note: in expansion of macro ‘elf_machine_type_class’
  175 |       && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class)

I think you want to add the final ')' *after* R_RISCV_TLSDESC.  Something
like:

diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
index eb0c874e72..b10dfe4954 100644
--- a/sysdeps/riscv/dl-machine.h
+++ b/sysdeps/riscv/dl-machine.h
@@ -51,8 +51,8 @@
      || (__WORDSIZE == 32 && (type) == R_RISCV_TLS_TPREL32)    \
      || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPREL64)   \
      || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPMOD64)   \
-     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))   \
-     || ((type) == R_RISCV_TLSDESC))                           \
+     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64)    \
+     || ((type) == R_RISCV_TLSDESC)))                          \
    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))

 /* Return nonzero iff ELF header is compatible with the running host.  */

> +     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))	\
> +     || ((type) == R_RISCV_TLSDESC))				\

R_RISCV_TLSDESC is not define in this patch, you need to either sync with
binutils elf.h or add it on this patch.

>     | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
>  
>  /* Return nonzero iff ELF header is compatible with the running host.  */
> @@ -219,6 +221,32 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  	}
>        break;
>  
> +    case R_RISCV_TLSDESC:
> +      struct tlsdesc *td = (struct tlsdesc *) addr_field;
> +      if (sym == NULL)
> +	{
> +	  td->entry = _dl_tlsdesc_undefweak;


This triggers multiple compiler warnings:

../sysdeps/riscv/dl-machine.h: In function ‘elf_machine_rela’:
../sysdeps/riscv/dl-machine.h:228:21: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
  228 |           td->entry = _dl_tlsdesc_undefweak;
      |                     ^
../sysdeps/riscv/dl-machine.h:244:25: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
  244 |               td->entry = _dl_tlsdesc_return;
      |                         ^

Because you declare _dl_tlsdesc_undefweak as:

  unsigned long _dl_tlsdesc_dynamic (struct tlsdesc *);

But the 'entry' at tlsdesc as:

   ptrdiff_t (*entry) (struct tlsdesc *);

Based on TLSDESC ABI I think using a unsigned as return value is wrong here.

> +	  td->arg = reloc->r_addend;

This triggers another build issue:

../sysdeps/riscv/dl-machine.h: In function ‘elf_machine_rela’:
../sysdeps/riscv/dl-machine.h:229:19: error: assignment to ‘void *’ from ‘Elf64_Sxword’ {aka ‘long int’} makes pointer from integer without a cast [-Werror=int-conversion]
  229 |           td->arg = reloc->r_addend;

You need to explicit cast to 'void *' here.


> +	}
> +      else
> +	{
> +# ifndef SHARED
> +	  CHECK_STATIC_TLS (map, sym_map);
> +# else
> +	  if (!TRY_STATIC_TLS (map, sym_map))
> +	    {
> +	      td->entry = _dl_tlsdesc_dynamic;
> +	      td->arg = _dl_make_tlsdesc_dynamic (sym_map, sym->st_value + reloc->r_addend);
> +	    }
> +	  else
> +# endif
> +	    {
> +	      td->entry = _dl_tlsdesc_return;
> +	      td->arg = (void *)(TLS_TPREL_VALUE(sym_map, sym) + reloc->r_addend);


Space after TLS_TPREL_VALUE

> +	    }
> +	}
> +      break;
> +
>      case R_RISCV_COPY:
>        {
>  	if (__glibc_unlikely (sym == NULL))
> @@ -289,6 +317,24 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>        else
>  	*reloc_addr = map->l_mach.plt;
>      }
> +  else if (__glibc_likely (r_type == R_RISCV_TLSDESC))
> +    {
> +      const Elf_Symndx symndx = ELFW (R_SYM) (reloc->r_info);
> +      const ElfW (Sym) *symtab = (const void *)D_PTR (map, l_info[DT_SYMTAB]);
> +      const ElfW (Sym) *sym = &symtab[symndx];
> +      const struct r_found_version *version = NULL;
> +
> +      if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
> +	{
> +	  const ElfW (Half) *vernum =
> +	      (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]);
> +	  version = &map->l_versions[vernum[symndx] & 0x7fff];
> +	}
> +
> +      /* Always initialize TLS descriptors completely, because lazy
> +	 initialization requires synchronization at every TLS access.  */
> +      elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, skip_ifunc);
> +    }
>    else if (__glibc_unlikely (r_type == R_RISCV_IRELATIVE))
>      {
>        ElfW(Addr) value = map->l_addr + reloc->r_addend;
> diff --git a/sysdeps/riscv/dl-tlsdesc.S b/sysdeps/riscv/dl-tlsdesc.S
> new file mode 100644
> index 0000000000..bf241ef76a
> --- /dev/null
> +++ b/sysdeps/riscv/dl-tlsdesc.S
> @@ -0,0 +1,203 @@
> +/* Thread-local storage handling in the ELF dynamic linker.
> +   RISC-V version.
> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +#include <tls.h>
> +#include <tlsdesc.h>
> +
> +#ifdef __riscv_float_abi_soft
> +# define FRAME_SIZE (-((-12 * SZREG) & ALMASK))
> +#else
> +# define FRAME_SIZE (-((-12 * SZREG - 20 * SZFREG) & ALMASK))
> +#endif
> +
> +	.text
> +
> +	/* Compute the thread pointer offset for symbols in the static
> +	   TLS block. The offset is the same for all threads.
> +	   Prototype:
> +	   _dl_tlsdesc_return (tlsdesc *) ;
> +	 */
> +ENTRY (_dl_tlsdesc_return)
> +	REG_L a0, TLSDESC_ARG(a0)
> +	jr t0
> +END (_dl_tlsdesc_return)
> +
> +	/* Handler for undefined weak TLS symbols.
> +	   Prototype:
> +	   _dl_tlsdesc_undefweak (tlsdesc *);
> +
> +	   The second word of the descriptor contains the addend.
> +	   Return the addend minus the thread pointer. This ensures
> +	   that when the caller adds on the thread pointer it gets back
> +	   the addend.  */
> +
> +ENTRY (_dl_tlsdesc_undefweak)
> +	REG_L a0, TLSDESC_ARG(a0)
> +	sub a0, a0, tp
> +	jr t0
> +END (_dl_tlsdesc_undefweak)
> +
> +#ifdef SHARED
> +	/* Handler for dynamic TLS symbols.
> +	   Prototype:
> +	   _dl_tlsdesc_dynamic (tlsdesc *) ;
> +
> +	   The second word of the descriptor points to a
> +	   tlsdesc_dynamic_arg structure.
> +
> +	   Returns the offset between the thread pointer and the
> +	   object referenced by the argument.
> +
> +	   unsigned long
> +	   _dl_tlsdesc_dynamic (struct tlsdesc *tdp)
> +	   {
> +	     struct tlsdesc_dynamic_arg *td = tdp->arg;
> +	     dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + TCBHEAD_DTV);
> +	     if (__builtin_expect (td->gen_count <= dtv[0].counter
> +		&& (dtv[td->tlsinfo.ti_module].pointer.val
> +		    != TLS_DTV_UNALLOCATED),
> +		1))
> +	       return dtv[td->tlsinfo.ti_module].pointer.val
> +		+ td->tlsinfo.ti_offset
> +		- __thread_pointer;
> +
> +	     return ___tls_get_addr (&td->tlsinfo) - __thread_pointer;
> +	   }
> +	 */
> +
> +ENTRY (_dl_tlsdesc_dynamic)
> +	/* Save just enough registers to support fast path, if we fall
> +	   into slow path we will save additional registers.  */
> +	add	sp, sp, -3*SZREG
> +	REG_S	t0, 0*SZREG(sp)
> +	REG_S	t1, 1*SZREG(sp)
> +	REG_S	t2, 2*SZREG(sp)
> +
> +	/* t0 = dtv */
> +	REG_L	t0, TCBHEAD_DTV(tp)
> +	/* a0 = tdp->arg */
> +	REG_L	a0, TLSDESC_ARG(a0)
> +	/* t1 = td->gen_count */
> +	REG_L	t1, TLSDESC_GEN_COUNT(a0)
> +	/* t2 = dtv[0].counter */
> +	REG_L	t2, DTV_COUNTER(t0)
> +	bltu	t2, t1, .Lslow
> +	/* t1 = td->tlsinfo.ti_module */
> +	REG_L	t1, TLSDESC_MODID(a0)
> +	slli	t1, t1, PTRLOG + 1 /* sizeof(dtv_t) == sizeof(void*) * 2 */
> +	add	t1, t1, t0
> +	/* t1 = dtv[td->tlsinfo.ti_module].pointer.val  */
> +	REG_L	t1, 0(t1)
> +	li	t2, TLS_DTV_UNALLOCATED
> +	beq	t1, t2, .Lslow
> +	/* t2 = td->tlsinfo.ti_offset */
> +	REG_L	t2, TLSDESC_MODOFF(a0)
> +	add	a0, t1, t2
> +.Lret:
> +	sub	a0, a0, tp
> +	REG_L	t0, 0*SZREG(sp)
> +	REG_L	t1, 1*SZREG(sp)
> +	REG_L	t2, 2*SZREG(sp)
> +	add	sp, sp, 3*SZREG
> +	jr t0
> +.Lslow:
> +	/* This is the slow path. We need to call __tls_get_addr() which
> +	   means we need to save and restore all the register that the
> +	   callee will trash.  */
> +
> +	/* Save the remaining registers that we must treat as caller save.  */
> +	addi	sp, sp, -FRAME_SIZE
> +	REG_S	ra, 0*SZREG(sp)
> +	REG_S	a1, 1*SZREG(sp)
> +	REG_S	a2, 2*SZREG(sp)
> +	REG_S	a3, 3*SZREG(sp)
> +	REG_S	a4, 4*SZREG(sp)
> +	REG_S	a5, 5*SZREG(sp)
> +	REG_S	a6, 6*SZREG(sp)
> +	REG_S	a7, 7*SZREG(sp)
> +	REG_S	t3, 8*SZREG(sp)
> +	REG_S	t4, 9*SZREG(sp)
> +	REG_S	t5, 10*SZREG(sp)
> +	REG_S	t6, 11*SZREG(sp)
> +#ifndef __riscv_float_abi_soft
> +	FREG_S	ft0, (12*SZREG + 0*SZFREG)(sp)
> +	FREG_S	ft1, (12*SZREG + 1*SZFREG)(sp)
> +	FREG_S	ft2, (12*SZREG + 2*SZFREG)(sp)
> +	FREG_S	ft3, (12*SZREG + 3*SZFREG)(sp)
> +	FREG_S	ft4, (12*SZREG + 4*SZFREG)(sp)
> +	FREG_S	ft5, (12*SZREG + 5*SZFREG)(sp)
> +	FREG_S	ft6, (12*SZREG + 6*SZFREG)(sp)
> +	FREG_S	ft7, (12*SZREG + 7*SZFREG)(sp)
> +	FREG_S	fa0, (12*SZREG + 8*SZFREG)(sp)
> +	FREG_S	fa1, (12*SZREG + 9*SZFREG)(sp)
> +	FREG_S	fa2, (12*SZREG + 10*SZFREG)(sp)
> +	FREG_S	fa3, (12*SZREG + 11*SZFREG)(sp)
> +	FREG_S	fa4, (12*SZREG + 12*SZFREG)(sp)
> +	FREG_S	fa5, (12*SZREG + 13*SZFREG)(sp)
> +	FREG_S	fa6, (12*SZREG + 14*SZFREG)(sp)
> +	FREG_S	fa7, (12*SZREG + 15*SZFREG)(sp)
> +	FREG_S	ft8, (12*SZREG + 16*SZFREG)(sp)
> +	FREG_S	ft9, (12*SZREG + 17*SZFREG)(sp)
> +	FREG_S	ft10, (12*SZREG + 18*SZFREG)(sp)
> +	FREG_S	ft11, (12*SZREG + 19*SZFREG)(sp)
> +#endif
> +
> +	call	__tls_get_addr
> +	addi	a0, a0, -TLS_DTV_OFFSET
> +
> +	REG_L	ra, 0*SZREG(sp)
> +	REG_L	a1, 1*SZREG(sp)
> +	REG_L	a2, 2*SZREG(sp)
> +	REG_L	a3, 3*SZREG(sp)
> +	REG_L	a4, 4*SZREG(sp)
> +	REG_L	a5, 5*SZREG(sp)
> +	REG_L	a6, 6*SZREG(sp)
> +	REG_L	a7, 7*SZREG(sp)
> +	REG_L	t3, 8*SZREG(sp)
> +	REG_L	t4, 9*SZREG(sp)
> +	REG_L	t5, 10*SZREG(sp)
> +	REG_L	t6, 11*SZREG(sp)
> +#ifndef __riscv_float_abi_soft
> +	FREG_L	ft0, (12*SZREG + 0*SZFREG)(sp)
> +	FREG_L	ft1, (12*SZREG + 1*SZFREG)(sp)
> +	FREG_L	ft2, (12*SZREG + 2*SZFREG)(sp)
> +	FREG_L	ft3, (12*SZREG + 3*SZFREG)(sp)
> +	FREG_L	ft4, (12*SZREG + 4*SZFREG)(sp)
> +	FREG_L	ft5, (12*SZREG + 5*SZFREG)(sp)
> +	FREG_L	ft6, (12*SZREG + 6*SZFREG)(sp)
> +	FREG_L	ft7, (12*SZREG + 7*SZFREG)(sp)
> +	FREG_L	fa0, (12*SZREG + 8*SZFREG)(sp)
> +	FREG_L	fa1, (12*SZREG + 9*SZFREG)(sp)
> +	FREG_L	fa2, (12*SZREG + 10*SZFREG)(sp)
> +	FREG_L	fa3, (12*SZREG + 11*SZFREG)(sp)
> +	FREG_L	fa4, (12*SZREG + 12*SZFREG)(sp)
> +	FREG_L	fa5, (12*SZREG + 13*SZFREG)(sp)
> +	FREG_L	fa6, (12*SZREG + 14*SZFREG)(sp)
> +	FREG_L	fa7, (12*SZREG + 15*SZFREG)(sp)
> +	FREG_L	ft8, (12*SZREG + 16*SZFREG)(sp)
> +	FREG_L	ft9, (12*SZREG + 17*SZFREG)(sp)
> +	FREG_L	ft10, (12*SZREG + 18*SZFREG)(sp)
> +	FREG_L	ft11, (12*SZREG + 19*SZFREG)(sp)
> +#endif
> +	addi	sp, sp, FRAME_SIZE
> +	j	.Lret
> +END (_dl_tlsdesc_dynamic)
> +#endif
> diff --git a/sysdeps/riscv/dl-tlsdesc.h b/sysdeps/riscv/dl-tlsdesc.h
> new file mode 100644
> index 0000000000..c7d1bb6d2e
> --- /dev/null
> +++ b/sysdeps/riscv/dl-tlsdesc.h
> @@ -0,0 +1,49 @@
> +/* Thread-local storage descriptor handling in the ELF dynamic linker.
> +   RISC-V version.
> +   Copyright (C) 2011-2023 Free Software Foundation, Inc.

I think it should be 2023 only here.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _DL_TLSDESC_H
> +# define _DL_TLSDESC_H 1
> +
> +#include <dl-tls.h>
> +
> +/* Type used to represent a TLS descriptor in the GOT.  */
> +struct tlsdesc
> +{
> +  ptrdiff_t (*entry) (struct tlsdesc *);
> +  void *arg;
> +};
> +
> +/* Type used as the argument in a TLS descriptor for a symbol that
> +   needs dynamic TLS offsets.  */
> +struct tlsdesc_dynamic_arg
> +{
> +  tls_index tlsinfo;
> +  size_t gen_count;
> +};
> +
> +extern unsigned long attribute_hidden
> +  _dl_tlsdesc_return(struct tlsdesc *),
> +  _dl_tlsdesc_undefweak(struct tlsdesc *);
> +
> +# ifdef SHARED
> +extern void *_dl_make_tlsdesc_dynamic (struct link_map *, size_t);
> +extern unsigned long attribute_hidden _dl_tlsdesc_dynamic(struct tlsdesc *);
> +# endif

Multiple style issues here: do not use comma for function prototypes,
add the attributes after function declaration, and add proper scape
after names:

  extern unsigned long _dl_tlsdesc_return (struct tlsdesc *)
    attribute_hidden;

  extern unsigned long _dl_tlsdesc_undefweak (struct tlsdesc *)
    attribute_hidden;

  # ifdef SHARED
  extern void * _dl_make_tlsdesc_dynamic (struct link_map *, size_t);
  extern unsigned long attribute_hidden _dl_tlsdesc_dynamic (struct tlsdesc *);
  # endif

The return type for _dl_tlsdesc_return, _dl_tlsdesc_undefweak, and
_dl_tlsdesc_dynamic seems wrong.

> +
> +#endif /* _DL_TLSDESC_H */
> diff --git a/sysdeps/riscv/linkmap.h b/sysdeps/riscv/linkmap.h
> index ac170bb342..2fa3f6d43f 100644
> --- a/sysdeps/riscv/linkmap.h
> +++ b/sysdeps/riscv/linkmap.h
> @@ -1,4 +1,5 @@
>  struct link_map_machine
>    {
>      ElfW(Addr) plt; /* Address of .plt.  */
> +    void *tlsdesc_table; /* Address of TLS descriptor hash table.  */
>    };
> diff --git a/sysdeps/riscv/tlsdesc.c b/sysdeps/riscv/tlsdesc.c
> new file mode 100644
> index 0000000000..a76aaa9fc5
> --- /dev/null
> +++ b/sysdeps/riscv/tlsdesc.c
> @@ -0,0 +1,38 @@
> +/* Manage TLS descriptors.  RISC-V version.
> +   Copyright (C) 2005-2023 Free Software Foundation, Inc.

I think it should be 2023 here.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <ldsodefs.h>
> +#include <tls.h>
> +#include <dl-tls.h>
> +#include <dl-tlsdesc.h>
> +#include <dl-unmap-segments.h>
> +#include <tlsdeschtab.h>
> +
> +/* Unmap the dynamic object, but also release its TLS descriptor table
> +   if there is one.  */
> +
> +void
> +_dl_unmap (struct link_map *map)
> +{
> +  _dl_unmap_segments (map);
> +
> +#ifdef SHARED
> +  if (map->l_mach.tlsdesc_table)
> +    htab_delete (map->l_mach.tlsdesc_table);
> +#endif
> +}
> diff --git a/sysdeps/riscv/tlsdesc.sym b/sysdeps/riscv/tlsdesc.sym
> new file mode 100644
> index 0000000000..652e72ea58
> --- /dev/null
> +++ b/sysdeps/riscv/tlsdesc.sym
> @@ -0,0 +1,19 @@
> +#include <stddef.h>
> +#include <sysdep.h>
> +#include <tls.h>
> +#include <link.h>
> +#include <dl-tls.h>
> +#include <dl-tlsdesc.h>
> +
> +--
> +
> +-- Abuse tls.h macros to derive offsets relative to the thread register.
> +
> +TLSDESC_ARG		offsetof(struct tlsdesc, arg)
> +TLSDESC_GEN_COUNT	offsetof(struct tlsdesc_dynamic_arg, gen_count)
> +TLSDESC_MODID		offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_module)
> +TLSDESC_MODOFF		offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_offset)
> +TCBHEAD_DTV		offsetof(tcbhead_t, dtv) - sizeof(tcbhead_t) - TLS_TCB_OFFSET
> +DTV_COUNTER		offsetof(dtv_t, counter)
> +TLS_DTV_UNALLOCATED	TLS_DTV_UNALLOCATED
> +TLS_DTV_OFFSET		TLS_DTV_OFFSET
  
Tatsuyuki Ishi Sept. 14, 2023, 8:39 a.m. UTC | #2
> On Sep 14, 2023, at 4:14, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote:
> 
> How did you actually build glibc? I saw multiple build issues with default
> configuration and even with --disable-werror, so I am doubtful that this
> patch was really proper tested. Please ensure that have-mtls-dialect-gnu2
> is set to 'yes' on config.make so the tests are actually run.

I’m sorry I’ve made multiple mistakes here. There were actually two prerequisite commits but I’ve forgot to include them in the patch series. This will be included in v4.

I used [1] to build a full toolchain and it defaulted to --disable-werror. I’ve manually enabled -Werror and fixed all compiler warnings in v4.

As for have-mtls-dialect-gnu2, RISC-V will use AArch64-style flags (-mtls-dialect={trad,desc}), not gnu2. However, I have configured my GCC fork with --with-tls=desc and all compilation is done with TLSDESC by default for my testing.

I assumed most testing was done through GCC’s testsuite, and I’ve got GCC’s testsuite to the point of no regression, however I was wrong and there are more in glibc’s testsuite. For v4 I’ve ran all tests in glibc/elf/, and all but two tests for TLS on static executables are passing. More info on my plan for fixing that in v4.

> 
>> # RISC-V's assembler also needs to know about PIC as it changes the definition
>> # of some assembler macros.
>> ASFLAGS-.os += $(pic-ccflag)
>> diff --git a/sysdeps/riscv/dl-lookupcfg.h b/sysdeps/riscv/dl-lookupcfg.h
>> new file mode 100644
>> index 0000000000..d7fe73636b
>> --- /dev/null
>> +++ b/sysdeps/riscv/dl-lookupcfg.h
>> @@ -0,0 +1,27 @@
>> +/* Configuration of lookup functions.
>> +   Copyright (C) 2006-2023 Free Software Foundation, Inc.
> 
> I think it should be only 2023 for new code.

Ack, all copyright headers for new files will be 2023 only in v4.


>>    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
>> 
>> /* Return nonzero iff ELF header is compatible with the running host.  */
>> @@ -219,6 +221,32 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>> 	}
>>       break;
>> 
>> +    case R_RISCV_TLSDESC:
>> +      struct tlsdesc *td = (struct tlsdesc *) addr_field;
>> +      if (sym == NULL)
>> +	{
>> +	  td->entry = _dl_tlsdesc_undefweak;
> 
> 
> This triggers multiple compiler warnings:
> 
> ../sysdeps/riscv/dl-machine.h: In function ‘elf_machine_rela’:
> ../sysdeps/riscv/dl-machine.h:228:21: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
>  228 |           td->entry = _dl_tlsdesc_undefweak;
>      |                     ^
> ../sysdeps/riscv/dl-machine.h:244:25: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
>  244 |               td->entry = _dl_tlsdesc_return;
>      |                         ^
> 
> Because you declare _dl_tlsdesc_undefweak as:
> 
>  unsigned long _dl_tlsdesc_dynamic (struct tlsdesc *);
> 
> But the 'entry' at tlsdesc as:
> 
>   ptrdiff_t (*entry) (struct tlsdesc *);
> 
> Based on TLSDESC ABI I think using a unsigned as return value is wrong here.

I am opting to not using ptrdiff_t because the offset can be larger than PTRDIFF_MAX. Using unsigned arithmetic avoids the signed overflow concern.

The descriptor signature is also defined in the RISC-V psABI as returning unsigned long for the same reason.

[1]: https://github.com/riscv-collab/riscv-gnu-toolchain
  
Adhemerval Zanella Netto Sept. 14, 2023, 12:09 p.m. UTC | #3
On 14/09/23 05:39, Tatsuyuki Ishi wrote:
> 
>> On Sep 14, 2023, at 4:14, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote:
>>
>> How did you actually build glibc? I saw multiple build issues with default
>> configuration and even with --disable-werror, so I am doubtful that this
>> patch was really proper tested. Please ensure that have-mtls-dialect-gnu2
>> is set to 'yes' on config.make so the tests are actually run.
> 
> I’m sorry I’ve made multiple mistakes here. There were actually two prerequisite commits but I’ve forgot to include them in the patch series. This will be included in v4.
> 
> I used [1] to build a full toolchain and it defaulted to --disable-werror. I’ve manually enabled -Werror and fixed all compiler warnings in v4.

For patch development I would advise strongly to use --disable-werror,
the patchwork hasn't flag it because we do not build/run on riscv.

> 
> As for have-mtls-dialect-gnu2, RISC-V will use AArch64-style flags (-mtls-dialect={trad,desc}), not gnu2. However, I have configured my GCC fork with --with-tls=desc and all compilation is done with TLSDESC by default for my testing.

So I take that the default would be still the -mtls-dialect=trad. In
this case I would suggest to change the have-mtls-dialect-gnu2 to be
enabled for -mtls-dialect=desc as well, the elf tests are generic and
should exercise both mode in a default configuration.

> 
> I assumed most testing was done through GCC’s testsuite, and I’ve got GCC’s testsuite to the point of no regression, however I was wrong and there are more in glibc’s testsuite. For v4 I’ve ran all tests in glibc/elf/, and all but two tests for TLS on static executables are passing. More info on my plan for fixing that in v4.
> 

I am not sure about gcc testsuite, but it would be good RISCV to check
for both mode and not only stress the compiler default. Unfortunately 
not all ports follow this.

>>
>>> # RISC-V's assembler also needs to know about PIC as it changes the definition
>>> # of some assembler macros.
>>> ASFLAGS-.os += $(pic-ccflag)
>>> diff --git a/sysdeps/riscv/dl-lookupcfg.h b/sysdeps/riscv/dl-lookupcfg.h
>>> new file mode 100644
>>> index 0000000000..d7fe73636b
>>> --- /dev/null
>>> +++ b/sysdeps/riscv/dl-lookupcfg.h
>>> @@ -0,0 +1,27 @@
>>> +/* Configuration of lookup functions.
>>> +   Copyright (C) 2006-2023 Free Software Foundation, Inc.
>>
>> I think it should be only 2023 for new code.
> 
> Ack, all copyright headers for new files will be 2023 only in v4.
> 
>>>    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
>>>
>>> /* Return nonzero iff ELF header is compatible with the running host.  */
>>> @@ -219,6 +221,32 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>>> }
>>>       break;
>>>
>>> +    case R_RISCV_TLSDESC:
>>> +      struct tlsdesc *td = (struct tlsdesc *) addr_field;
>>> +      if (sym == NULL)
>>> +{
>>> +  td->entry = _dl_tlsdesc_undefweak;
>>
>>
>> This triggers multiple compiler warnings:
>>
>> ../sysdeps/riscv/dl-machine.h: In function ‘elf_machine_rela’:
>> ../sysdeps/riscv/dl-machine.h:228:21: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
>>  228 |           td->entry = _dl_tlsdesc_undefweak;
>>      |                     ^
>> ../sysdeps/riscv/dl-machine.h:244:25: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
>>  244 |               td->entry = _dl_tlsdesc_return;
>>      |                         ^
>>
>> Because you declare _dl_tlsdesc_undefweak as:
>>
>>  unsigned long _dl_tlsdesc_dynamic (struct tlsdesc *);
>>
>> But the 'entry' at tlsdesc as:
>>
>>   ptrdiff_t (*entry) (struct tlsdesc *);
>>
>> Based on TLSDESC ABI I think using a unsigned as return value is wrong here.
> 
> I am opting to not using ptrdiff_t because the offset can be larger than PTRDIFF_MAX. Using unsigned arithmetic avoids the signed overflow concern.
> 
> The descriptor signature is also defined in the RISC-V psABI as returning unsigned long for the same reason.
> 
> [1]: https://github.com/riscv-collab/riscv-gnu-toolchain
> 

The C standard specified that subtracting two points should be signed
integer type, which maps to ptrdiff_t on most C and POSIX interfaces.
The malloc would fail with requests larger than ptrdiff_t (check 
9bf8e29ca136094f73f69f725f15c51facc97206 and BZ#23741) and gcc might
generate wrong optimizations in such cases (we still allow values 
larger than ptrdiff_t for mmap, but some other libc like bionic and
musl will fail even for mmap).

Although the required code that operates with tlsdesc uses a 
non-standard ABI, meaning most code would either use assembly or C with 
some extra care, I really don't see a compelling reason to have RISCV 
deviates from other ports that implemented TLSDESC.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=23741
[2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999
  
Fangrui Song Jan. 27, 2024, 2:22 a.m. UTC | #4
On Thu, Sep 14, 2023 at 5:09 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 14/09/23 05:39, Tatsuyuki Ishi wrote:
> >
> >> On Sep 14, 2023, at 4:14, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote:
> >>
> >> How did you actually build glibc? I saw multiple build issues with default
> >> configuration and even with --disable-werror, so I am doubtful that this
> >> patch was really proper tested. Please ensure that have-mtls-dialect-gnu2
> >> is set to 'yes' on config.make so the tests are actually run.
> >
> > I’m sorry I’ve made multiple mistakes here. There were actually two prerequisite commits but I’ve forgot to include them in the patch series. This will be included in v4.
> >
> > I used [1] to build a full toolchain and it defaulted to --disable-werror. I’ve manually enabled -Werror and fixed all compiler warnings in v4.
>
> For patch development I would advise strongly to use --disable-werror,
> the patchwork hasn't flag it because we do not build/run on riscv.
>
> >
> > As for have-mtls-dialect-gnu2, RISC-V will use AArch64-style flags (-mtls-dialect={trad,desc}), not gnu2. However, I have configured my GCC fork with --with-tls=desc and all compilation is done with TLSDESC by default for my testing.
>
> So I take that the default would be still the -mtls-dialect=trad. In
> this case I would suggest to change the have-mtls-dialect-gnu2 to be
> enabled for -mtls-dialect=desc as well, the elf tests are generic and
> should exercise both mode in a default configuration.

Agree. We need to test both -mtls-dialect=trad and -mtls-dialect=desc.
We will likely need a variable like have-mtls-dialect-desc...

> >
> > I assumed most testing was done through GCC’s testsuite, and I’ve got GCC’s testsuite to the point of no regression, however I was wrong and there are more in glibc’s testsuite. For v4 I’ve ran all tests in glibc/elf/, and all but two tests for TLS on static executables are passing. More info on my plan for fixing that in v4.
> >

Latest lld (https://github.com/llvm/llvm-project/pull/79239) supports
TLSDESC and it optimizes TLSDESC to LE/IE regardless of R_RISCV_RELAX,
so it is compatible with the static executable tests.

Reviewers can create /usr/local/bin/ld as a symlink to the latest lld
and do a GCC build:)

On the LLVM side, the RISC-V TLSDESC work (LLVM, Clang, lld) has been
completed today.
https://maskray.me/blog/2024-01-23-riscv-tlsdesc-works
Clang cannot build glibc, but there may be something to use Clang to
build just the elf/ tests :) ?

> I am not sure about gcc testsuite, but it would be good RISCV to check
> for both mode and not only stress the compiler default. Unfortunately
> not all ports follow this.
>
> >>
> >>> # RISC-V's assembler also needs to know about PIC as it changes the definition
> >>> # of some assembler macros.
> >>> ASFLAGS-.os += $(pic-ccflag)
> >>> diff --git a/sysdeps/riscv/dl-lookupcfg.h b/sysdeps/riscv/dl-lookupcfg.h
> >>> new file mode 100644
> >>> index 0000000000..d7fe73636b
> >>> --- /dev/null
> >>> +++ b/sysdeps/riscv/dl-lookupcfg.h
> >>> @@ -0,0 +1,27 @@
> >>> +/* Configuration of lookup functions.
> >>> +   Copyright (C) 2006-2023 Free Software Foundation, Inc.
> >>
> >> I think it should be only 2023 for new code.
> >
> > Ack, all copyright headers for new files will be 2023 only in v4.

Perhaps 2024 for v5 :)

> >>>    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
> >>>
> >>> /* Return nonzero iff ELF header is compatible with the running host.  */
> >>> @@ -219,6 +221,32 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
> >>> }
> >>>       break;
> >>>
> >>> +    case R_RISCV_TLSDESC:
> >>> +      struct tlsdesc *td = (struct tlsdesc *) addr_field;
> >>> +      if (sym == NULL)
> >>> +{
> >>> +  td->entry = _dl_tlsdesc_undefweak;
> >>
> >>
> >> This triggers multiple compiler warnings:
> >>
> >> ../sysdeps/riscv/dl-machine.h: In function ‘elf_machine_rela’:
> >> ../sysdeps/riscv/dl-machine.h:228:21: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
> >>  228 |           td->entry = _dl_tlsdesc_undefweak;
> >>      |                     ^
> >> ../sysdeps/riscv/dl-machine.h:244:25: error: assignment to ‘ptrdiff_t (*)(struct tlsdesc *)’ {aka ‘long int (*)(struct tlsdesc *)’} from incompatible pointer type ‘long unsigned int (*)(struct tlsdesc *)’ [-Werror=incompatible-pointer-types]
> >>  244 |               td->entry = _dl_tlsdesc_return;
> >>      |                         ^
> >>
> >> Because you declare _dl_tlsdesc_undefweak as:
> >>
> >>  unsigned long _dl_tlsdesc_dynamic (struct tlsdesc *);
> >>
> >> But the 'entry' at tlsdesc as:
> >>
> >>   ptrdiff_t (*entry) (struct tlsdesc *);
> >>
> >> Based on TLSDESC ABI I think using a unsigned as return value is wrong here.
> >
> > I am opting to not using ptrdiff_t because the offset can be larger than PTRDIFF_MAX. Using unsigned arithmetic avoids the signed overflow concern.
> >
> > The descriptor signature is also defined in the RISC-V psABI as returning unsigned long for the same reason.
> >
> > [1]: https://github.com/riscv-collab/riscv-gnu-toolchain
> >
>
> The C standard specified that subtracting two points should be signed
> integer type, which maps to ptrdiff_t on most C and POSIX interfaces.
> The malloc would fail with requests larger than ptrdiff_t (check
> 9bf8e29ca136094f73f69f725f15c51facc97206 and BZ#23741) and gcc might
> generate wrong optimizations in such cases (we still allow values
> larger than ptrdiff_t for mmap, but some other libc like bionic and
> musl will fail even for mmap).
>
> Although the required code that operates with tlsdesc uses a
> non-standard ABI, meaning most code would either use assembly or C with
> some extra care, I really don't see a compelling reason to have RISCV
> deviates from other ports that implemented TLSDESC.
>
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=23741
> [2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999
  

Patch

diff --git a/sysdeps/riscv/Makefile b/sysdeps/riscv/Makefile
index 8fb10b164f..bb4bcd29b2 100644
--- a/sysdeps/riscv/Makefile
+++ b/sysdeps/riscv/Makefile
@@ -2,6 +2,14 @@  ifeq ($(subdir),misc)
 sysdep_headers += sys/asm.h
 endif
 
+ifeq ($(subdir),elf)
+sysdep-dl-routines += tlsdesc dl-tlsdesc
+endif
+
+ifeq ($(subdir),csu)
+gen-as-const-headers += tlsdesc.sym
+endif
+
 # RISC-V's assembler also needs to know about PIC as it changes the definition
 # of some assembler macros.
 ASFLAGS-.os += $(pic-ccflag)
diff --git a/sysdeps/riscv/dl-lookupcfg.h b/sysdeps/riscv/dl-lookupcfg.h
new file mode 100644
index 0000000000..d7fe73636b
--- /dev/null
+++ b/sysdeps/riscv/dl-lookupcfg.h
@@ -0,0 +1,27 @@ 
+/* Configuration of lookup functions.
+   Copyright (C) 2006-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#define DL_UNMAP_IS_SPECIAL
+
+#include_next <dl-lookupcfg.h>
+
+struct link_map;
+
+extern void _dl_unmap (struct link_map *map);
+
+#define DL_UNMAP(map) _dl_unmap (map)
diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
index c0c9bd93ad..eb0c874e72 100644
--- a/sysdeps/riscv/dl-machine.h
+++ b/sysdeps/riscv/dl-machine.h
@@ -25,6 +25,7 @@ 
 #include <elf/elf.h>
 #include <sys/asm.h>
 #include <dl-tls.h>
+#include <dl-tlsdesc.h>
 #include <dl-irel.h>
 #include <dl-static-tls.h>
 #include <dl-machine-rel.h>
@@ -50,7 +51,8 @@ 
      || (__WORDSIZE == 32 && (type) == R_RISCV_TLS_TPREL32)	\
      || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPREL64)	\
      || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_DTPMOD64)	\
-     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64)))	\
+     || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))	\
+     || ((type) == R_RISCV_TLSDESC))				\
    | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY)))
 
 /* Return nonzero iff ELF header is compatible with the running host.  */
@@ -219,6 +221,32 @@  elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
 	}
       break;
 
+    case R_RISCV_TLSDESC:
+      struct tlsdesc *td = (struct tlsdesc *) addr_field;
+      if (sym == NULL)
+	{
+	  td->entry = _dl_tlsdesc_undefweak;
+	  td->arg = reloc->r_addend;
+	}
+      else
+	{
+# ifndef SHARED
+	  CHECK_STATIC_TLS (map, sym_map);
+# else
+	  if (!TRY_STATIC_TLS (map, sym_map))
+	    {
+	      td->entry = _dl_tlsdesc_dynamic;
+	      td->arg = _dl_make_tlsdesc_dynamic (sym_map, sym->st_value + reloc->r_addend);
+	    }
+	  else
+# endif
+	    {
+	      td->entry = _dl_tlsdesc_return;
+	      td->arg = (void *)(TLS_TPREL_VALUE(sym_map, sym) + reloc->r_addend);
+	    }
+	}
+      break;
+
     case R_RISCV_COPY:
       {
 	if (__glibc_unlikely (sym == NULL))
@@ -289,6 +317,24 @@  elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
       else
 	*reloc_addr = map->l_mach.plt;
     }
+  else if (__glibc_likely (r_type == R_RISCV_TLSDESC))
+    {
+      const Elf_Symndx symndx = ELFW (R_SYM) (reloc->r_info);
+      const ElfW (Sym) *symtab = (const void *)D_PTR (map, l_info[DT_SYMTAB]);
+      const ElfW (Sym) *sym = &symtab[symndx];
+      const struct r_found_version *version = NULL;
+
+      if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
+	{
+	  const ElfW (Half) *vernum =
+	      (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]);
+	  version = &map->l_versions[vernum[symndx] & 0x7fff];
+	}
+
+      /* Always initialize TLS descriptors completely, because lazy
+	 initialization requires synchronization at every TLS access.  */
+      elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, skip_ifunc);
+    }
   else if (__glibc_unlikely (r_type == R_RISCV_IRELATIVE))
     {
       ElfW(Addr) value = map->l_addr + reloc->r_addend;
diff --git a/sysdeps/riscv/dl-tlsdesc.S b/sysdeps/riscv/dl-tlsdesc.S
new file mode 100644
index 0000000000..bf241ef76a
--- /dev/null
+++ b/sysdeps/riscv/dl-tlsdesc.S
@@ -0,0 +1,203 @@ 
+/* Thread-local storage handling in the ELF dynamic linker.
+   RISC-V version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sysdep.h>
+#include <tls.h>
+#include <tlsdesc.h>
+
+#ifdef __riscv_float_abi_soft
+# define FRAME_SIZE (-((-12 * SZREG) & ALMASK))
+#else
+# define FRAME_SIZE (-((-12 * SZREG - 20 * SZFREG) & ALMASK))
+#endif
+
+	.text
+
+	/* Compute the thread pointer offset for symbols in the static
+	   TLS block. The offset is the same for all threads.
+	   Prototype:
+	   _dl_tlsdesc_return (tlsdesc *) ;
+	 */
+ENTRY (_dl_tlsdesc_return)
+	REG_L a0, TLSDESC_ARG(a0)
+	jr t0
+END (_dl_tlsdesc_return)
+
+	/* Handler for undefined weak TLS symbols.
+	   Prototype:
+	   _dl_tlsdesc_undefweak (tlsdesc *);
+
+	   The second word of the descriptor contains the addend.
+	   Return the addend minus the thread pointer. This ensures
+	   that when the caller adds on the thread pointer it gets back
+	   the addend.  */
+
+ENTRY (_dl_tlsdesc_undefweak)
+	REG_L a0, TLSDESC_ARG(a0)
+	sub a0, a0, tp
+	jr t0
+END (_dl_tlsdesc_undefweak)
+
+#ifdef SHARED
+	/* Handler for dynamic TLS symbols.
+	   Prototype:
+	   _dl_tlsdesc_dynamic (tlsdesc *) ;
+
+	   The second word of the descriptor points to a
+	   tlsdesc_dynamic_arg structure.
+
+	   Returns the offset between the thread pointer and the
+	   object referenced by the argument.
+
+	   unsigned long
+	   _dl_tlsdesc_dynamic (struct tlsdesc *tdp)
+	   {
+	     struct tlsdesc_dynamic_arg *td = tdp->arg;
+	     dtv_t *dtv = *(dtv_t **)((char *)__thread_pointer + TCBHEAD_DTV);
+	     if (__builtin_expect (td->gen_count <= dtv[0].counter
+		&& (dtv[td->tlsinfo.ti_module].pointer.val
+		    != TLS_DTV_UNALLOCATED),
+		1))
+	       return dtv[td->tlsinfo.ti_module].pointer.val
+		+ td->tlsinfo.ti_offset
+		- __thread_pointer;
+
+	     return ___tls_get_addr (&td->tlsinfo) - __thread_pointer;
+	   }
+	 */
+
+ENTRY (_dl_tlsdesc_dynamic)
+	/* Save just enough registers to support fast path, if we fall
+	   into slow path we will save additional registers.  */
+	add	sp, sp, -3*SZREG
+	REG_S	t0, 0*SZREG(sp)
+	REG_S	t1, 1*SZREG(sp)
+	REG_S	t2, 2*SZREG(sp)
+
+	/* t0 = dtv */
+	REG_L	t0, TCBHEAD_DTV(tp)
+	/* a0 = tdp->arg */
+	REG_L	a0, TLSDESC_ARG(a0)
+	/* t1 = td->gen_count */
+	REG_L	t1, TLSDESC_GEN_COUNT(a0)
+	/* t2 = dtv[0].counter */
+	REG_L	t2, DTV_COUNTER(t0)
+	bltu	t2, t1, .Lslow
+	/* t1 = td->tlsinfo.ti_module */
+	REG_L	t1, TLSDESC_MODID(a0)
+	slli	t1, t1, PTRLOG + 1 /* sizeof(dtv_t) == sizeof(void*) * 2 */
+	add	t1, t1, t0
+	/* t1 = dtv[td->tlsinfo.ti_module].pointer.val  */
+	REG_L	t1, 0(t1)
+	li	t2, TLS_DTV_UNALLOCATED
+	beq	t1, t2, .Lslow
+	/* t2 = td->tlsinfo.ti_offset */
+	REG_L	t2, TLSDESC_MODOFF(a0)
+	add	a0, t1, t2
+.Lret:
+	sub	a0, a0, tp
+	REG_L	t0, 0*SZREG(sp)
+	REG_L	t1, 1*SZREG(sp)
+	REG_L	t2, 2*SZREG(sp)
+	add	sp, sp, 3*SZREG
+	jr t0
+.Lslow:
+	/* This is the slow path. We need to call __tls_get_addr() which
+	   means we need to save and restore all the register that the
+	   callee will trash.  */
+
+	/* Save the remaining registers that we must treat as caller save.  */
+	addi	sp, sp, -FRAME_SIZE
+	REG_S	ra, 0*SZREG(sp)
+	REG_S	a1, 1*SZREG(sp)
+	REG_S	a2, 2*SZREG(sp)
+	REG_S	a3, 3*SZREG(sp)
+	REG_S	a4, 4*SZREG(sp)
+	REG_S	a5, 5*SZREG(sp)
+	REG_S	a6, 6*SZREG(sp)
+	REG_S	a7, 7*SZREG(sp)
+	REG_S	t3, 8*SZREG(sp)
+	REG_S	t4, 9*SZREG(sp)
+	REG_S	t5, 10*SZREG(sp)
+	REG_S	t6, 11*SZREG(sp)
+#ifndef __riscv_float_abi_soft
+	FREG_S	ft0, (12*SZREG + 0*SZFREG)(sp)
+	FREG_S	ft1, (12*SZREG + 1*SZFREG)(sp)
+	FREG_S	ft2, (12*SZREG + 2*SZFREG)(sp)
+	FREG_S	ft3, (12*SZREG + 3*SZFREG)(sp)
+	FREG_S	ft4, (12*SZREG + 4*SZFREG)(sp)
+	FREG_S	ft5, (12*SZREG + 5*SZFREG)(sp)
+	FREG_S	ft6, (12*SZREG + 6*SZFREG)(sp)
+	FREG_S	ft7, (12*SZREG + 7*SZFREG)(sp)
+	FREG_S	fa0, (12*SZREG + 8*SZFREG)(sp)
+	FREG_S	fa1, (12*SZREG + 9*SZFREG)(sp)
+	FREG_S	fa2, (12*SZREG + 10*SZFREG)(sp)
+	FREG_S	fa3, (12*SZREG + 11*SZFREG)(sp)
+	FREG_S	fa4, (12*SZREG + 12*SZFREG)(sp)
+	FREG_S	fa5, (12*SZREG + 13*SZFREG)(sp)
+	FREG_S	fa6, (12*SZREG + 14*SZFREG)(sp)
+	FREG_S	fa7, (12*SZREG + 15*SZFREG)(sp)
+	FREG_S	ft8, (12*SZREG + 16*SZFREG)(sp)
+	FREG_S	ft9, (12*SZREG + 17*SZFREG)(sp)
+	FREG_S	ft10, (12*SZREG + 18*SZFREG)(sp)
+	FREG_S	ft11, (12*SZREG + 19*SZFREG)(sp)
+#endif
+
+	call	__tls_get_addr
+	addi	a0, a0, -TLS_DTV_OFFSET
+
+	REG_L	ra, 0*SZREG(sp)
+	REG_L	a1, 1*SZREG(sp)
+	REG_L	a2, 2*SZREG(sp)
+	REG_L	a3, 3*SZREG(sp)
+	REG_L	a4, 4*SZREG(sp)
+	REG_L	a5, 5*SZREG(sp)
+	REG_L	a6, 6*SZREG(sp)
+	REG_L	a7, 7*SZREG(sp)
+	REG_L	t3, 8*SZREG(sp)
+	REG_L	t4, 9*SZREG(sp)
+	REG_L	t5, 10*SZREG(sp)
+	REG_L	t6, 11*SZREG(sp)
+#ifndef __riscv_float_abi_soft
+	FREG_L	ft0, (12*SZREG + 0*SZFREG)(sp)
+	FREG_L	ft1, (12*SZREG + 1*SZFREG)(sp)
+	FREG_L	ft2, (12*SZREG + 2*SZFREG)(sp)
+	FREG_L	ft3, (12*SZREG + 3*SZFREG)(sp)
+	FREG_L	ft4, (12*SZREG + 4*SZFREG)(sp)
+	FREG_L	ft5, (12*SZREG + 5*SZFREG)(sp)
+	FREG_L	ft6, (12*SZREG + 6*SZFREG)(sp)
+	FREG_L	ft7, (12*SZREG + 7*SZFREG)(sp)
+	FREG_L	fa0, (12*SZREG + 8*SZFREG)(sp)
+	FREG_L	fa1, (12*SZREG + 9*SZFREG)(sp)
+	FREG_L	fa2, (12*SZREG + 10*SZFREG)(sp)
+	FREG_L	fa3, (12*SZREG + 11*SZFREG)(sp)
+	FREG_L	fa4, (12*SZREG + 12*SZFREG)(sp)
+	FREG_L	fa5, (12*SZREG + 13*SZFREG)(sp)
+	FREG_L	fa6, (12*SZREG + 14*SZFREG)(sp)
+	FREG_L	fa7, (12*SZREG + 15*SZFREG)(sp)
+	FREG_L	ft8, (12*SZREG + 16*SZFREG)(sp)
+	FREG_L	ft9, (12*SZREG + 17*SZFREG)(sp)
+	FREG_L	ft10, (12*SZREG + 18*SZFREG)(sp)
+	FREG_L	ft11, (12*SZREG + 19*SZFREG)(sp)
+#endif
+	addi	sp, sp, FRAME_SIZE
+	j	.Lret
+END (_dl_tlsdesc_dynamic)
+#endif
diff --git a/sysdeps/riscv/dl-tlsdesc.h b/sysdeps/riscv/dl-tlsdesc.h
new file mode 100644
index 0000000000..c7d1bb6d2e
--- /dev/null
+++ b/sysdeps/riscv/dl-tlsdesc.h
@@ -0,0 +1,49 @@ 
+/* Thread-local storage descriptor handling in the ELF dynamic linker.
+   RISC-V version.
+   Copyright (C) 2011-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _DL_TLSDESC_H
+# define _DL_TLSDESC_H 1
+
+#include <dl-tls.h>
+
+/* Type used to represent a TLS descriptor in the GOT.  */
+struct tlsdesc
+{
+  ptrdiff_t (*entry) (struct tlsdesc *);
+  void *arg;
+};
+
+/* Type used as the argument in a TLS descriptor for a symbol that
+   needs dynamic TLS offsets.  */
+struct tlsdesc_dynamic_arg
+{
+  tls_index tlsinfo;
+  size_t gen_count;
+};
+
+extern unsigned long attribute_hidden
+  _dl_tlsdesc_return(struct tlsdesc *),
+  _dl_tlsdesc_undefweak(struct tlsdesc *);
+
+# ifdef SHARED
+extern void *_dl_make_tlsdesc_dynamic (struct link_map *, size_t);
+extern unsigned long attribute_hidden _dl_tlsdesc_dynamic(struct tlsdesc *);
+# endif
+
+#endif /* _DL_TLSDESC_H */
diff --git a/sysdeps/riscv/linkmap.h b/sysdeps/riscv/linkmap.h
index ac170bb342..2fa3f6d43f 100644
--- a/sysdeps/riscv/linkmap.h
+++ b/sysdeps/riscv/linkmap.h
@@ -1,4 +1,5 @@ 
 struct link_map_machine
   {
     ElfW(Addr) plt; /* Address of .plt.  */
+    void *tlsdesc_table; /* Address of TLS descriptor hash table.  */
   };
diff --git a/sysdeps/riscv/tlsdesc.c b/sysdeps/riscv/tlsdesc.c
new file mode 100644
index 0000000000..a76aaa9fc5
--- /dev/null
+++ b/sysdeps/riscv/tlsdesc.c
@@ -0,0 +1,38 @@ 
+/* Manage TLS descriptors.  RISC-V version.
+   Copyright (C) 2005-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <ldsodefs.h>
+#include <tls.h>
+#include <dl-tls.h>
+#include <dl-tlsdesc.h>
+#include <dl-unmap-segments.h>
+#include <tlsdeschtab.h>
+
+/* Unmap the dynamic object, but also release its TLS descriptor table
+   if there is one.  */
+
+void
+_dl_unmap (struct link_map *map)
+{
+  _dl_unmap_segments (map);
+
+#ifdef SHARED
+  if (map->l_mach.tlsdesc_table)
+    htab_delete (map->l_mach.tlsdesc_table);
+#endif
+}
diff --git a/sysdeps/riscv/tlsdesc.sym b/sysdeps/riscv/tlsdesc.sym
new file mode 100644
index 0000000000..652e72ea58
--- /dev/null
+++ b/sysdeps/riscv/tlsdesc.sym
@@ -0,0 +1,19 @@ 
+#include <stddef.h>
+#include <sysdep.h>
+#include <tls.h>
+#include <link.h>
+#include <dl-tls.h>
+#include <dl-tlsdesc.h>
+
+--
+
+-- Abuse tls.h macros to derive offsets relative to the thread register.
+
+TLSDESC_ARG		offsetof(struct tlsdesc, arg)
+TLSDESC_GEN_COUNT	offsetof(struct tlsdesc_dynamic_arg, gen_count)
+TLSDESC_MODID		offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_module)
+TLSDESC_MODOFF		offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_offset)
+TCBHEAD_DTV		offsetof(tcbhead_t, dtv) - sizeof(tcbhead_t) - TLS_TCB_OFFSET
+DTV_COUNTER		offsetof(dtv_t, counter)
+TLS_DTV_UNALLOCATED	TLS_DTV_UNALLOCATED
+TLS_DTV_OFFSET		TLS_DTV_OFFSET