diff mbox

Gthreads patch to disable static initializer macros

Message ID CAH6eHdT1cf3UpwsQL_y0eGfSgMrG+7Y4idwz9nT9JhO18prpGg@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Feb. 5, 2012, 8:26 p.m. UTC
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?

Comments

Jack Howarth Feb. 5, 2012, 11:53 p.m. UTC | #1
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
Jonathan Wakely Feb. 6, 2012, 12:09 a.m. UTC | #2
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.
Jakub Jelinek Feb. 6, 2012, 6:40 a.m. UTC | #3
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
Mike Stump Feb. 6, 2012, 7:24 p.m. UTC | #4
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.
Jonathan Wakely Feb. 7, 2012, 10:55 a.m. UTC | #5
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 mbox

Patch

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