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]

static tls memory leak


This patch fixes some errors in static tls handling, TLS_DTV_AT_TP
case.  tst-tls13 fails on powerpc64 if gcc happens to align the TLS
segment, due to dlclose not freeing static tls space properly.  With
one version of gcc, at -O3 I was seeing tst-tlsmod13a with
  TLS 0x000e70 0x0000000000010e70 0x0000000000010e70 0x000000 0x000008 R 0x10
At -O2
  TLS 0x000e70 0x0000000000010e70 0x0000000000010e70 0x000000 0x000008 R 0x4
The two object files are byte for byte identical except for this
alignment difference, 16 vs 4, and the corresponding alignment
difference in the .tbss section.  I'm sure it is quite permissible for
gcc to request a larger alignment.

So in the -O3 case glibc allocates 8 bytes for tlsmod13, pads 8 bytes,
then allocates 8 bytes for tlsmod13a.  On dlclose, the 8 bytes of
tlsmod13a space is freed, but the pad confuses dlclose code when
trying to free the block for tlsmod13, so it isn't freed.  Net result
is a leak of 16 bytes every dlopen/dlclose cycle.  glibc allocates
TLS_STATIC_SURPLUS = 64 + 16 * 100 = 1664 bytes space for static TLS
which explains why we bomb on the 104th iteration in the test loop.

Note that the padding belongs to the second tls block allocated, not
the first.  (It's the second block alignment that causes glibc to pad,
not the first block alignment.)  Thus a proper fix means tracking the
padding added at dlopen time and freeing it at dlclose, as I do in the
following patch.  I've also fixed an error in dl-close.c "Extend the
contiguous chunk being reclaimed" code, and better supported
non-contiguous freeing as was done for TLS_TCB_AT_TP.

This patch does not teach the TLS_TCB_AT_TP code to similarly free
padding.  The _dl_debug_printf statements I used when developing this
patch should help anyone who wants to fix x86.  If you'd like a patch
without the debug print, please ask.

2011-03-15  Alan Modra  <amodra@gmail.com>

	* elf/dl-reloc.c (_dl_try_allocate_static_tls <TLS_DTV_AT_TP>): Handle
	l_tls_firstbyte_offset non-zero.  Save padding offset in
	l_tls_firstbyte_offset for later use.  Add debug print.
	* elf/dl-close.c (_dl_close_worker <TLS_DTV_AT_TP>): Correct code
	freeing static tls block.  Add debug print.
	* elf/fl-tls.c (_dl_determine_tlsoffset): Add debug print.

diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 23cb59c..9d07fd6 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -77,23 +77,33 @@ _dl_try_allocate_static_tls (struct link_map *map)
 
   map->l_tls_offset = GL(dl_tls_static_used) = offset;
 #elif TLS_DTV_AT_TP
+  size_t offset;
   size_t used;
-  size_t check;
 
-  size_t offset = roundup (GL(dl_tls_static_used), map->l_tls_align);
-  used = offset + map->l_tls_blocksize;
-  check = used;
   /* dl_tls_static_used includes the TCB at the beginning.  */
+  offset = (((GL(dl_tls_static_used)
+	      - map->l_tls_firstbyte_offset
+	      + map->l_tls_align - 1) & -map->l_tls_align)
+	    + map->l_tls_firstbyte_offset);
+  used = offset + map->l_tls_blocksize;
 
-  if (check > GL(dl_tls_static_size))
+  if (used > GL(dl_tls_static_size))
     goto fail;
 
   map->l_tls_offset = offset;
+  map->l_tls_firstbyte_offset = GL(dl_tls_static_used);
   GL(dl_tls_static_used) = used;
 #else
 # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 #endif
 
