diff mbox

elf: Assume TLS is initialized in _dl_map_object_from_fd (was: Re: Question about elf/dl-load.c)

Message ID 80382a38-1490-264a-789e-0a979f10de53@redhat.com
State New
Headers show

Commit Message

Florian Weimer Nov. 14, 2016, 2:57 p.m. UTC 
On 11/11/2016 10:17 PM, Florian Weimer wrote:

> And so on.  My question is this: Can we actually enter the final part,

> under “We can do the TLS right now!” in a shared build?

>

> I doubt that because during the initial link, we will hit the second

> break statement for libraries, and the third break statement for the

> main program.  Before user code runs, rtld.c sets up the TLS data

> structures, so any future dlopen calls hit the second break again

> because GL(dl_tls_dtv_slotinfo_list) != NULL at this point.

>

> Is this reasoning correct?  I put in a debug printf and ran the elf

> tests, and there was nothing printed.


I've simplified the code accordingly and added asserts.

I have verified that the elf/* test suite still passes on s390, s390x, 
ppc64, ppc, x86_64, i386.  On x86_64, I also performed an installed-tree 
test (and the assert did not fire).

Thanks,
Florian

Comments

Florian Weimer Nov. 22, 2016, 2:08 p.m. UTC | #1 
On 11/14/2016 03:57 PM, Florian Weimer wrote:
> On 11/11/2016 10:17 PM, Florian Weimer wrote:

>

>> And so on.  My question is this: Can we actually enter the final part,

>> under “We can do the TLS right now!” in a shared build?

>>

>> I doubt that because during the initial link, we will hit the second

>> break statement for libraries, and the third break statement for the

>> main program.  Before user code runs, rtld.c sets up the TLS data

>> structures, so any future dlopen calls hit the second break again

>> because GL(dl_tls_dtv_slotinfo_list) != NULL at this point.

>>

>> Is this reasoning correct?  I put in a debug printf and ran the elf

>> tests, and there was nothing printed.

>

> I've simplified the code accordingly and added asserts.

>

> I have verified that the elf/* test suite still passes on s390, s390x,

> ppc64, ppc, x86_64, i386.  On x86_64, I also performed an installed-tree

> test (and the assert did not fire).


I will check this in soon unless someone objects.

TLS initialization has P&C issues, and I hope this cleanup makes it 
easier to fix that.  (It is impossible to review dead code for P&C issues.)

Thanks,
Florian
Florian Weimer Nov. 23, 2016, 12:44 p.m. UTC | #2 
I have checked this in after verifying that it does not break basic 
desktop usage on Fedora rawhide (x86_64).

Thanks,
Florian
diff mbox

Patch

elf: Assume TLS is initialized in _dl_map_object_from_fd

libc.so uses TLS data, so when dlopen is called later, the
TLS data structures have already been initialized.

2016-11-14  Florian Weimer  <fweimer@redhat.com>

	* elf/dl-load.c (_dl_map_object_from_fd): Delayed TLS data
	structure initialization is no longer needed.

diff --git a/elf/dl-load.c b/elf/dl-load.c
index c0d6249..51fb0d0 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1135,54 +1135,14 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	    }
 
 #ifdef SHARED
-	  if (l->l_prev == NULL || (mode & __RTLD_AUDIT) != 0)
-	    /* We are loading the executable itself when the dynamic linker
-	       was executed directly.  The setup will happen later.  */
-	    break;
-
-# ifdef _LIBC_REENTRANT
-	  /* In a static binary there is no way to tell if we dynamically
-	     loaded libpthread.  */
-	  if (GL(dl_error_catch_tsd) == &_dl_initial_error_catch_tsd)
-# endif
+	  /* We are loading the executable itself when the dynamic
+	     linker was executed directly.  The setup will happen
+	     later.  Otherwise, the TLS data structures are already
+	     initialized, and we assigned a TLS modid above.  */
+	  assert (l->l_prev == NULL || (mode & __RTLD_AUDIT) != 0);
+#else
+	  assert (false && "TLS not initialized in static application");
 #endif
-	    {
-	      /* We have not yet loaded libpthread.
-		 We can do the TLS setup right now!  */
-
-	      void *tcb;
-
-	      /* The first call allocates TLS bookkeeping data structures.
-		 Then we allocate the TCB for the initial thread.  */
-	      if (__glibc_unlikely (_dl_tls_setup ())
-		  || __glibc_unlikely ((tcb = _dl_allocate_tls (NULL)) == NULL))
-		{
-		  errval = ENOMEM;
-		  errstring = N_("\
-cannot allocate TLS data structures for initial thread");
-		  goto call_lose;
-		}
-
-	      /* Now we install the TCB in the thread register.  */
-	      errstring = TLS_INIT_TP (tcb);
-	      if (__glibc_likely (errstring == NULL))
-		{
-		  /* Now we are all good.  */
-		  l->l_tls_modid = ++GL(dl_tls_max_dtv_idx);
-		  break;
-		}
-
-	      /* The kernel is too old or somesuch.  */
-	      errval = 0;
-	      _dl_deallocate_tls (tcb, 1);
-	      goto call_lose;
-	    }
-
-	  /* Uh-oh, the binary expects TLS support but we cannot
-	     provide it.  */
-	  errval = 0;
-	  errstring = N_("cannot handle TLS data");
-	  goto call_lose;
 	  break;
 
 	case PT_GNU_STACK: