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