[RFC] killing remaining USE___THREAD use

Message ID mvmeh071f0p.fsf@hawking.suse.de
State Committed
Headers

Commit Message

Andreas Schwab May 6, 2014, 8:35 a.m. UTC
  Ondřej Bílka <neleai@seznam.cz> writes:

> I checked how its bitrotten, it gives following error:
>
> In file included from rtld.c:376:0:
> dynamic-link.h: In function ‘_dl_start’:
> dynamic-link.h:179:8: warning: "ELF_MACHINE_NO_RELA" is not defined
> [-Wundef]
> rtld.c:481:21: error: incompatible types when assigning to type ‘struct
> <anonymous>’ from type ‘void *’
> rtld.c:489:2: error: incompatible type for argument 1 of
> ‘__builtin_mempcpy’
> rtld.c:489:2: note: expected ‘void *’ but argument is of type ‘struct
> <anonymous>’

This is easy to fix, but the question remains on what this is intented
to do.

Andreas.
  

Comments

Roland McGrath May 6, 2014, 10:06 p.m. UTC | #1
I'm not sure what you think the mystery is.  There was a period when we
thought we'd want to use __thread inside ld.so itself.  We later concluded
that RTLD_PRIVATE_ERRNO served the only need for it at least as well and it
simplified things not to need ld.so to have a PT_TLS.  I'm not sure whether
USE___THREAD-on state in ld.so was ever really used and tested or not, but
I don't think it matters.  Either it was always broken or it bit-rotted.
Either way, it's dead code now and there's no reason not to remove it.
There is no mystery as to the intent of the code.

Removing that code will leave us with the TLS_INIT_TP macro having a second
argument that is never used (always passed 0).  That argument is only
consulted on i386 (and AFAIK was only ever used there).  We could remove
it, though it doesn't hurt to leave that much smaller piece of dead code
around just to move a little more slowly.  (Actually there are two pieces:
in linux/i386/tls.h and in hurd/i386/tls.h.)  It's in macros (and an inline
for Hurd) where it should always be optimized away at compile time.  The
comment in sysdeps/generic/tls.h that should be setting the API for these
macros inverts the sense of the second argument.  That should be updated
either to fix that or to remove the argument if that's what we decide.


Thanks,
Roland
  

Patch

diff --git a/elf/rtld.c b/elf/rtld.c
index 9d121dc..45d77e0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -300,7 +300,6 @@  _dl_start_final (void *arg, struct dl_start_final_info *info)
   GL(dl_rtld_map).l_text_end = (ElfW(Addr)) _etext;
   /* Copy the TLS related data if necessary.  */
 #ifndef DONT_USE_BOOTSTRAP_MAP
-# if USE___THREAD
   assert (info->l.l_tls_modid != 0);
   GL(dl_rtld_map).l_tls_blocksize = info->l.l_tls_blocksize;
   GL(dl_rtld_map).l_tls_align = info->l.l_tls_align;
@@ -309,12 +308,6 @@  _dl_start_final (void *arg, struct dl_start_final_info *info)
   GL(dl_rtld_map).l_tls_initimage = info->l.l_tls_initimage;
   GL(dl_rtld_map).l_tls_offset = info->l.l_tls_offset;
   GL(dl_rtld_map).l_tls_modid = 1;
-# else
-#  if NO_TLS_OFFSET != 0
-  GL(dl_rtld_map).l_tls_offset = NO_TLS_OFFSET;
-#  endif
-# endif
-
 #endif
 
 #if HP_TIMING_AVAIL
@@ -396,9 +389,7 @@  _dl_start (void *arg)
        ++cnt)
     bootstrap_map.l_info[cnt] = 0;
 # endif
-# if USE___THREAD
   bootstrap_map.l_tls_modid = 0;
-# endif
 #endif
 
   /* Figure out the run-time load address of the dynamic linker itself.  */
