diff mbox

Gthreads patch to disable static initializer macros

Message ID yddzkcqzqui.fsf@manam.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Feb. 10, 2012, 2:48 p.m. UTC
Jonathan Wakely <jwakely.gcc@gmail.com> writes:

> 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.)

Indeed, thanks for your hard work on this.  The following patch on top
of your previous one allows the 30_threads tests to PASS on Tru64 UNIX.

Bootstrapped without regressions on alpha-dec-osf5.1b, ok for mainline?

I've also noticed one feature of your patch that I don't like:
__GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of
pthread_{mutex,cond}_init_function instead of passing them on as all
other gthr-posix.h functions do.  This might lead to bad error handling,
and my previous patch (based on an older version of yours you had
attached to the PR) changed them to return int instead.  I suppose
changing this now is out of question, and this is left as 4.8 material.

	Rainer


2012-02-03  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	libstdc++-v3:
	PR libstdc++/51296
	* config/os/osf/ctype_base.h,
	config/os/osf/ctype_configure_char.cc,
	config/os/osf/ctype_inline.h, config/os/osf/error_constants.h:
	Copy from config/os/generic.
	* config/os/osf/os_defines.h: Likewise.
	(_GTHREAD_USE_MUTEX_INIT_FUNC, _GTHREAD_USE_COND_INIT_FUNC):
	Define.
	* configure.host <osf*>: Use os/osf for os_include_dir.

Comments

Jonathan Wakely Feb. 10, 2012, 6 p.m. UTC | #1
On 10 February 2012 14:48, Rainer Orth wrote:
> I've also noticed one feature of your patch that I don't like:
> __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of
> pthread_{mutex,cond}_init_function instead of passing them on as all
> other gthr-posix.h functions do.  This might lead to bad error handling,
> and my previous patch (based on an older version of yours you had
> attached to the PR) changed them to return int instead.  I suppose
> changing this now is out of question, and this is left as 4.8 material.

I didn't like that feature of the patch much either, but the signature
of the mutex_init function is specified in gthr.h:

     __GTHREAD_MUTEX_INIT_FUNCTION
            some systems can't initialize a mutex without a
        function call.  On such systems, define this to a
        function which looks like this:
          void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)

The recursive one says it should have the same signature, and I (maybe
wrongly) assumed the cond_init one should too.
Jonathan Wakely Feb. 10, 2012, 6:03 p.m. UTC | #2
On 10 February 2012 14:48, Rainer Orth wrote:
>
> Bootstrapped without regressions on alpha-dec-osf5.1b, ok for mainline?

Yes, this is OK, thanks for your help testing and getting all of this working.
Rainer Orth Feb. 10, 2012, 6:20 p.m. UTC | #3
Jon,

> On 10 February 2012 14:48, Rainer Orth wrote:
>> I've also noticed one feature of your patch that I don't like:
>> __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of
>> pthread_{mutex,cond}_init_function instead of passing them on as all
>> other gthr-posix.h functions do.  This might lead to bad error handling,
>> and my previous patch (based on an older version of yours you had
>> attached to the PR) changed them to return int instead.  I suppose
>> changing this now is out of question, and this is left as 4.8 material.
>
> I didn't like that feature of the patch much either, but the signature
> of the mutex_init function is specified in gthr.h:
>
>      __GTHREAD_MUTEX_INIT_FUNCTION
>             some systems can't initialize a mutex without a
>         function call.  On such systems, define this to a
>         function which looks like this:
>           void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
>
> The recursive one says it should have the same signature, and I (maybe

I don't see this, neither in gthr.h nor in gthr-posix.h.

> wrongly) assumed the cond_init one should too.

I don't think this is cast in stone: the gthr*.h headers aren't
installed in a public place and aren't considered an external interface
to the best of my knowledge, so it should be possible to change.

Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc,
libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in
libstdc++), so changing the 13 calls is doable.

	Rainer
Jonathan Wakely Feb. 10, 2012, 6:25 p.m. UTC | #4
On 10 February 2012 18:20, Rainer Orth wrote:
> Jon,
>
>> On 10 February 2012 14:48, Rainer Orth wrote:
>>> I've also noticed one feature of your patch that I don't like:
>>> __GTHREAD_{MUTEX,COND}_INIT_FUNCTION ignore the return values of
>>> pthread_{mutex,cond}_init_function instead of passing them on as all
>>> other gthr-posix.h functions do.  This might lead to bad error handling,
>>> and my previous patch (based on an older version of yours you had
>>> attached to the PR) changed them to return int instead.  I suppose
>>> changing this now is out of question, and this is left as 4.8 material.
>>
>> I didn't like that feature of the patch much either, but the signature
>> of the mutex_init function is specified in gthr.h:
>>
>>      __GTHREAD_MUTEX_INIT_FUNCTION
>>             some systems can't initialize a mutex without a
>>         function call.  On such systems, define this to a
>>         function which looks like this:
>>           void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
>>
>> The recursive one says it should have the same signature, and I (maybe
>
> I don't see this, neither in gthr.h nor in gthr-posix.h.

lines 54-62 in http://gcc.gnu.org/viewcvs/trunk/libgcc/gthr.h?view=markup

>> wrongly) assumed the cond_init one should too.
>
> I don't think this is cast in stone: the gthr*.h headers aren't
> installed in a public place and aren't considered an external interface
> to the best of my knowledge, so it should be possible to change.
>
> Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc,
> libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in
> libstdc++), so changing the 13 calls is doable.


