Message ID | CAH6eHdT1cf3UpwsQL_y0eGfSgMrG+7Y4idwz9nT9JhO18prpGg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Sun, Feb 05, 2012 at 08:26:22PM +0000, Jonathan Wakely wrote: > PRs libstdc++/51296 and libstdc++/51906 are both caused by problems > with the Pthreads static initializer macros such as > PTHREAD_MUTEX_INITIALIZER. > > On Tru64 PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER can > only be used for statically-initialized variables, as allowed by POSIX > currently, but http://austingroupbugs.net/view.php?id=70#c127 removes > that limitation for the next POSIX standard. C++11 needs to use those > macros for variables with automatic and dynamic scope, because the > init functions can't be used in constexpr constructors. > > On Mac OS X 10.7 the PTHREAD_RECURSIVE_MUTEX_INITIALIZER is buggy. > This has shown up now because C++11 threads weren't enabled on darwin > before 4.7 > > My suggestion is to modify gthr-posix.h with the attached patch, so > that target maintainers can define e.g. > _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC to force use of the init > function instead of the __GTHREAD_RECURSIVE_MUTEX_INIT initializer. > The patch includes a change to do that for darwin. > > This has been testing on darwin, if Rainer tests it successfully on > Tru64 will the gthr-posix.h change be acceptable? There are no unexpected libstdc++ testsuite regressions on x86_64-apple-darwin11 with this patch applied to current gcc trunk. > diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h > index 46054f6..45b15a8 100644 > --- a/libgcc/gthr-posix.h > +++ b/libgcc/gthr-posix.h > @@ -74,6 +74,20 @@ typedef struct timespec __gthread_time_t; > #define __GTHREAD_COND_INIT PTHREAD_COND_INITIALIZER > #define __GTHREAD_TIME_INIT {0,0} > > +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC > +# undef __GTHREAD_MUTEX_INIT > +# define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function > +#endif > +#ifdef _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC > +# undef __GTHREAD_RECURSIVE_MUTEX_INIT > +# undef __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION > +# define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION __gthread_recursive_mutex_init_function > +#endif > +#ifdef _GTHREAD_USE_COND_INIT_FUNC > +# undef __GTHREAD_COND_INIT > +# define __GTHREAD_COND_INIT_FUNCTION __gthread_cond_init_function > +#endif > + > #if SUPPORTS_WEAK && GTHREAD_USE_WEAK > # ifndef __gthrw_pragma > # define __gthrw_pragma(pragma) > @@ -730,6 +744,15 @@ __gthread_setspecific (__gthread_key_t __key, const void *__ptr) > return __gthrw_(pthread_setspecific) (__key, __ptr); > } > > +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC > +static inline void > +__gthread_mutex_init_function (__gthread_mutex_t *__mutex) > +{ > + if (__gthread_active_p ()) > + __gthrw_(pthread_mutex_init) (__mutex, NULL); > +} > +#endif > + > static inline int > __gthread_mutex_destroy (__gthread_mutex_t *__mutex) > { > @@ -778,7 +801,8 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) > return 0; > } > > -#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP > +#if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ > + || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) > static inline int > __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) > { > @@ -828,6 +852,15 @@ __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) > return __gthread_mutex_unlock (__mutex); > } > > +#ifdef _GTHREAD_USE_COND_INIT_FUNC > +static inline void > +__gthread_cond_init_function (__gthread_cond_t *__cond) > +{ > + if (__gthread_active_p ()) > + __gthrw_(pthread_cond_init) (__cond, NULL); > +} > +#endif > + > static inline int > __gthread_cond_broadcast (__gthread_cond_t *__cond) > { > diff --git a/libstdc++-v3/config/os/bsd/darwin/os_defines.h b/libstdc++-v3/config/os/bsd/darwin/os_defines.h > index ccefeaf..421478d 100644 > --- a/libstdc++-v3/config/os/bsd/darwin/os_defines.h > +++ b/libstdc++-v3/config/os/bsd/darwin/os_defines.h > @@ -39,4 +39,7 @@ > // -flat_namespace to work around the way that it doesn't. > #define _GLIBCXX_WEAK_DEFINITION __attribute__ ((weak)) > > +// Static initializer macro is buggy in darwin, see libstdc++/51906 > +#define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC > + > #endif
On 5 February 2012 23:53, Jack Howarth wrote: > > There are no unexpected libstdc++ testsuite regressions on x86_64-apple-darwin11 with this > patch applied to current gcc trunk. Thanks, Jack. If I can't get approval for the change I'll workaround it in libstdc++, but IMHO it would be cleaner to change gthr-posix.h to not define macros that can't be used safely.
On Sun, Feb 05, 2012 at 08:26:22PM +0000, Jonathan Wakely wrote: > This has been testing on darwin, if Rainer tests it successfully on > Tru64 will the gthr-posix.h change be acceptable? Ok with a suitable ChangeLog entry. > diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h > index 46054f6..45b15a8 100644 > --- a/libgcc/gthr-posix.h > +++ b/libgcc/gthr-posix.h > @@ -74,6 +74,20 @@ typedef struct timespec __gthread_time_t; > #define __GTHREAD_COND_INIT PTHREAD_COND_INITIALIZER > #define __GTHREAD_TIME_INIT {0,0} > > +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC > +# undef __GTHREAD_MUTEX_INIT > +# define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function > +#endif > +#ifdef _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC > +# undef __GTHREAD_RECURSIVE_MUTEX_INIT > +# undef __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION > +# define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION __gthread_recursive_mutex_init_function > +#endif > +#ifdef _GTHREAD_USE_COND_INIT_FUNC > +# undef __GTHREAD_COND_INIT > +# define __GTHREAD_COND_INIT_FUNCTION __gthread_cond_init_function > +#endif > + > #if SUPPORTS_WEAK && GTHREAD_USE_WEAK > # ifndef __gthrw_pragma > # define __gthrw_pragma(pragma) > @@ -730,6 +744,15 @@ __gthread_setspecific (__gthread_key_t __key, const void *__ptr) > return __gthrw_(pthread_setspecific) (__key, __ptr); > } > > +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC > +static inline void > +__gthread_mutex_init_function (__gthread_mutex_t *__mutex) > +{ > + if (__gthread_active_p ()) > + __gthrw_(pthread_mutex_init) (__mutex, NULL); > +} > +#endif > + > static inline int > __gthread_mutex_destroy (__gthread_mutex_t *__mutex) > { > @@ -778,7 +801,8 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) > return 0; > } > > -#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP > +#if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ > + || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) > static inline int > __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) > { > @@ -828,6 +852,15 @@ __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) > return __gthread_mutex_unlock (__mutex); > } > > +#ifdef _GTHREAD_USE_COND_INIT_FUNC > +static inline void > +__gthread_cond_init_function (__gthread_cond_t *__cond) > +{ > + if (__gthread_active_p ()) > + __gthrw_(pthread_cond_init) (__cond, NULL); > +} > +#endif > + > static inline int > __gthread_cond_broadcast (__gthread_cond_t *__cond) > { > diff --git a/libstdc++-v3/config/os/bsd/darwin/os_defines.h b/libstdc++-v3/config/os/bsd/darwin/os_defines.h > index ccefeaf..421478d 100644 > --- a/libstdc++-v3/config/os/bsd/darwin/os_defines.h > +++ b/libstdc++-v3/config/os/bsd/darwin/os_defines.h > @@ -39,4 +39,7 @@ > // -flat_namespace to work around the way that it doesn't. > #define _GLIBCXX_WEAK_DEFINITION __attribute__ ((weak)) > > +// Static initializer macro is buggy in darwin, see libstdc++/51906 > +#define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC > + > #endif Jakub
On Feb 5, 2012, at 12:26 PM, Jonathan Wakely wrote: > PRs libstdc++/51296 and libstdc++/51906 are both caused by problems > with the Pthreads static initializer macros such as > PTHREAD_MUTEX_INITIALIZER. > On Mac OS X 10.7 the PTHREAD_RECURSIVE_MUTEX_INITIALIZER is buggy. Thanks for all you work on this.
On 6 February 2012 19:24, Mike Stump wrote: > On Feb 5, 2012, at 12:26 PM, Jonathan Wakely wrote: >> PRs libstdc++/51296 and libstdc++/51906 are both caused by problems >> with the Pthreads static initializer macros such as >> PTHREAD_MUTEX_INITIALIZER. > >> On Mac OS X 10.7 the PTHREAD_RECURSIVE_MUTEX_INITIALIZER is buggy. > > Thanks for all you work on this. Well I broke it so I had to fix it ;) I'm pleased to say we should now have an almost complete C++11 thread implementation for most POSIX platforms, with hundreds of existing libstdc++ tests moving from UNSUPPORTED to PASS on some secondary platforms (aix and darwin, maybe hpux too.)
diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h index 46054f6..45b15a8 100644 --- a/libgcc/gthr-posix.h +++ b/libgcc/gthr-posix.h @@ -74,6 +74,20 @@ typedef struct timespec __gthread_time_t; #define __GTHREAD_COND_INIT PTHREAD_COND_INITIALIZER #define __GTHREAD_TIME_INIT {0,0} +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC +# undef __GTHREAD_MUTEX_INIT +# define __GTHREAD_MUTEX_INIT_FUNCTION __gthread_mutex_init_function +#endif +#ifdef _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC +# undef __GTHREAD_RECURSIVE_MUTEX_INIT +# undef __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION +# define __GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION __gthread_recursive_mutex_init_function +#endif +#ifdef _GTHREAD_USE_COND_INIT_FUNC +# undef __GTHREAD_COND_INIT +# define __GTHREAD_COND_INIT_FUNCTION __gthread_cond_init_function +#endif + #if SUPPORTS_WEAK && GTHREAD_USE_WEAK # ifndef __gthrw_pragma # define __gthrw_pragma(pragma) @@ -730,6 +744,15 @@ __gthread_setspecific (__gthread_key_t __key, const void *__ptr) return __gthrw_(pthread_setspecific) (__key, __ptr); } +#ifdef _GTHREAD_USE_MUTEX_INIT_FUNC +static inline void +__gthread_mutex_init_function (__gthread_mutex_t *__mutex) +{ + if (__gthread_active_p ()) + __gthrw_(pthread_mutex_init) (__mutex, NULL); +} +#endif + static inline int __gthread_mutex_destroy (__gthread_mutex_t *__mutex) { @@ -778,7 +801,8 @@ __gthread_mutex_unlock (__gthread_mutex_t *__mutex) return 0; } -#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP +#if !defined( PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) \ + || defined(_GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC) static inline int __gthread_recursive_mutex_init_function (__gthread_recursive_mutex_t *__mutex) { @@ -828,6 +852,15 @@ __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex) return __gthread_mutex_unlock (__mutex); } +#ifdef _GTHREAD_USE_COND_INIT_FUNC +static inline void +__gthread_cond_init_function (__gthread_cond_t *__cond) +{ + if (__gthread_active_p ()) + __gthrw_(pthread_cond_init) (__cond, NULL); +} +#endif + static inline int __gthread_cond_broadcast (__gthread_cond_t *__cond) { diff --git a/libstdc++-v3/config/os/bsd/darwin/os_defines.h b/libstdc++-v3/config/os/bsd/darwin/os_defines.h index ccefeaf..421478d 100644 --- a/libstdc++-v3/config/os/bsd/darwin/os_defines.h +++ b/libstdc++-v3/config/os/bsd/darwin/os_defines.h @@ -39,4 +39,7 @@ // -flat_namespace to work around the way that it doesn't. #define _GLIBCXX_WEAK_DEFINITION __attribute__ ((weak)) +// Static initializer macro is buggy in darwin, see libstdc++/51906 +#define _GTHREAD_USE_RECURSIVE_MUTEX_INIT_FUNC + #endif