public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: Tatsuyuki Ishi <ishitatsuyuki@gmail.com>
Cc: libc-alpha@sourceware.org, rui314@gmail.com,
	ruiu@bluewhale.systems, schwab@linux-m68k.org
Subject: Re: [PATCH v3] RISC-V: Implement TLS Descriptors.
Date: Wed, 13 Sep 2023 16:14:50 -0300	[thread overview]
Message-ID: <93e0270b-bdc8-3443-1d2a-947a31945d99@linaro.org> (raw)
In-Reply-To: <20230913172633.39229-1-ishitatsuyuki@gmail.com>



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

  reply	other threads:[~2023-09-13 19:14 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-17 18:12 [PATCH] " Tatsuyuki Ishi
2023-08-17 18:35 ` Andreas Schwab
2023-09-08 10:55 ` [PATCH v2] " Tatsuyuki Ishi
2023-09-13 17:26 ` [PATCH v3] " Tatsuyuki Ishi
2023-09-13 19:14   ` Adhemerval Zanella Netto [this message]
2023-09-14  8:39     ` Tatsuyuki Ishi
2023-09-14 12:09       ` Adhemerval Zanella Netto
2024-01-27  2:22         ` Fangrui Song
2023-09-13 19:07 ` [PATCH] " Andrew Waterman
2023-09-14  8:40 ` [PATCH v4 0/3] " Tatsuyuki Ishi
2023-09-14  8:40   ` [PATCH v4 1/3] RISC-V: Add include guard for dl-tls.h Tatsuyuki Ishi
2024-01-27  1:14     ` Fangrui Song
2023-09-14  8:40   ` [PATCH v4 2/3] RISC-V: Add TLSDESC reloc definitions Tatsuyuki Ishi
2024-01-27  1:12     ` Fangrui Song
2023-09-14  8:40   ` [PATCH v4 3/3] RISC-V: Implement TLS Descriptors Tatsuyuki Ishi
2023-11-23 11:39     ` Florian Weimer
2024-03-29  5:55 ` [PATCH v5 0/3] " Tatsuyuki Ishi
2024-03-29  5:55   ` [PATCH v5 1/3] RISC-V: Add include guard for dl-tls.h Tatsuyuki Ishi
2024-03-29  5:55   ` [PATCH v5 2/3] RISC-V: Add TLSDESC reloc definitions Tatsuyuki Ishi
2024-03-29  5:55   ` [PATCH v5 3/3] RISC-V: Implement TLS Descriptors Tatsuyuki Ishi
2024-03-29  6:18 ` [PATCH v6 0/3] " Tatsuyuki Ishi
2024-03-29  6:18   ` [PATCH v6 1/3] RISC-V: Add include guard for dl-tls.h Tatsuyuki Ishi
2024-04-03 11:48     ` Adhemerval Zanella Netto
2024-03-29  6:18   ` [PATCH v6 2/3] RISC-V: Add TLSDESC reloc definitions Tatsuyuki Ishi
2024-04-03  5:10     ` Fangrui Song
2024-04-03  8:03     ` Andreas Schwab
2024-03-29  6:18   ` [PATCH v6 3/3] RISC-V: Implement TLS Descriptors Tatsuyuki Ishi
2024-04-01 13:23     ` Florian Weimer
2024-04-01 19:29     ` Adhemerval Zanella Netto
2024-04-02  3:36       ` Tatsuyuki Ishi
2024-04-02 13:35         ` Adhemerval Zanella Netto
2024-04-02 15:25           ` Palmer Dabbelt
2024-04-02 15:32             ` Adhemerval Zanella Netto
2024-04-02 16:37               ` Palmer Dabbelt
2024-04-30 17:05   ` [PATCH v6 0/3] " Palmer Dabbelt
2024-04-30 18:33     ` Adhemerval Zanella Netto
2024-05-01  1:36       ` Tatsuyuki Ishi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=93e0270b-bdc8-3443-1d2a-947a31945d99@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=ishitatsuyuki@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=rui314@gmail.com \
    --cc=ruiu@bluewhale.systems \
    --cc=schwab@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).