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

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Fri, Feb 21, 2014 at 12:34:50AM -0500, 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?

Looks OK to me.

Siddhesh

> 
> [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)
> ---
> 
> Cheers,
> Carlos.

Attachment: pgp0TtIbye528.pgp
Description: PGP signature