This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #16613] Allow audit libraries to use TLS.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 25 Feb 2014 13:19:39 -0500
- Subject: Re: [PATCH][BZ #16613] Allow audit libraries to use TLS.
- Authentication-results: sourceware.org; auth=none
- References: <53061E14 dot 6030903 at redhat dot com> <20140221045532 dot GH16378 at spoyarek dot pnq dot redhat dot com> <5306E57A dot 5030808 at redhat dot com>
On 02/21/2014 12:34 AM, Carlos O'Donell wrote:
> On 02/20/2014 11:55 PM, Siddhesh Poyarekar wrote:
>>> + if (! __builtin_expect (GL(dl_tls_dtv_gaps), true))
>>
>> This should use __glibc_{,un}likely. Also, _dl_tls_next_modid has the
>> opposite expectation for the value of dl_tls_dtv_gaps:
>>
>> if (__builtin_expect (GL(dl_tls_dtv_gaps), false))
>>
>> Since we're explicitly specifying the likelihood of the branch, you
>> might want to explain why each of those annotations are true, or fix
>> the annotation which isn't.
>
> Good catch.
>
> The _dl_count_modids annotation is wrong. Having gaps is rare and only
> happens during failure conditions.
>
> v2
> - Use __glibc_likely.
>
> Comments?
>
> [Difference from v1]
>
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index cbafdd9..ad124b8 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -109,9 +109,14 @@ size_t
> internal_function
> _dl_count_modids (void)
> {
> - if (! __builtin_expect (GL(dl_tls_dtv_gaps), true))
> + /* It is rare that we have gaps; see elf/dl-open.c (_dl_open) where
> + we fail to load a module and unload it leaving a gap. If we don't
> + have gaps then the number of modids is the current maximum so
> + return that. */
> + if (__glibc_likely (!GL(dl_tls_dtv_gaps)))
> return GL(dl_tls_max_dtv_idx);
>
> + /* We have gaps and are forced to count the non-NULL entries. */
> size_t n = 0;
> struct dtv_slotinfo_list *runp = GL(dl_tls_dtv_slotinfo_list);
> while (runp != NULL)
> ---
I've checked in v2 to fix this issue.
commit d050367659e04685a0eab910e86ea6829a8d24f9
Author: Carlos O'Donell <carlos@redhat.com>
Date: Tue Feb 25 13:00:36 2014 -0500
BZ #16613: Support TLS in audit libraries.
This commit fixes a bug where the dynamic loader would crash
when loading audit libraries, via LD_AUDIT, where those libraries
used TLS. The dynamic loader was not considering that the audit
libraries would use TLS and failed to bump the TLS generation
counter leaving TLS usage inconsistent after loading the audit
libraries.
https://sourceware.org/ml/libc-alpha/2014-02/msg00569.html
Cheers,
Carlos.