This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: don't accept IE relocs for module that has dynamic TLS blocks


On Oct 10, 2006, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Feb  6, 2006, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Sep 22, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> On Sep 17, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:
>>>>> The patch also adds a test to fix a problem I ran into while testing
>>>>> these optimizations, that turned out to be possible to trigger with
>>>>> the old code.

>>> Split out, per Roland's request.

>> Any reason why this hasn't been reviewed or installed yet?  It's been
>> pending for a while, and it would be nice if GLIBC 2.4 had this fix.

> Ping?

> Re-tested updated patch.

Ping?

I've last re-tested this bug fix on the date in the ChangeLog entry
below.  It still applies cleanly.


As shown in the testcase, when we accept a relocation to static TLS,
we must make sure the referenced symbol has never been assigned to
dynamic TLS before.  This requires __tls_get_addr() to commit a
module to a new state in which it's no longer eligible for static
TLS, and have the code used to test for static TLS adjusted,
especially the code that tries to commit a module to static TLS.

for ChangeLog
2007-10-23  Alexandre Oliva  <aoliva@redhat.com>

	* include/link.h (FORCED_DYNAMIC_TLS_OFFSET): Define.
	* elf/dl-tls.c (_dl_allocate_tls_init): Check for it.
	* elf/dl-close.c (_dl_close): Likewise.
	* elf/dl-reloc.c (CHECK_STATIC_TLS): Likewise.
	(_dl_allocate_static_tls): Likewise.
	* elf/dl-tls.c (__tls_get_addr): Protect from race
	conditions in setting l_tls_offset to it.
	* elf/tst-tls16.c: New test.
	* elf/tst-tlsmod16a.c, elf/tst-tlsmod16b.c: Supporting libraries.
	* elf/Makefile: Set it up.

Index: elf/Makefile
===================================================================
--- elf/Makefile.orig	2007-10-30 05:35:39.000000000 -0200
+++ elf/Makefile	2007-10-30 06:03:10.000000000 -0200
@@ -165,7 +165,7 @@ tests += loadtest restest1 preloadtest l
 	 restest2 next dblload dblunload reldep5 reldep6 reldep7 reldep8 \
 	 circleload1 tst-tls3 tst-tls4 tst-tls5 tst-tls6 tst-tls7 tst-tls8 \
 	 tst-tls10 tst-tls11 tst-tls12 tst-tls13 tst-tls14 tst-tls15 \
-	 tst-tls-dlinfo \
+	 tst-tls16 tst-tls-dlinfo \
 	 tst-align tst-align2 $(tests-execstack-$(have-z-execstack)) \
 	 tst-dlmodcount tst-dlopenrpath tst-deep1 \
 	 tst-dlmopen1 tst-dlmopen2 tst-dlmopen3 \
@@ -199,7 +199,7 @@ modules-names = testobj1 testobj2 testob
 		tst-tlsmod5 tst-tlsmod6 tst-tlsmod7 tst-tlsmod8 \
 		tst-tlsmod9 tst-tlsmod10 tst-tlsmod11 tst-tlsmod12 \
 		tst-tlsmod13 tst-tlsmod13a tst-tlsmod14a tst-tlsmod14b \
-		tst-tlsmod15a tst-tlsmod15b \
+		tst-tlsmod15a tst-tlsmod15b tst-tlsmod16a tst-tlsmod16b \
 		circlemod1 circlemod1a circlemod2 circlemod2a \
 		circlemod3 circlemod3a \
 		reldep8mod1 reldep8mod2 reldep8mod3 \
@@ -492,6 +492,7 @@ tst-tlsmod12.so-no-z-defs = yes
 tst-tlsmod14a.so-no-z-defs = yes
 tst-tlsmod14b.so-no-z-defs = yes
 tst-tlsmod15a.so-no-z-defs = yes
+tst-tlsmod16b.so-no-z-defs = yes
 circlemod2.so-no-z-defs = yes
 circlemod3.so-no-z-defs = yes
 circlemod3a.so-no-z-defs = yes
@@ -711,6 +712,9 @@ $(objpfx)tst-tls-dlinfo.out: $(objpfx)ts
 
 
 
+$(objpfx)tst-tls16: $(libdl)
+$(objpfx)tst-tls16.out: $(objpfx)tst-tlsmod16a.so $(objpfx)tst-tlsmod16b.so
+
 CFLAGS-tst-align.c = $(stack-align-test-flags)
 CFLAGS-tst-align2.c = $(stack-align-test-flags)
 CFLAGS-tst-alignmod.c = $(stack-align-test-flags)