+  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_RELOC, 0))
+    _dl_debug_printf ("\nstatic tls: %s\n"
+		      "  l_tls_offset = %Zu, l_tls_firstbyte_offset = %Zu,"
+		      " dl_tls_static_used = %Zu\n",
+		      map->l_name[0] ? map->l_name : rtld_progname,
+		      map->l_tls_offset, map->l_tls_firstbyte_offset,
+		      GL(dl_tls_static_used));
   /* If the object is not yet relocated we cannot initialize the
      static TLS region.  Delay it.  */
   if (map->l_real->l_relocated)
diff --git a/elf/dl-close.c b/elf/dl-close.c
index efb2b58..fa32591 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -591,25 +591,51 @@ _dl_close_worker (struct link_map *map)
 			}
 		    }
 #elif TLS_DTV_AT_TP
-		  if ((size_t) imap->l_tls_offset == tls_free_end)
+		  if (tls_free_start == NO_TLS_OFFSET)
+		    {
+		      tls_free_start = imap->l_tls_firstbyte_offset;
+		      tls_free_end = imap->l_tls_offset + imap->l_tls_blocksize;
+		    }
+		  else if (imap->l_tls_firstbyte_offset == tls_free_end)
 		    /* Extend the contiguous chunk being reclaimed.  */
-		    tls_free_end -= imap->l_tls_blocksize;
+		    tls_free_end = imap->l_tls_offset + imap->l_tls_blocksize;
 		  else if (imap->l_tls_offset + imap->l_tls_blocksize
 			   == tls_free_start)
 		    /* Extend the chunk backwards.  */
-		    tls_free_start = imap->l_tls_offset;
-		  else
+		    tls_free_start = imap->l_tls_firstbyte_offset;
+
+		  /* This isn't contiguous with the last chunk freed.
+		     One of them will be leaked unless we can free
+		     one block right away.  */
+		  else if (imap->l_tls_offset + imap->l_tls_blocksize
+			   == GL(dl_tls_static_used))
+		    GL(dl_tls_static_used) = imap->l_tls_firstbyte_offset;
+		  else if (tls_free_end == GL(dl_tls_static_used))
 		    {
-		      /* This isn't contiguous with the last chunk freed.
-			 One of them will be leaked.  */
-		      if (tls_free_end == GL(dl_tls_static_used))
-			GL(dl_tls_static_used) = tls_free_start;
-		      tls_free_start = imap->l_tls_offset;
-		      tls_free_end = tls_free_start + imap->l_tls_blocksize;
+		      GL(dl_tls_static_used) = tls_free_start;
+		      tls_free_start = imap->l_tls_firstbyte_offset;
+		      tls_free_end = imap->l_tls_offset + imap->l_tls_blocksize;
+		    }
+		  else if (tls_free_end < imap->l_tls_firstbyte_offset)
+		    {
+		      /* We pick the later block.  It has a chance to
+			 be freed.  */
+		      tls_free_start = imap->l_tls_firstbyte_offset;
+		      tls_free_end = imap->l_tls_offset + imap->l_tls_blocksize;
 		    }
 #else
 # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 #endif
+		  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_RELOC, 0))
+		    _dl_debug_printf ("\nstatic tls free: %s\n"
+				      "  tls_free_start = %Zu, "
+				      "tls_free_end = %Zu, "
+				      "dl_tls_static_used = %Zu\n",
+				      imap->l_name[0] ? imap->l_name
+				      : rtld_progname,
+				      tls_free_start, tls_free_end,
+				      GL(dl_tls_static_used));
+
 		}
 	    }
 
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 824adc1..8db7c85 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -249,6 +249,11 @@ _dl_determine_tlsoffset (void)
 # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 #endif
 
+  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_RELOC, 0))
+    _dl_debug_printf ("\ninitial static tls: "
+		      "dl_tls_static_used = %Zu, dl_tls_status_size = %Zu\n",
+		      GL(dl_tls_static_used), GL(dl_tls_static_size));
+
   /* The alignment requirement for the static TLS block.  */
   GL(dl_tls_static_align) = max_align;
 }

-- 
Alan Modra
Australia Development Lab, IBM


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