Re: [PATCH][BZ #16613] Allow audit libraries to use TLS.

["Carlos O'Donell" <carlos 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.