Index: elf/dl-close.c
===================================================================
--- elf/dl-close.c.orig	2007-10-30 05:35:39.000000000 -0200
+++ elf/dl-close.c	2007-10-30 06:03:10.000000000 -0200
@@ -531,7 +531,8 @@ _dl_close_worker (struct link_map *map)
 		/* All dynamically loaded modules with TLS are unloaded.  */
 		GL(dl_tls_max_dtv_idx) = GL(dl_tls_static_nelem);
 
-	      if (imap->l_tls_offset != NO_TLS_OFFSET)
+	      if (imap->l_tls_offset != NO_TLS_OFFSET
+		  && imap->l_tls_offset != FORCED_DYNAMIC_TLS_OFFSET)
 		{
 		  /* Collect a contiguous chunk built from the objects in
 		     this search list, going in either direction.  When the
Index: elf/dl-reloc.c
===================================================================
--- elf/dl-reloc.c.orig	2007-10-30 05:34:14.000000000 -0200
+++ elf/dl-reloc.c	2007-10-30 06:03:10.000000000 -0200
@@ -47,8 +47,10 @@ void
 internal_function __attribute_noinline__
 _dl_allocate_static_tls (struct link_map *map)
 {
-  /* If the alignment requirements are too high fail.  */
-  if (map->l_tls_align > GL(dl_tls_static_align))
+  /* If we've already used the variable with dynamic access, or if the
+     alignment requirements are too high, fail.  */
+  if (map->l_tls_offset == FORCED_DYNAMIC_TLS_OFFSET
+      || map->l_tls_align > GL(dl_tls_static_align))
     {
     fail:
       _dl_signal_error (0, map->l_name, NULL, N_("\
@@ -255,10 +257,12 @@ _dl_relocate_object (struct link_map *l,
        an attempt to allocate it in surplus space on the fly.  If that
        can't be done, we fall back to the error that DF_STATIC_TLS is
        intended to produce.  */
-#define CHECK_STATIC_TLS(map, sym_map)					      \
-    do {								      \
-      if (__builtin_expect ((sym_map)->l_tls_offset == NO_TLS_OFFSET, 0))     \
-	_dl_allocate_static_tls (sym_map);				      \
+#define CHECK_STATIC_TLS(map, sym_map)					\
+    do {								\
+      if (__builtin_expect ((sym_map)->l_tls_offset == NO_TLS_OFFSET	\
+			    || ((sym_map)->l_tls_offset			\
+				== FORCED_DYNAMIC_TLS_OFFSET), 0))	\
+	_dl_allocate_static_tls (sym_map);				\
     } while (0)
 
 #include "dynamic-link.h"
Index: elf/tst-tls16.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ elf/tst-tls16.c	2007-10-30 06:03:10.000000000 -0200
@@ -0,0 +1,52 @@
+#include <dlfcn.h>
+#include <stdio.h>
+
+static int
+do_test (void)
+{
+  void *h = dlopen ("tst-tlsmod16a.so", RTLD_LAZY | RTLD_GLOBAL);
+  if (h == NULL)
+    {
+      puts ("unexpectedly failed to open tst-tlsmod16a.so");
+      exit (1);
+    }
+
+  void *p = dlsym (h, "tlsvar");
+
+  /* This dlopen should indeed fail, because tlsvar was assigned to
+     dynamic TLS, and the new module requests it to be in static TLS.
+     However, there's a possibility that dlopen succeeds if the
+     variable is, for whatever reason, assigned to static TLS, or if
+     the module fails to require static TLS, or even if TLS is not
+     supported.  */
+  h = dlopen ("tst-tlsmod16b.so", RTLD_NOW | RTLD_GLOBAL);
+  if (h == NULL)
+    {
+      return 0;
+    }
+
+  puts ("unexpectedly succeeded to open tst-tlsmod16b.so");
+
+
+  void *(*fp) (void) = (void *(*) (void)) dlsym (h, "in_dso");
+  if (fp == NULL)
+    {
+      puts ("cannot find in_dso");
+      exit (1);
+    }
+
+  /* If the dlopen passes, at least make sure the address returned by
+     dlsym is the same as that returned by the initial-exec access.
+     If the variable was assigned to dynamic TLS during dlsym, this
+     portion will fail.  */
+  if (fp () != p)
+    {
+      puts ("returned values do not match");
+      exit (1);
+    }
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
Index: elf/tst-tlsmod16a.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ elf/tst-tlsmod16a.c	2007-10-30 06:03:10.000000000 -0200
@@ -0,0 +1,7 @@
+#include <tls.h>
+
+#if defined HAVE___THREAD && defined HAVE_TLS_MODEL_ATTRIBUTE
+int __thread tlsvar;
+#else
+int tlsvar;
+#endif
Index: elf/tst-tlsmod16b.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ elf/tst-tlsmod16b.c	2007-10-30 06:03:10.000000000 -0200
@@ -0,0 +1,13 @@
+#include <tls.h>
+
+#if defined HAVE___THREAD && defined HAVE_TLS_MODEL_ATTRIBUTE
+extern __thread int tlsvar __attribute__((tls_model("initial-exec")));
+#else
+extern int tlsvar;
+#endif
+
+void *
+in_dso (void)
+{
+  return &tlsvar;
+}
Index: include/link.h
===================================================================
--- include/link.h.orig	2007-10-30 05:35:40.000000000 -0200
+++ include/link.h	2007-10-30 06:03:10.000000000 -0200
@@ -279,6 +279,15 @@ struct link_map
 #ifndef NO_TLS_OFFSET
 # define NO_TLS_OFFSET	0
 #endif
+#ifndef FORCED_DYNAMIC_TLS_OFFSET
+# if NO_TLS_OFFSET == 0
+#  define FORCED_DYNAMIC_TLS_OFFSET 1
+# elif NO_TLS_OFFSET == -1
+#  define FORCED_DYNAMIC_TLS_OFFSET -2
+# else
+#  error "FORCED_DYNAMIC_TLS_OFFSET is not defined"
+# endif
+#endif
     /* For objects present at startup time: offset in the static TLS block.  */
     ptrdiff_t l_tls_offset;
     /* Index of the module in the dtv array.  */
Index: elf/dl-tls.c
===================================================================
--- elf/dl-tls.c.orig	2007-10-30 05:34:14.000000000 -0200
+++ elf/dl-tls.c	2007-10-30 06:03:10.000000000 -0200
@@ -413,7 +413,8 @@ _dl_allocate_tls_init (void *result)
 	     not be the generation counter.  */
 	  maxgen = MAX (maxgen, listp->slotinfo[cnt].gen);
 
-	  if (map->l_tls_offset == NO_TLS_OFFSET)
+	  if (map->l_tls_offset == NO_TLS_OFFSET
+	      || map->l_tls_offset == FORCED_DYNAMIC_TLS_OFFSET)
 	    {
 	      /* For dynamically loaded modules we simply store
 		 the value indicating deferred allocation.  */
@@ -702,6 +703,7 @@ __tls_get_addr (GET_ADDR_ARGS)
   if (__builtin_expect (dtv[0].counter != GL(dl_tls_generation), 0))
     the_map = _dl_update_slotinfo (GET_ADDR_MODULE);
 
+ retry:
   p = dtv[GET_ADDR_MODULE].pointer.val;
 
   if (__builtin_expect (p == TLS_DTV_UNALLOCATED, 0))
@@ -722,6 +724,28 @@ __tls_get_addr (GET_ADDR_ARGS)
 	  the_map = listp->slotinfo[idx].map;
 	}
 
+      /* Make sure that, if a dlopen running in parallel forces the
+	 variable into static storage, we'll wait until the address in
+	 the static TLS block is set up, and use that.  If we're
+	 undecided yet, make sure we make the decision holding the
+	 lock as well.  */
+      if (__builtin_expect (the_map->l_tls_offset
+			    != FORCED_DYNAMIC_TLS_OFFSET, 0))
+	{
+	  __rtld_lock_lock_recursive (GL(dl_load_lock));
+	  if (__builtin_expect (the_map->l_tls_offset == NO_TLS_OFFSET, 1))
+	    {
+	      the_map->l_tls_offset = FORCED_DYNAMIC_TLS_OFFSET;
+	      __rtld_lock_unlock_recursive (GL(dl_load_lock));
+	    }
+	  else
+	    {
+	      __rtld_lock_unlock_recursive (GL(dl_load_lock));
+	      if (__builtin_expect (the_map->l_tls_offset
+				    != FORCED_DYNAMIC_TLS_OFFSET, 1))
+		goto retry;
+	    }
+	}
       p = dtv[GET_ADDR_MODULE].pointer.val = allocate_and_init (the_map);
       dtv[GET_ADDR_MODULE].pointer.is_static = false;
     }


-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member         http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]