elf: Remove dead l_need_tls_init static-TLS init path

Message ID 20260609132547.1577550-1-adhemerval.zanella@linaro.org (mailing list archive)
State Under Review
Delegated to: Florian Weimer
Headers
Series elf: Remove dead l_need_tls_init static-TLS init path |

Checks

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

Commit Message

Adhemerval Zanella Netto June 9, 2026, 1:25 p.m. UTC
  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(-)
  

Comments

Florian Weimer June 9, 2026, 2:13 p.m. UTC | #1
* 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
  
Adhemerval Zanella Netto June 9, 2026, 4:04 p.m. UTC | #2
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);
  
Florian Weimer June 10, 2026, 7:33 a.m. UTC | #3
* 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
  
Adhemerval Zanella Netto June 10, 2026, 11:46 a.m. UTC | #4
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.
  
Adhemerval Zanella Netto June 23, 2026, 12:23 p.m. UTC | #5
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.*/
  

Patch

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