Question about elf/dl-load.c

Question about elf/dl-load.c

[Florian Weimer]

I found this bit in _dl_map_object_from_fd:

	case PT_TLS:
	  if (ph->p_memsz == 0)
	    /* Nothing to do for an empty segment.  */
	    break;

	  l->l_tls_blocksize = ph->p_memsz;
	  l->l_tls_align = ph->p_align;
	  if (ph->p_align == 0)
	    l->l_tls_firstbyte_offset = 0;
	  else
	    l->l_tls_firstbyte_offset = ph->p_vaddr & (ph->p_align - 1);
	  l->l_tls_initimage_size = ph->p_filesz;
	  /* Since we don't know the load address yet only store the
	     offset.  We will adjust it later.  */
	  l->l_tls_initimage = (void *) ph->p_vaddr;

	  /* If not loading the initial set of shared libraries,
	     check whether we should permit loading a TLS segment.  */
	  if (__glibc_likely (l->l_type == lt_library)
	      /* If GL(dl_tls_dtv_slotinfo_list) == NULL, then rtld.c did
		 not set up TLS data structures, so don't use them now.  */
	      || __glibc_likely (GL(dl_tls_dtv_slotinfo_list) != NULL))
	    {
	      /* Assign the next available module ID.  */
	      l->l_tls_modid = _dl_next_tls_modid ();
	      break;
	    }

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

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.

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

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

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


[-- Attachment #2: pt_tls.patch --]
[-- Type: text/x-patch, Size: 2443 bytes --]

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:

Re: [PATCH] elf: Assume TLS is initialized in _dl_map_object_from_fd

[Florian Weimer]

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

Re: [PATCH] elf: Assume TLS is initialized in _dl_map_object_from_fd

[Florian Weimer]

I have checked this in after verifying that it does not break basic 
desktop usage on Fedora rawhide (x86_64).

Thanks,
Florian