Message ID | x57jobtqx89w.fsf@frobland.mtv.corp.google.com |
---|---|
State | New |
Headers | show |
On Thu, Jan 26, 2012 at 05:07:38PM -0800, Roland McGrath wrote: > What do folks think about this change? It obviously should have no effect > whatsoever on builds not using glibc. I'm pretty confident that it won't > have any bad effect on a build using any extant version of glibc that had > pthreads at all. > pthread_create is what has been referenced there up to GCC 3.4. So please see the rationale for changing it from pthread_create to pthread_cancel. http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01093.html http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02517.html (and lots of gcc-patches mails in between the two). Jakub
I see. There certainly should have been a comment in the code about why pthread_cancel was chosen. I still think it's a particularly poor choice. For glibc, I think pthread_getspecific or pthread_key_create are better choices. Those are much smaller functions that don't bring very much dead code into the link, and they are things actually used by libstdc++ et al. For supporting the interception cases, it's really not entirely safe to use any of the public API functions. Someone might have intercepted any of them in the same way as was cited for pthread_create. In glibc, we have exported __ names for various things, including __pthread_getspecific and __pthread_key_create. IMHO one of those is the best choice. They are part of the permanent public ABI and so won't ever go away, but they are not part of the public API that anyone writing an interceptor library would ever want to touch. Thanks, Roland
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h index 46054f6..688253d 100644 --- a/libgcc/gthr-posix.h +++ b/libgcc/gthr-posix.h @@ -241,18 +241,24 @@ __gthread_active_p (void) #else /* neither FreeBSD nor Solaris */ +/* The bionic (Android) C library does not provide pthread_cancel. + The GNU C library provides pthread_cancel, but it is a poor + choice there. For static linking, referring to pthread_cancel + causes it to be linked in even when it's never actually used. + For a program to be multi-threaded the only thing that it + certainly must be using is pthread_create. */ + +#if defined(__GLIBC__) || defined(__BIONIC__) +# define GTHR_ACTIVE_PROXY __gthrw_(pthread_create) +#else +# define GTHR_ACTIVE_PROXY __gthrw_(pthread_cancel) +#endif + static inline int __gthread_active_p (void) { -/* Android's C library does not provide pthread_cancel, check for - `pthread_create' instead. */ -#ifndef __BIONIC__ static void *const __gthread_active_ptr - = __extension__ (void *) &__gthrw_(pthread_cancel); -#else - static void *const __gthread_active_ptr - = __extension__ (void *) &__gthrw_(pthread_create); -#endif + = __extension__ (void *) >HR_ACTIVE_PROXY; return __gthread_active_ptr != 0; }