Re: About tls comment.

[OndÅej BÃlka <neleai at seznam dot cz>]

On Sat, Nov 02, 2013 at 07:46:29AM +0100, Andreas Schwab wrote:
> Rich Felker <dalias@aerifal.cx> writes:
> 
> > On Fri, Nov 01, 2013 at 07:14:01PM +0100, Andreas Schwab wrote:
> >> OndÅej BÃlka <neleai@seznam.cz> writes:
> >> 
> >> > It dates back to 2002. This comment puzzles me, as we do tls
> >> > initialization just below or did I missed something? 
> >> 
> >> See <http://repo.or.cz/w/glibc/history.git/commitdiff/19485a5>.
> >
> > It's not immediately apparent to me how this answers the question. If
> > you know more and it's not too big a time-waster, could you comment a
> > bit more on it?
> 
> You have to ask Uli on it.
> 
>From diff this looks like obsolete comment after code changed.

More specifically what is wrong with patch below?

It should fix a bug
https://sourceware.org/bugzilla/show_bug.cgi?id=15199
as LD_PRELOAD is handled before tls is initialized.

However it triggers following bug which causes tests that use tls fail.
https://sourceware.org/bugzilla/show_bug.cgi?id=13823

A second question is that I found macro NONTLS_INIT_TP which is never
defined or used. OK to remove that?

diff --git a/elf/rtld.c b/elf/rtld.c
index 51682f2..fdd9279 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1390,6 +1390,9 @@ of this helper program; chances are you did not intend to run this program.\n\
     /* Assign a module ID.  Do this before loading any audit modules.  */
     GL(dl_rtld_map).l_tls_modid = _dl_next_tls_modid ();
 
+  bool was_tls_init_tp_called = tls_init_tp_called;
+  tcbp = init_tls ();
+
   /* If we have auditing DSOs to load, do it now.  */
   if (__builtin_expect (audit_list != NULL, 0))
     {
@@ -1397,9 +1400,6 @@ of this helper program; chances are you did not intend to run this program.\n\
       struct audit_ifaces *last_audit = NULL;
       struct audit_list *al = audit_list->next;
 
-      /* Since we start using the auditing DSOs right away we need to
-	 initialize the data structures now.  */
-      tcbp = init_tls ();
 
       /* Initialize security features.  We need to do it this early
 	 since otherwise the constructors of the audit libraries will
@@ -1801,16 +1801,6 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
     _dl_receive_error (print_missing_version, version_check_doit, &args);
   }
 
-  /* We do not initialize any of the TLS functionality unless any of the
-     initial modules uses TLS.  This makes dynamic loading of modules with
-     TLS impossible, but to support it requires either eagerly doing setup
-     now or lazily doing it later.  Doing it now makes us incompatible with
-     an old kernel that can't perform TLS_INIT_TP, even if no TLS is ever
-     used.  Trying to do it lazily is too hairy to try when there could be
-     multiple threads (from a non-TLS-using libpthread).  */
-  bool was_tls_init_tp_called = tls_init_tp_called;
-  if (tcbp == NULL)
-    tcbp = init_tls ();
 
   if (__builtin_expect (audit_list == NULL, 1))
     /* Initialize security features.  But only if we have not done it
@@ -2217,9 +2207,6 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
 	_dl_start_profile ();
     }
 
-#ifndef NONTLS_INIT_TP
-# define NONTLS_INIT_TP do { } while (0)
-#endif
 
   if (!was_tls_init_tp_called && GL(dl_tls_max_dtv_idx) > 0)
     ++GL(dl_tls_generation);