Re: [PATCH] elf: Remove dead l_need_tls_init static-TLS init path

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

Ping.

On 09/06/26 10:25, Adhemerval Zanella wrote:
> Since af34b1376a3 ("elf: Initialize static TLS before relocation
> processing", BZ 34164) dropped the 'defer-if-not-relocated' branch in
> _dl_try_allocate_static_tls, nothing sets l_need_tls_init any more.  The
> second pass in update_tls_slotinfo, guarded by l_need_tls_init, is
> therefore dead: its _dl_update_slotinfo / _dl_init_static_tls calls never
> run, and the static TLS image is initialised inline during relocation (IE
> model) or lazily on first dynamic-TLS access instead.
> 
> Remove the dead loop, the now write-only l_need_tls_init field and its
> clear in _dl_allocate_tls_init.  No functional change.
> 
> Checked on aarch64-linux-gnu, x86_64-linux-gnu, and i686-linux-gnu.
> I also run the elf tests on armv7-a, alpha, loongarch64, mips64le,
> powerpc, riscv, and s390x using qemu system.
> ---
>  elf/dl-open.c  | 50 +++++++++++---------------------------------------
>  elf/dl-tls.c   |  9 +++------
>  include/link.h |  3 ---
>  3 files changed, 14 insertions(+), 48 deletions(-)
> 
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index cf4749694f9..ba06e837bae 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -382,36 +382,6 @@ update_tls_slotinfo (struct link_map *new)
>  TLS generation counter wrapped!  Please report this."));
>    /* Can be read concurrently.  */
>    atomic_store_release (&GL(dl_tls_generation), newgen);
> -
> -  /* We need a second pass for static tls data, because
> -     _dl_update_slotinfo must not be run while calls to
> -     _dl_add_to_slotinfo are still pending.  */
> -  for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
> -    {
> -      struct link_map *imap = new->l_searchlist.r_list[i];
> -
> -      if (imap->l_need_tls_init && imap->l_tls_blocksize > 0)
> -	{
> -	  /* For static TLS we have to allocate the memory here and
> -	     now, but we can delay updating the DTV.  */
> -	  imap->l_need_tls_init = 0;
> -#ifdef SHARED
> -	  /* Update the slot information data for the current
> -	     generation.  */
> -
> -	  /* FIXME: This can terminate the process on memory
> -	     allocation failure.  It is not possible to raise
> -	     exceptions from this context; to fix this bug,
> -	     _dl_update_slotinfo would have to be split into two
> -	     operations, similar to resize_scopes and update_scopes
> -	     above.  This is related to bug 16134.  */
> -	  _dl_update_slotinfo (imap->l_tls_modid, newgen);
> -#endif
> -
> -	  _dl_init_static_tls (imap);
> -	  assert (imap->l_need_tls_init == 0);
> -	}
> -    }
>  }
>  
>  /* Mark the objects as NODELETE if required.  This is delayed until
> @@ -671,16 +641,18 @@ dl_open_worker_begin (void *a)
>    if (mode & RTLD_GLOBAL)
>      add_to_global_resize (new);
>  
> -  /* Install the new modules in the DTV slotinfo and initialise their
> -     static TLS *before* relocation, so an IFUNC resolver firing during
> -     the relocation loop below can reach its DSO's __thread storage via
> -     __tls_get_addr / TLSDESC.  Without this, the resolver's TLS access
> -     for a just-loaded module would index into an unallocated DTV slot
> -     and crash.  If relocation later fails, the subsequent _dl_close_worker
> -     cleans up these slotinfo entries via remove_slotinfo.  */
> +  /* Register the new modules in the DTV slotinfo and bump the TLS
> +     generation counter *before* relocation, so an IFUNC resolver firing
> +     during the relocation loop below can reach its DSO's __thread storage
> +     via __tls_get_addr / TLSDESC.  Without this, the new module is not yet
> +     in GL(dl_tls_dtv_slotinfo_list), so the resolver's dynamic-TLS lookup
> +     fails to find it and faults.  The static-TLS image itself is copied
> +     lazily on first access; for the initial-exec model the static-TLS
> +     offset is reserved inline during relocation (see
> +     _dl_try_allocate_static_tls), not here.  If relocation later fails,
> +     the subsequent _dl_close_worker cleans up these slotinfo entries via
> +     remove_slotinfo.  */
>    if (any_tls)
> -    /* FIXME: This calls _dl_update_slotinfo, which aborts the process
> -       on memory allocation failure.  See bug 16134.  */
>      update_tls_slotinfo (new);
>  
>    /* Perform relocation.  This can trigger lazy binding in IFUNC
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 1380bd70831..f2a99e8edb5 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -697,17 +697,14 @@ _dl_allocate_tls_init (void *result, bool main_thread)
>  	     For audit modules or dependencies with initial-exec TLS,
>  	     we can not set the initial TLS image on default loader
>  	     initialization because it would already be set by the
> -	     audit setup, which uses the dlopen code and already
> -	     clears l_need_tls_init.  Calls with !main_thread from
> -	     pthread_create need to initialize TLS for the current
> -	     thread regardless of namespace.  */
> +	     audit setup, which uses the dlopen code.  Calls with
> +	     !main_thread from pthread_create need to initialize TLS
> +	     for the current thread regardless of namespace.  */
>  	  if (map->l_ns != LM_ID_BASE && main_thread)
>  	    continue;
>  	  memset (__mempcpy (dest, map->l_tls_initimage,
>  			     map->l_tls_initimage_size), '\0',
>  		  map->l_tls_blocksize - map->l_tls_initimage_size);
> -	  if (main_thread)
> -	    map->l_need_tls_init = 0;
>  	}
>  
>        total += cnt;
> diff --git a/include/link.h b/include/link.h
> index 8f851d2212d..e299ca35fd4 100644
> --- a/include/link.h
> +++ b/include/link.h
> @@ -194,9 +194,6 @@ struct link_map
>  				      the l_libname list.  */
>      unsigned int l_faked:1;	/* Nonzero if this is a faked descriptor
>  				   without associated file.  */
> -    unsigned int l_need_tls_init:1; /* Nonzero if GL(dl_init_static_tls)
> -				       should be called on this link map
> -				       when relocation finishes.  */
>      unsigned int l_auditing:1;	/* Nonzero if the DSO is used in auditing.  */
>      unsigned int l_audit_any_plt:1; /* Nonzero if at least one audit module
>  				       is interested in the PLT interception.*/