@@ -417,15 +408,13 @@  _dl_start (void *arg)
      to it.  When we have something like GOTOFF relocs, we can use a plain
      reference to find the runtime address.  Without that, we have to rely
      on the `l_addr' value, which is not the value we want when prelinked.  */
-#if USE___THREAD
   dtv_t initdtv[3];
   ElfW(Ehdr) *ehdr
-# ifdef DONT_USE_BOOTSTRAP_MAP
+#ifdef DONT_USE_BOOTSTRAP_MAP
     = (ElfW(Ehdr) *) &_begin;
-# else
-#  error This will not work with prelink.
+#else
     = (ElfW(Ehdr) *) bootstrap_map.l_addr;
-# endif
+#endif
   ElfW(Phdr) *phdr = (ElfW(Phdr) *) ((void *) ehdr + ehdr->e_phoff);
   size_t cnt = ehdr->e_phnum;	/* PT_TLS is usually the last phdr.  */
   while (cnt-- > 0)
@@ -451,21 +440,21 @@  _dl_start (void *arg)
 	   on the stack.  We'll get the final memory later when we
 	   know all about the various objects loaded at startup
 	   time.  */
-# if TLS_TCB_AT_TP
+#if TLS_TCB_AT_TP
 	tlsblock = alloca (roundup (bootstrap_map.l_tls_blocksize,
 				    TLS_INIT_TCB_ALIGN)
 			   + TLS_INIT_TCB_SIZE
 			   + max_align);
-# elif TLS_DTV_AT_TP
+#elif TLS_DTV_AT_TP
 	tlsblock = alloca (roundup (TLS_INIT_TCB_SIZE,
 				    bootstrap_map.l_tls_align)
 			   + bootstrap_map.l_tls_blocksize
 			   + max_align);
-# else
+#else
 	/* In case a model with a different layout for the TCB and DTV
 	   is defined add another #elif here and in the following #ifs.  */
-#  error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
-# endif
+# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
 	/* Align the TLS block.  */
 	tlsblock = (void *) (((uintptr_t) tlsblock + max_align - 1)
 			     & ~(max_align - 1));
@@ -476,33 +465,34 @@  _dl_start (void *arg)
 	initdtv[1].counter = 0;
 
 	/* Initialize the TLS block.  */
-# if TLS_TCB_AT_TP
-	initdtv[2].pointer = tlsblock;
-# elif TLS_DTV_AT_TP
+#if TLS_TCB_AT_TP
+	initdtv[2].pointer.val = tlsblock;
+#elif TLS_DTV_AT_TP
 	bootstrap_map.l_tls_offset = roundup (TLS_INIT_TCB_SIZE,
 					      bootstrap_map.l_tls_align);
-	initdtv[2].pointer = (char *) tlsblock + bootstrap_map.l_tls_offset;
-# else
-#  error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
-# endif
-	p = __mempcpy (initdtv[2].pointer, bootstrap_map.l_tls_initimage,
+	initdtv[2].pointer.val = (char *) tlsblock + bootstrap_map.l_tls_offset;
+#else
+# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
+	initdtv[2].pointer.is_static = true;
+	p = __mempcpy (initdtv[2].pointer.val, bootstrap_map.l_tls_initimage,
 		       bootstrap_map.l_tls_initimage_size);
-# ifdef HAVE_BUILTIN_MEMSET
+#ifdef HAVE_BUILTIN_MEMSET
 	__builtin_memset (p, '\0', (bootstrap_map.l_tls_blocksize
 				    - bootstrap_map.l_tls_initimage_size));
-# else
+#else
 	{
 	  size_t remaining = (bootstrap_map.l_tls_blocksize
 			      - bootstrap_map.l_tls_initimage_size);
 	  while (remaining-- > 0)
 	    *p++ = '\0';
 	}
-# endif
+#endif
 
 	/* Install the pointer to the dtv.  */
 
 	/* Initialize the thread pointer.  */
-# if TLS_TCB_AT_TP
+#if TLS_TCB_AT_TP
 	bootstrap_map.l_tls_offset
 	  = roundup (bootstrap_map.l_tls_blocksize, TLS_INIT_TCB_ALIGN);
 
@@ -511,12 +501,12 @@  _dl_start (void *arg)
 
 	const char *lossage = TLS_INIT_TP ((char *) tlsblock
 					   + bootstrap_map.l_tls_offset, 0);
-# elif TLS_DTV_AT_TP
+#elif TLS_DTV_AT_TP
 	INSTALL_DTV (tlsblock, initdtv);
 	const char *lossage = TLS_INIT_TP (tlsblock, 0);
-# else
-#  error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
-# endif
+#else
+# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
 	if (__glibc_unlikely (lossage != NULL))
 	  _dl_fatal_printf ("cannot set up thread-local storage: %s\n",
 			    lossage);
@@ -527,7 +517,6 @@  _dl_start (void *arg)
 	/* There can only be one PT_TLS entry.  */
 	break;
       }
-#endif	/* USE___THREAD */
 
 #ifdef ELF_MACHINE_BEFORE_RTLD_RELOC
   ELF_MACHINE_BEFORE_RTLD_RELOC (bootstrap_map.l_info);
@@ -774,12 +763,7 @@  cannot allocate TLS data structures for initial thread");
 
   /* And finally install it for the main thread.  If ld.so itself uses
      TLS we know the thread pointer was initialized earlier.  */
-  const char *lossage
-#ifdef USE___THREAD
-    = TLS_INIT_TP (tcbp, USE___THREAD);
-#else
-    = TLS_INIT_TP (tcbp, 0);
-#endif
+  const char *lossage = TLS_INIT_TP (tcbp, 1);
   if (__glibc_unlikely (lossage != NULL))
     _dl_fatal_printf ("cannot set up thread-local storage: %s\n", lossage);
   tls_init_tp_called = true;
@@ -2253,12 +2237,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
      TLS we know the thread pointer was initialized earlier.  */
   if (! tls_init_tp_called)
     {
-      const char *lossage
-#ifdef USE___THREAD
-	= TLS_INIT_TP (tcbp, USE___THREAD);
-#else
-	= TLS_INIT_TP (tcbp, 0);
-#endif
+      const char *lossage = TLS_INIT_TP (tcbp, 1);
       if (__glibc_unlikely (lossage != NULL))
 	_dl_fatal_printf ("cannot set up thread-local storage: %s\n",
 			  lossage);