elf: Remove dead l_need_tls_init static-TLS init path

  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(-)
  

* Adhemerval Zanella:

> @@ -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);

Is the FIXME truly gone?

Thanks,
Florian
  

On 09/06/26 11:13, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> @@ -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);
> 
> Is the FIXME truly gone?
No, BZ 16134 is not fixed, only relocated. That lazy path still aborts:
  
  __tls_get_addr -> _dl_update_slotinfo -> _dl_resize_dtv -> oom

Per:

elf/dt-tls.c

 921                   /* Resizing the dtv aborts on failure: bug 16134.  */
 922                   dtv = _dl_resize_dtv (dtv, max_modid, THREAD_SELF);
  

* Adhemerval Zanella Netto:

> On 09/06/26 11:13, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> @@ -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);
>> 
>> Is the FIXME truly gone?
> No, BZ 16134 is not fixed, only relocated. That lazy path still aborts:
>   
>   __tls_get_addr -> _dl_update_slotinfo -> _dl_resize_dtv -> oom
>
> Per:
>
> elf/dt-tls.c
>
>  921                   /* Resizing the dtv aborts on failure: bug 16134.  */
>  922                   dtv = _dl_resize_dtv (dtv, max_modid, THREAD_SELF);

Sorry, then why remove the comment?

Thanks,
Florian
  

On 10/06/26 04:33, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 09/06/26 11:13, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> @@ -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);
>>>
>>> Is the FIXME truly gone?
>> No, BZ 16134 is not fixed, only relocated. That lazy path still aborts:
>>   
>>   __tls_get_addr -> _dl_update_slotinfo -> _dl_resize_dtv -> oom
>>
>> Per:
>>
>> elf/dt-tls.c
>>
>>  921                   /* Resizing the dtv aborts on failure: bug 16134.  */
>>  922                   dtv = _dl_resize_dtv (dtv, max_modid, THREAD_SELF);
> 
> Sorry, then why remove the comment?
> 
> Thanks,
> Florian
>

Because with this cleanup update_tls_slotinfo does not call _dl_update_slotinfo,
so the bug does happen at this call anymore.
  

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.*/
  

* Adhemerval Zanella:

> 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);

I think update_tls_slotinfo can now be inlined into its caller?  At the
least the function is now about updating the TLS generation counter, so
the name doesn't quite match anymore.

Thanks,
Florian

PS: I won't make it to the patch review meeting today.
  

On 03/08/26 10:01, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> 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);
> 
> I think update_tls_slotinfo can now be inlined into its caller?  At the
> least the function is now about updating the TLS generation counter, so
> the name doesn't quite match anymore.

It makes sense, I will send a v2.

> 
> Thanks,
> Florian
> 
> PS: I won't make it to the patch review meeting today.
>