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

["Carlos O'Donell" <carlos at redhat dot com>]

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)
---

Cheers,
Carlos.