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

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


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

[PATCH] Fix __libc_rwlock_* in NPTL


Hi!

#include <dlfcn.h>
#include <libintl.h>
#include <locale.h>
#include <malloc.h>

int
main (void)
{
  mallopt (M_PERTURB, 152);
  dlopen ("libpthread.so.0", RTLD_LAZY);
  setlocale (LC_ALL, "ja_JP.UTF-8");
  textdomain ("libc");
  dgettext (NULL, "Hangup");
  return 0;
}

hangs with trunk glibc when linked with -ldl, but not -lpthread.
The problem is that while __libc_rwlock_{rd,wr,un}lock use
__libc_ptf_call macro and therefore in libc.so will call
some __libc_pthread_functions.* function, __libc_rwlock_{init,fini}
use __libc_maybe_call, which
1) means additional relocations in libc.so (two for each even)
2) if libpthread.so is lt_loaded, then the weak relocs in libc.so
   are resolved to NULL and therefore __libc_rwlock_{init,fini}
   is essentially a nop.  So conversions_lock (the only one
   for which we use __libc_rwlock_init ATM) is unitialized, but
   later we call __libc_rwlock_rdlock etc. on it - if libpthread.so
   is loaded at that point, it is accessing unitialized data
One possible fix might seem to be making __libc_rwlock_init
also __libc_ptf_call (i.e. add another function pointer to
__libc_pthread_functions).  But that is IMHO still risky,
we don't have any hook which would be called when libpthread.so
is called.  If a program not linked against libpthread.so
calls dgettext which needs conversions (__libc_rwlock_init
would do nothing, as __libc_pthread_functions wasn't inited,
__libc_rwlock_rdlock etc. would be a nop too for the same reasons),
then dlopens libpthread.so and then calls dgettext for the same domain
(which is still around, hasn't been unloaded), then nothing calls
__libc_rwlock_init again, but __libc_rwlock_rdlock is used on unitialized
data.

IMHO we don't need to call any function for this inside of libc.so,
we know the pthread library is NPTL and how are its rwlocks initialized.
Unfortunately, GCC, even including GCC 4.3, generates terrible code
for ((NAME) = (pthread_rwlock_t) PTHREAD_RWLOCK_INITIALIZER, 0)
- filed upstream PR about it - but ATM we know it is all zeros.
I have added a test for that with a comment to tst-initializers1.c,
so that if we ever change it, we'll be reminded to change
__libc_rwlock_init. 

2007-10-10  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/pthread/bits/libc-lock.h (__libc_rwlock_init): Inside of
	libc.so just clear NAME.
	(__libc_rwlock_fini): Nop inside of libc.so.
	* tst-initializers1.c (main): Test if PTHREAD_RWLOCK_INITIALIZER is
	all zeros.

--- libc/nptl/sysdeps/pthread/bits/libc-lock.h.jj	2007-10-10 12:50:11.000000000 +0200
+++ libc/nptl/sysdeps/pthread/bits/libc-lock.h	2007-10-10 13:21:42.000000000 +0200
@@ -172,8 +172,15 @@ typedef pthread_key_t __libc_key_t;
 # define __libc_lock_init(NAME) \
   __libc_maybe_call (__pthread_mutex_init, (&(NAME), NULL), 0)
 #endif
-#define __libc_rwlock_init(NAME) \
+#if defined SHARED && !defined NOT_IN_libc
+/* ((NAME) = (__libc_rwlock_t) PTHREAD_RWLOCK_INITIALIZER, 0) is
+   inefficient.  */
+# define __libc_rwlock_init(NAME) \
+  (__builtin_memset (&(NAME), '\0', sizeof (NAME)), 0)
+#else
+# define __libc_rwlock_init(NAME) \
   __libc_maybe_call (__pthread_rwlock_init, (&(NAME), NULL), 0)
+#endif
 
 /* Same as last but this time we initialize a recursive mutex.  */
 #if defined _LIBC && (!defined NOT_IN_libc || defined IS_IN_libpthread)
@@ -214,8 +221,12 @@ typedef pthread_key_t __libc_key_t;
 # define __libc_lock_fini(NAME) \
   __libc_maybe_call (__pthread_mutex_destroy, (&(NAME)), 0)
 #endif
-#define __libc_rwlock_fini(NAME) \
+#if defined SHARED && !defined NOT_IN_libc
+# define __libc_rwlock_fini(NAME) ((void) 0)
+#else
+# define __libc_rwlock_fini(NAME) \
   __libc_maybe_call (__pthread_rwlock_destroy, (&(NAME)), 0)
+#endif
 
 /* Finalize recursive named lock.  */
 #if defined _LIBC && (!defined NOT_IN_libc || defined IS_IN_libpthread)
--- libc/nptl/tst-initializers1.c.jj	2007-06-04 08:42:05.000000000 +0200
+++ libc/nptl/tst-initializers1.c	2007-10-10 13:26:04.000000000 +0200
@@ -47,5 +47,12 @@ main (void)
   if (rwl_writer.__data.__flags
       != PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP)
     return 6;
+  /* <bits/libc-lock.h> __libc_rwlock_init definition for libc.so
+     relies on PTHREAD_RWLOCK_INITIALIZER being all zeros.  If
+     that ever changes, <bits/libc-lock.h> needs updating.  */
+  size_t i;
+  for (i = 0; i < sizeof (rwl_normal); i++)
+    if (((char *) &rwl_normal)[i] != '\0')
+      return 7;
   return 0;
 }

	Jakub


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