Private ports which provide their own gthr header (I think they're
rare, but do exist) would need to modify their functions to return an
integer - but at least it would be a compile-time failure so they'd
know to change it.
Rainer Orth Feb. 10, 2012, 6:28 p.m. UTC | #5
Jonathan Wakely <jwakely.gcc@gmail.com> writes:

>>> I didn't like that feature of the patch much either, but the signature
>>> of the mutex_init function is specified in gthr.h:
>>>
>>>      __GTHREAD_MUTEX_INIT_FUNCTION
>>>             some systems can't initialize a mutex without a
>>>         function call.  On such systems, define this to a
>>>         function which looks like this:
>>>           void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
>>>
>>> The recursive one says it should have the same signature, and I (maybe
>>
>> I don't see this, neither in gthr.h nor in gthr-posix.h.
>
> lines 54-62 in http://gcc.gnu.org/viewcvs/trunk/libgcc/gthr.h?view=markup

I think that's overinterpreting the comment.
__gthread_recursive_mutex_init_function happily returns errors with no
indication it should be otherwise.

>>> wrongly) assumed the cond_init one should too.
>>
>> I don't think this is cast in stone: the gthr*.h headers aren't
>> installed in a public place and aren't considered an external interface
>> to the best of my knowledge, so it should be possible to change.
>>
>> Currently, __GTHREAD_MUTEX_INIT_FUNCTION is only used in libgcc,
>> libgfortran, and libstdc++ (and __GTHREAD_COND_INIT_FUNCTION only in
>> libstdc++), so changing the 13 calls is doable.
>
> Private ports which provide their own gthr header (I think they're
> rare, but do exist) would need to modify their functions to return an
> integer - but at least it would be a compile-time failure so they'd
> know to change it.

Right, and that's the cost of keeping a port out of the FSF tree.

	Rainer
diff mbox

Patch

# HG changeset patch
# Parent badf67959a1e06e2f8c98df3797878e3123aad8e
Use __GTHREAD_MUTEX_INIT_FUNCTION on Tru64 UNIX (PR libstdc++/51296)

diff --git a/libstdc++-v3/config/os/generic/ctype_base.h b/libstdc++-v3/config/os/osf/ctype_base.h
copy from libstdc++-v3/config/os/generic/ctype_base.h
copy to libstdc++-v3/config/os/osf/ctype_base.h
diff --git a/libstdc++-v3/config/os/generic/ctype_configure_char.cc b/libstdc++-v3/config/os/osf/ctype_configure_char.cc
copy from libstdc++-v3/config/os/generic/ctype_configure_char.cc
copy to libstdc++-v3/config/os/osf/ctype_configure_char.cc
diff --git a/libstdc++-v3/config/os/generic/ctype_inline.h b/libstdc++-v3/config/os/osf/ctype_inline.h
copy from libstdc++-v3/config/os/generic/ctype_inline.h
copy to libstdc++-v3/config/os/osf/ctype_inline.h
diff --git a/libstdc++-v3/config/os/generic/error_constants.h b/libstdc++-v3/config/os/osf/error_constants.h
copy from libstdc++-v3/config/os/generic/error_constants.h
copy to libstdc++-v3/config/os/osf/error_constants.h
diff --git a/libstdc++-v3/config/os/generic/os_defines.h b/libstdc++-v3/config/os/osf/os_defines.h
copy from libstdc++-v3/config/os/generic/os_defines.h
copy to libstdc++-v3/config/os/osf/os_defines.h
--- a/libstdc++-v3/config/os/generic/os_defines.h
+++ b/libstdc++-v3/config/os/osf/os_defines.h
@@ -1,6 +1,6 @@ 
-// Specific definitions for generic platforms  -*- C++ -*-
+// Specific definitions for Tru64 UNIX  -*- C++ -*-
 
-// Copyright (C) 2000, 2009, 2010 Free Software Foundation, Inc.
+// Copyright (C) 2000, 2009, 2010, 2012 Free Software Foundation, Inc.
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
 // software; you can redistribute it and/or modify it under the
@@ -33,4 +33,9 @@ 
 // System-specific #define, typedefs, corrections, etc, go here.  This
 // file will come before all others.
 
+// Tru64 UNIX requires using pthread_mutex_init()/pthread_cond_init() to
+// initialized non-statically allocated mutexes/condvars.
+#define _GTHREAD_USE_MUTEX_INIT_FUNC
+#define _GTHREAD_USE_COND_INIT_FUNC
+
 #endif
diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
--- a/libstdc++-v3/configure.host
+++ b/libstdc++-v3/configure.host
@@ -280,7 +280,7 @@  case "${host_os}" in
     os_include_dir="os/bsd/netbsd"
     ;;
   osf*)
-    os_include_dir="os/generic"
+    os_include_dir="os/osf"
     # libstdc++.so relies on emutls on Tru64 UNIX, which only works with the
     # real functions implemented in libpthread.so, not with the dummies in
     # libgcc, so always pass -lpthread.