diff mbox

[v3] libstdc++/49745 (review required for the gthr-posix.h changes)

Message ID 4E2036FA.8020901@oracle.com
State New
Headers show

Commit Message

Paolo Carlini July 15, 2011, 12:47 p.m. UTC 
Hi,

this is what I did in terms of implementing Jon's and Jakub's 
suggestions: many thanks to both of you!

The patch should be in general quite conservative and safe, in 
particular, the gthr-posix.h changes, which I cannot approve myself, 
touch bits only used by libstdc++-v3 + the macro avoiding the 
unconditional inclusion of <unistd.h>, which, according to Jakub's 
analysis, should be required only by objc.

I built and tested c++ and its lib and built all languages.  Ok?

Thanks,
Paolo.

///////////////////////
/gcc
2011-07-15  Paolo Carlini  <paolo.carlini@oracle.com>
	    Jakub Jelinek  <jakub@redhat.com>
	    Jonathan Wakely  <jwakely.gcc@gmail.com>

	PR libstdc++/49745
	* gthr-posix.h: Do not include <unistd.h> unconditionally; use
	_GTHREADS_ASSUME_POSIX_TIMEOUTS instead of _POSIX_TIMEOUTS.

/libstdc++-v3
2011-07-15  Paolo Carlini  <paolo.carlini@oracle.com>
	    Jakub Jelinek  <jakub@redhat.com>

	PR libstdc++/49745
	* acinclude.m4 ([GLIBCXX_CHECK_GTHREADS]): Check separately for
	_POSIX_TIMEOUTS and define _GTHREADS_ASSUME_POSIX_TIMEOUTS.
	* libstdc++-v3/libsupc++/guard.cc: Include <unistd.h>.
	* testsuite/17_intro/headers/c++1998/49745.cc: New.
	* configure: Regenerate.
	* config.h.in: Likewise.

Comments

Jason Merrill July 15, 2011, 1:36 p.m. UTC | #1 
On 07/15/2011 08:47 AM, Paolo Carlini wrote:
> The patch should be in general quite conservative and safe, in
> particular, the gthr-posix.h changes, which I cannot approve myself,
> touch bits only used by libstdc++-v3 + the macro avoiding the
> unconditional inclusion of <unistd.h>, which, according to Jakub's
> analysis, should be required only by objc.

I'm a little uncomfortable with defining a macro in libstdc++ to be used 
by the gthr files in gcc.  I would lean more toward a gthr-posix-conf.h 
generated by GCC configure.

Jason
Paolo Carlini July 15, 2011, 1:44 p.m. UTC | #2 
On 07/15/2011 03:36 PM, Jason Merrill wrote:
> I'm a little uncomfortable with defining a macro in libstdc++ to be 
> used by the gthr files in gcc.  I would lean more toward a 
> gthr-posix-conf.h generated by GCC configure.
For sure, I gonna need some help for that... or maybe Jakub can do it?

Paolo.
Jakub Jelinek July 15, 2011, 1:45 p.m. UTC | #3 
On Fri, Jul 15, 2011 at 09:36:48AM -0400, Jason Merrill wrote:
> On 07/15/2011 08:47 AM, Paolo Carlini wrote:
> >The patch should be in general quite conservative and safe, in
> >particular, the gthr-posix.h changes, which I cannot approve myself,
> >touch bits only used by libstdc++-v3 + the macro avoiding the
> >unconditional inclusion of <unistd.h>, which, according to Jakub's
> >analysis, should be required only by objc.
> 
> I'm a little uncomfortable with defining a macro in libstdc++ to be
> used by the gthr files in gcc.  I would lean more toward a
> gthr-posix-conf.h generated by GCC configure.

gthr-posix.h already uses other macros defined by other library
headers, like _LIBOBJC.  gthr-posix-conf.h looks like an overkill to me,
but if e.g. libstdc++ headers defined a 0/1 macro always
(_GTHREAD_USE_TIMEDLOCK 0 would mean don't define it, _GTHREAD_USE_TIMEDLOCK 1
would mean you can safely assume timedlock is available, and not presence
of that macro would lead to inclusion of unistd.h and deciding itself
based on _POSIX_TIMEOUTS:

#ifndef _GTHREAD_USE_TIMEDLOCK
#include <unistd.h>
#ifdef _POSIX_TIMEOUTS
#if _POSIX_TIMEOUTS >= 0
#define _GTHREAD_USE_TIMEDLOCK 1
#else
#define _GTHREAD_USE_TIMEDLOCK 0
#else
#define _GTHREAD_USE_TIMEDLOCK 0
#endif
and use

#if _GTHREAD_USE_TIMEDLOCK
...
#endif

then when gthr.h is used outside of libstdc++ it wouldn't depend on
libstdc++ configury.

	Jakub
diff mbox

Patch

Index: libstdc++-v3/libsupc++/guard.cc
===================================================================
--- libstdc++-v3/libsupc++/guard.cc	(revision 176310)
+++ libstdc++-v3/libsupc++/guard.cc	(working copy)
@@ -35,6 +35,7 @@ 
     && defined(_GLIBCXX_ATOMIC_BUILTINS_4) && defined(_GLIBCXX_HAVE_LINUX_FUTEX)
 # include <climits>
 # include <syscall.h>
+# include <unistd.h>
 # define _GLIBCXX_USE_FUTEX
 # define _GLIBCXX_FUTEX_WAIT 0
 # define _GLIBCXX_FUTEX_WAKE 1
Index: libstdc++-v3/testsuite/17_intro/headers/c++1998/49745.cc
===================================================================
--- libstdc++-v3/testsuite/17_intro/headers/c++1998/49745.cc	(revision 0)
+++ libstdc++-v3/testsuite/17_intro/headers/c++1998/49745.cc	(revision 0)
@@ -0,0 +1,22 @@ 
+// { dg-do compile }
+
+// Copyright (C) 2011 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
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// libstdc++/49745
+#include <iostream>
+int truncate = 0;
Index: libstdc++-v3/acinclude.m4
===================================================================
--- libstdc++-v3/acinclude.m4	(revision 176310)
+++ libstdc++-v3/acinclude.m4	(working copy)
@@ -3155,6 +3155,22 @@ 
   ac_save_CXXFLAGS="$CXXFLAGS"
   CXXFLAGS="$CXXFLAGS -fno-exceptions -I${toplevel_srcdir}/gcc"
 
+  AC_MSG_CHECKING([for _POSIX_TIMEOUTS >= 0 in <unistd.h>])
+
+  AC_TRY_COMPILE([#include <unistd.h>],
+    [
+      #if !defined(_POSIX_TIMEOUTS) || _POSIX_TIMEOUTS < 0
+      #error
+      #endif
+    ], [ac_assume_posix_timeouts=yes], [ac_assume_posix_timeouts=no])
+
+  AC_MSG_RESULT([$ac_assume_posix_timeouts])
+
+  if test x"$ac_assume_posix_timeouts" = x"yes"; then
+    AC_DEFINE(_GTHREADS_ASSUME_POSIX_TIMEOUTS, 1,
+	      [Define if _POSIX_TIMEOUT is defined >= 0 by <unistd.h>.])
+  fi
+
   target_thread_file=`$CXX -v 2>&1 | sed -n 's/^Thread model: //p'`
   case $target_thread_file in
     posix)
@@ -3163,7 +3179,10 @@ 
 
   AC_MSG_CHECKING([for gthreads library])
 
-  AC_TRY_COMPILE([#include "gthr.h"],
+  AC_TRY_COMPILE([
+                  #include "gthr.h"
+		  #include <unistd.h>
+                 ],
     [
       #ifndef __GTHREADS_CXX0X
       #error
Index: gcc/gthr-posix.h
===================================================================
--- gcc/gthr-posix.h	(revision 176310)
+++ gcc/gthr-posix.h	(working copy)
@@ -1,7 +1,7 @@ 
 /* Threads compatibility routines for libgcc2 and libobjc.  */
 /* Compile this one with gcc.  */
 /* Copyright (C) 1997, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007,
-   2008, 2009, 2010 Free Software Foundation, Inc.
+   2008, 2009, 2010, 2011 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -39,7 +39,10 @@ 
 #endif
 
 #include <pthread.h>
+
+#if defined(_LIBOBJC) || defined(_LIBOBJC_WEAK)
 #include <unistd.h>
+#endif
 
 typedef pthread_t __gthread_t;
 typedef pthread_key_t __gthread_key_t;
@@ -100,11 +103,9 @@ 
 
 __gthrw3(pthread_mutex_lock)
 __gthrw3(pthread_mutex_trylock)
-#ifdef _POSIX_TIMEOUTS
-#if _POSIX_TIMEOUTS >= 0
+#ifdef _GTHREADS_ASSUME_POSIX_TIMEOUTS
 __gthrw3(pthread_mutex_timedlock)
 #endif
-#endif /* _POSIX_TIMEOUTS */
 __gthrw3(pthread_mutex_unlock)
 __gthrw3(pthread_mutex_init)
 __gthrw3(pthread_mutex_destroy)
@@ -131,11 +132,9 @@ 
 
 __gthrw(pthread_mutex_lock)
 __gthrw(pthread_mutex_trylock)
-#ifdef _POSIX_TIMEOUTS
-#if _POSIX_TIMEOUTS >= 0
+#ifdef _GTHREADS_ASSUME_POSIX_TIMEOUTS
 __gthrw(pthread_mutex_timedlock)
 #endif
-#endif /* _POSIX_TIMEOUTS */
 __gthrw(pthread_mutex_unlock)
 __gthrw(pthread_mutex_init)
 __gthrw(pthread_mutex_destroy)
@@ -753,8 +752,7 @@ 
     return 0;
 }
 
-#ifdef _POSIX_TIMEOUTS
-#if _POSIX_TIMEOUTS >= 0
+#ifdef _GTHREADS_ASSUME_POSIX_TIMEOUTS
 static inline int
 __gthread_mutex_timedlock (__gthread_mutex_t *__mutex,
 			   const __gthread_time_t *__abs_timeout)
@@ -765,7 +763,6 @@ 
     return 0;
 }
 #endif
-#endif
 
 static inline int
 __gthread_mutex_unlock (__gthread_mutex_t *__mutex)
@@ -811,8 +808,7 @@ 
   return __gthread_mutex_trylock (__mutex);
 }
 
-#ifdef _POSIX_TIMEOUTS
-#if _POSIX_TIMEOUTS >= 0
+#ifdef _GTHREADS_ASSUME_POSIX_TIMEOUTS
 static inline int
 __gthread_recursive_mutex_timedlock (__gthread_recursive_mutex_t *__mutex,
 				     const __gthread_time_t *__abs_timeout)
@@ -820,7 +816,6 @@ 
   return __gthread_mutex_timedlock (__mutex, __abs_timeout);
 }
 #endif
-#endif
 
 static inline int
 __gthread_recursive_mutex_unlock (__gthread_recursive_mutex_t *__mutex)