Patchwork [v3] Handle different versions of Solaris 8 <iso/math_iso.h>, <iso/stdlib_iso.h>

login
register
mail settings
Submitter Rainer Orth
Date Aug. 26, 2011, 12:46 p.m.
Message ID <yddmxewpcgu.fsf@manam.CeBiTec.Uni-Bielefeld.DE>
Download mbox | patch
Permalink /patch/111767/
State New
Headers show

Comments

Rainer Orth - Aug. 26, 2011, 12:46 p.m.
All my testing of the __cplusplus 199711L patches had been on Solaris
8+/x86.  During last weekend's bootstrap on the whole range of systems
(Solaris 8 to 11, SPARC and x86), it turned out that there are possible
variations of <iso/math_iso.h> and <iso/stdlib_iso.h> between Solaris 8
FCS and patches, so we cannot statically configure which overloads are
present, but need autoconf checks for that.

The situation is as follows:

* Solaris 8 FCS shipped rev. 1.1 of <iso/math_iso.h> which only had
  double std::abs(double).  Later, in patches 111721-04 (SPARC) and
  112757-01 (x86), rev. 1.3 whas shipped that has everything that's also
  present in Solaris 9 and up.

* Similarly, Solaris 8 FCS has rev. 1.1 of <iso/stdlib_iso.h> without
  any overloads.  Patches 109607-02 (SPARC) and 109608-02 (x86) added
  long std::abs(long) and ldiv_t div(lng, long) in rev. 1.3.

Since <bits/os_defines.h> is included before configure results,
configure needs to define the affected
__CORRECT_ISO_CPP_MATH_H_PROTO[12] and __CORRECT_ISO_CPP_STDLIB_H_PROTO
directly.  The following patch does just that.

Bootstrapped without regressions on x86_64-unknown-linux-gnu and
i386-pc-solaris2.11, bootstraps on i386-pc-solaris2.8 (with the old
rev. 1.1 headers) and sparc-sun-solaris2.8 (with the the rev. 1.3
headers) are still in progress, but I've verified that the
__CORRECT_ISO_CPP_* macros are all defined correctly..  Since errors in
previous versions of the patch manifested themselves in build failures
immediately, I'm pretty certain that there are no errors.

Ok for mainline if bootstraps pass?

Thanks.
        Rainer


2011-08-25  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* acinclude.m4 (GLIBCXX_CHECK_MATH_PROTO)
	(GLIBCXX_CHECK_STDLIB_PROTO): New tests.
	* configure.ac (GLIBCXX_CHECK_MATH_PROTO)
	(GLIBCXX_CHECK_STDLIB_PROTO): Call them.
	* configure: Regenerate.
	* config.h.in: Regenerate.
	* config/os/solaris/solaris2.8/os_defines.h
	(__CORRECT_ISO_CPP_MATH_H_PROTO2): Don't define.
	* config/os/solaris/solaris2.9: Remove.
	* configure.host (solaris2.8): Merge with ...
	(solaris2.9, solaris2.1[0-9]): ... this.
	Always use os/solaris/solaris2.8.
Paolo Carlini - Aug. 26, 2011, 12:57 p.m.
Hi,
> Ok for mainline if bootstraps pass?
Not a comment strictly about this patch, but why we have things like #if 
__cplusplus >= 199711L anywhere? For sure the library is not supposed to 
be used together with old C++ front-ends.

Paolo.
Rainer Orth - Aug. 26, 2011, 12:59 p.m.
Hi Paolo,

>> Ok for mainline if bootstraps pass?
> Not a comment strictly about this patch, but why we have things like #if
> __cplusplus >= 199711L anywhere? For sure the library is not supposed to be
> used together with old C++ front-ends.

I thought about this myself, but at least the overloads are only present
with __cplusplus >= 199711L.  I think it's best to match this to avoid
strange problems if a user plays strange games with __cplusplus.

	Rainer
Paolo Carlini - Aug. 26, 2011, 1:09 p.m.
On 8/26/11 2:59 PM, Rainer Orth wrote:
> Hi Paolo,
>
>>> Ok for mainline if bootstraps pass?
>> Not a comment strictly about this patch, but why we have things like #if
>> __cplusplus>= 199711L anywhere? For sure the library is not supposed to be
>> used together with old C++ front-ends.
> I thought about this myself, but at least the overloads are only present
> with __cplusplus>= 199711L.
I don't understand: isn't __cplusplus now *always* >= 199711L? Or you 
want to protect vs the user undefining __cplusplus and then defining it 
to a different value?!? I don't have the Standard at hand (in theory I'm 
in vacation ;), maybe Marc can help, but I don't think it's legal, is it?

Paolo.
Rainer Orth - Aug. 26, 2011, 2:07 p.m.
Paolo,

>>>> Ok for mainline if bootstraps pass?
>>> Not a comment strictly about this patch, but why we have things like #if
>>> __cplusplus>= 199711L anywhere? For sure the library is not supposed to be
>>> used together with old C++ front-ends.
>> I thought about this myself, but at least the overloads are only present
>> with __cplusplus>= 199711L.
> I don't understand: isn't __cplusplus now *always* >= 199711L? Or you want
> to protect vs the user undefining __cplusplus and then defining it to a
> different value?!? I don't have the Standard at hand (in theory I'm in

exactly: just g++ -D__cplusplus=1 or something.

> vacation ;), maybe Marc can help, but I don't think it's legal, is it?

No idea.
	Rainer
Paolo Carlini - Aug. 26, 2011, 3:13 p.m.
Hi,

> exactly: just g++ -D__cplusplus=1 or something.

Irrespective of what the Standard strictly says, I think the latter would only make sense if it would allow the user to return, consistently, to the pre-4.7 behavior, for compatibility reasons or something. Is it the case? Is the above enough for that? Or some of the changes which went in are effective anyway even if __cplusplus is reverted by hand to 1? I think this is the question deciding what we really want to do.

Paolo
Rainer Orth - Aug. 26, 2011, 3:18 p.m.
Hi Paolo,

>> exactly: just g++ -D__cplusplus=1 or something.
>
> Irrespective of what the Standard strictly says, I think the latter would only make sense if it would allow the user to return, consistently, to the pre-4.7 behavior, for compatibility reasons or something. Is it the case? Is the above enough for that? Or some of the changes which went in are effective anyway even if __cplusplus is reverted by hand to 1? I think this is the question deciding what we really want to do.

I'm pretty sure this is the case for Solaris.  The other changes we've
made to support __cplusplus 199711L were no-ops without the last one to
change __cplusplus from 1 to the C++ 98 value.  So, redefining
__cplusplus to 1 should return us back to the old status.

	Rainer
Paolo Carlini - Aug. 26, 2011, 3:29 p.m.
Hi,

> I'm pretty sure this is the case for Solaris.  The other changes we've
> made to support __cplusplus 199711L were no-ops without the last one to change __cplusplus from 1 to the C++ 98 value.  So, redefining
> __cplusplus to 1 should return us back to the old status.

I see, then I think the patch is Ok. Since you are so well positioned to test on Solaris machines, I would recommend running the library testsuite with -D__cplusplus=1 added to CXXFLAGS, as a final check.

Paolo
Rainer Orth - Aug. 26, 2011, 5:01 p.m.
Hi Paolo,

>> I'm pretty sure this is the case for Solaris.  The other changes we've
>> made to support __cplusplus 199711L were no-ops without the last one to change __cplusplus from 1 to the C++ 98 value.  So, redefining
>> __cplusplus to 1 should return us back to the old status.
>
> I see, then I think the patch is Ok. Since you are so well positioned to test on Solaris machines, I would recommend running the library testsuite with -D__cplusplus=1 added to CXXFLAGS, as a final check.

I've just done that on i386-pc-solaris2.11, but had to use -U__cplusplus
-D__cplusplus=1 to avoid the redefinition warning.  This way, I get only
a single regression:

-PASS: abi/header_cxxabi.c (test for excess errors)
+FAIL: abi/header_cxxabi.c (test for excess errors)

FAIL: abi/header_cxxabi.c (test for excess errors)
Excess errors:
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:167:1: error: unknown type name 'namespace'
/var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:168:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token

which is pretty obvious given that this test is supposed to be compiled
as C :-)

I guess the patch is ok now?

Thanks.
        Rainer
Jonathan Wakely - Aug. 26, 2011, 5:13 p.m.
On 26 August 2011 14:09, Paolo Carlini wrote:
> On 8/26/11 2:59 PM, Rainer Orth wrote:
>>
>> Hi Paolo,
>>
>>>> Ok for mainline if bootstraps pass?
>>>
>>> Not a comment strictly about this patch, but why we have things like #if
>>> __cplusplus>= 199711L anywhere? For sure the library is not supposed to
>>> be
>>> used together with old C++ front-ends.
>>
>> I thought about this myself, but at least the overloads are only present
>> with __cplusplus>= 199711L.
>
> I don't understand: isn't __cplusplus now *always* >= 199711L? Or you want
> to protect vs the user undefining __cplusplus and then defining it to a
> different value?!? I don't have the Standard at hand (in theory I'm in
> vacation ;), maybe Marc can help, but I don't think it's legal, is it?

[cpp.predefined]/3:

"If any of the pre-defined macro names in this subclause, or the
identifier defined, is the subject of a #define or a #undef
preprocessing directive, the behavior is undefined."
Jonathan Wakely - Aug. 26, 2011, 5:23 p.m.
On 26 August 2011 18:13, Jonathan Wakely wrote:
> On 26 August 2011 14:09, Paolo Carlini wrote:
>> On 8/26/11 2:59 PM, Rainer Orth wrote:
>>>
>>> Hi Paolo,
>>>
>>>>> Ok for mainline if bootstraps pass?
>>>>
>>>> Not a comment strictly about this patch, but why we have things like #if
>>>> __cplusplus>= 199711L anywhere? For sure the library is not supposed to
>>>> be
>>>> used together with old C++ front-ends.
>>>
>>> I thought about this myself, but at least the overloads are only present
>>> with __cplusplus>= 199711L.
>>
>> I don't understand: isn't __cplusplus now *always* >= 199711L? Or you want
>> to protect vs the user undefining __cplusplus and then defining it to a
>> different value?!? I don't have the Standard at hand (in theory I'm in
>> vacation ;), maybe Marc can help, but I don't think it's legal, is it?
>
> [cpp.predefined]/3:
>
> "If any of the pre-defined macro names in this subclause, or the
> identifier defined, is the subject of a #define or a #undef
> preprocessing directive, the behavior is undefined."
>

More specifically, __cplusplus is ***NOT*** a feature-test macro like
_POSIX_SOURCE that can be set by users to request different language
standards.

Setting __cplusplus will have no effect on the front-end, but might
confuse the library (or other third-party headers) just as using
-D__GXX_EXPERIMENTAL_CXX0X__ without -std=g++0x will cause big
problems, because the front-end will be using -std=c++98 mode but the
library will think C++0x support is enabled.  Doing this will cause
pain.

If there is ***any*** maintenance overhead involved in "supporting"
users who try to redefine __cplusplus then I think it's a mistake.
I'm certainly not going to think of the effects on those users when I
make changes to the library.
Paolo Carlini - Aug. 27, 2011, 10:33 a.m.
Hi,

> -PASS: abi/header_cxxabi.c (test for excess errors)
> +FAIL: abi/header_cxxabi.c (test for excess errors)
> 
> FAIL: abi/header_cxxabi.c (test for excess errors)
> Excess errors:
> /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:167:1: error: unknown type name 'namespace'
> /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:168:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
> 
> which is pretty obvious given that this test is supposed to be compiled
> as C :-)

To be honest, I'm not at all sure to understand what's going on here, maybe we should return to the fail later.

> I guess the patch is ok now?

Yes. Nice that Jon replied in the meanwhile and clarified the undefined behavior issue: for the time being I think we can keep the __cplusplus checks, should also help during this testing period. We can certainly clean up the thing later. Maybe you could add a comment somewhere summarizing what Jon wrote.

By the way I noticed only today (sorry, I'm using a phone ;) that all the new configury work impacts only Solaris configuration, thus in general we are on pretty safe ground

Paolo
Paolo Carlini - Aug. 28, 2011, 6:16 p.m.
Hi again,

> FAIL: abi/header_cxxabi.c (test for excess errors)
> Excess errors:
> /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:167:1: error: unknown type name 'namespace'
> /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/libstdc++-v3/include/i386-pc-solaris2.11/bits/c++config.h:168:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before '{' token
> 
> which is pretty obvious given that this test is supposed to be compiled as C :-)

In the meanwhile I had the time to catch up with the email and got your point (for the accidental reader: being a C testcase *any* defined __cplusplus is wrong)

Paolo
Rainer Orth - Aug. 29, 2011, 12:59 p.m.
Hi Paolo,

>> I guess the patch is ok now?
>
> Yes. Nice that Jon replied in the meanwhile and clarified the undefined behavior issue: for the time being I think we can keep the __cplusplus checks, should also help during this testing period. We can certainly clean up the thing later. Maybe you could add a comment somewhere summarizing what Jon wrote.

for the moment, I've installed the patch as is.  It's probably better to
add some statement along those lines to user docs (and mention the
change in gcc-4.7/changes.html :-)

Btw., I just noticed the following in Oracle Studio 12.3 Beta CC(1):

     -features=a
          Enables/disables various C++ language features.

          The following table lists the -features suboption key-
          words and their meanings.  The prefix no% applied to a
          suboption disables that suboption.

          Value          Meaning
[...]
          cplusplus_redef
               Allow the normally pre-defined macro __cplusplus
               to be redefined by a -D option on the command
               line. Attempting to redefine __cplusplus with a
               #define directive in source code is not allowed.

               Example:
                CC -features=cplusplus_redef -D__cplusplus=1 ...

               The g++ compiler typically predefines the
               __cplusplus macro to 1, and source code might
               depend on this non-standard value. (The standard
               value is 199711L for compilers implementing the
               1998 C++ standard or the 2003 update. Future stan-
               dards will require a larger value for the macro.)

               Do not use this option unless you need to redefine
               __cplusplus to 1 in order to compile code intended
               for g++.

	Rainer
Paolo Carlini - Aug. 29, 2011, 1:30 p.m.
Hi,
> Hi Paolo,
>
>>> I guess the patch is ok now?
>> Yes. Nice that Jon replied in the meanwhile and clarified the undefined behavior issue: for the time being I think we can keep the __cplusplus checks, should also help during this testing period. We can certainly clean up the thing later. Maybe you could add a comment somewhere summarizing what Jon wrote.
> for the moment, I've installed the patch as is.  It's probably better to
> add some statement along those lines to user docs (and mention the
> change in gcc-4.7/changes.html :-)
Ok.
> Btw., I just noticed the following in Oracle Studio 12.3 Beta CC(1):
>
>       -features=a
>            Enables/disables various C++ language features.
>
>            The following table lists the -features suboption key-
>            words and their meanings.  The prefix no% applied to a
>            suboption disables that suboption.
>
>            Value          Meaning
> [...]
>            cplusplus_redef
>                 Allow the normally pre-defined macro __cplusplus
>                 to be redefined by a -D option on the command
>                 line. Attempting to redefine __cplusplus with a
>                 #define directive in source code is not allowed.
>
>                 Example:
>                  CC -features=cplusplus_redef -D__cplusplus=1 ...
>
>                 The g++ compiler typically predefines the
>                 __cplusplus macro to 1, and source code might
>                 depend on this non-standard value. (The standard
>                 value is 199711L for compilers implementing the
>                 1998 C++ standard or the 2003 update. Future stan-
>                 dards will require a larger value for the macro.)
>
>                 Do not use this option unless you need to redefine
>                 __cplusplus to 1 in order to compile code intended
>                 for g++.

Interesting. I suppose you are going to inform the SunStudio engineer 
that some of the above has to be adjusted for gcc 4.7+ (otherwise, 
please let me know, I can probably do it via Stephen Clamage)

Thanks,
Paolo.
Rainer Orth - Aug. 29, 2011, 3:12 p.m.
Hi Paolo,

> Interesting. I suppose you are going to inform the SunStudio engineer 
> that some of the above has to be adjusted for gcc 4.7+ (otherwise, 
> please let me know, I can probably do it via Stephen Clamage)

that's the plan.  I had some discussions about those issues when I
brought up the fixincludes changes within Solaris 11 Platinum Beta
earlier this year.  I plan to report the remaining fixincludes hacks
which are simply errors in the Solaris headers.

	Rainer

Patch

# HG changeset patch
# Parent b3524f20d0077532a567b222d37ef05976af2743
Handle different versions of Solaris 8 <iso/math_iso.h>

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -1693,6 +1693,100 @@  AC_DEFUN([GLIBCXX_COMPUTE_STDIO_INTEGER_
 ])
 
 dnl
+dnl Check whether required C++ overloads are present in <math.h>.
+dnl
+
+AC_DEFUN([GLIBCXX_CHECK_MATH_PROTO], [
+
+  AC_LANG_SAVE
+  AC_LANG_CPLUSPLUS
+
+  case "$host" in
+    *-*-solaris2.*)
+      # Solaris 8 FCS only had an overload for double std::abs(double) in
+      # <iso/math_iso.h>.  Patches 111721-04 (SPARC) and 112757-01 (x86)
+      # introduced the full set also found from Solaris 9 onwards.
+      AC_MSG_CHECKING([for float std::abs(float) overload])
+      AC_CACHE_VAL(glibcxx_cv_abs_float, [
+	AC_COMPILE_IFELSE([AC_LANG_SOURCE(
+	  [#include <math.h>
+	   namespace std {
+	     inline float abs(float __x)
+	     {  return __builtin_fabsf(__x); }
+	   }
+	])],
+        [glibcxx_cv_abs_float=no],
+        [glibcxx_cv_abs_float=yes]
+      )])
+
+      # autoheader cannot handle indented templates.
+      AH_VERBATIM([__CORRECT_ISO_CPP_MATH_H_PROTO1],
+        [/* Define if all C++ overloads are available in <math.h>.  */
+#if __cplusplus >= 199711L
+#undef __CORRECT_ISO_CPP_MATH_H_PROTO1
+#endif])
+      AH_VERBATIM([__CORRECT_ISO_CPP_MATH_H_PROTO2],
+        [/* Define if only double std::abs(double) is available in <math.h>.  */
+#if __cplusplus >= 199711L
+#undef __CORRECT_ISO_CPP_MATH_H_PROTO2
+#endif])
+
+      if test $glibcxx_cv_abs_float = yes; then
+        AC_DEFINE(__CORRECT_ISO_CPP_MATH_H_PROTO1)
+      else
+        AC_DEFINE(__CORRECT_ISO_CPP_MATH_H_PROTO2)
+      fi
+      AC_MSG_RESULT($glibcxx_cv_abs_float)
+      ;;
+  esac
+
+  AC_LANG_RESTORE
+])
+
+dnl
+dnl Check whether required C++ overloads are present in <stdlib.h>.
+dnl
+
+AC_DEFUN([GLIBCXX_CHECK_STDLIB_PROTO], [
+
+  AC_LANG_SAVE
+  AC_LANG_CPLUSPLUS
+
+  case "$host" in
+    *-*-solaris2.*)
+      # Solaris 8 FCS lacked the overloads for long std::abs(long) and
+      # ldiv_t std::div(long, long) in <iso/stdlib_iso.h>.  Patches 109607-02
+      # (SPARC) and 109608-02 (x86) introduced them.
+      AC_MSG_CHECKING([for long std::abs(long) overload])
+      AC_CACHE_VAL(glibcxx_cv_abs_long, [
+	AC_COMPILE_IFELSE([AC_LANG_SOURCE(
+	  [#include <stdlib.h>
+	   namespace std {
+	     inline long
+	     abs(long __i) { return labs(__i); }
+	   }
+        ])],
+        [glibcxx_cv_abs_long=no],
+        [glibcxx_cv_abs_long=yes]
+      )])
+
+      # autoheader cannot handle indented templates.
+      AH_VERBATIM([__CORRECT_ISO_CPP_STDLIB_H_PROTO],
+        [/* Define if all C++ overloads are available in <stdlib.h>.  */
+#if __cplusplus >= 199711L
+#undef __CORRECT_ISO_CPP_STDLIB_H_PROTO
+#endif])
+      if test $glibcxx_cv_abs_long = yes; then
+        AC_DEFINE(__CORRECT_ISO_CPP_STDLIB_H_PROTO, 1)
+      fi
+      AC_MSG_RESULT($glibcxx_cv_abs_long)
+      ;;
+  esac
+
+  AC_LANG_RESTORE
+])
+
+dnl
 dnl Check whether macros, etc are present for <system_error>
 dnl
 AC_DEFUN([GLIBCXX_CHECK_SYSTEM_ERROR], [
diff --git a/libstdc++-v3/config/os/solaris/solaris2.8/os_defines.h b/libstdc++-v3/config/os/solaris/solaris2.8/os_defines.h
--- a/libstdc++-v3/config/os/solaris/solaris2.8/os_defines.h
+++ b/libstdc++-v3/config/os/solaris/solaris2.8/os_defines.h
@@ -1,4 +1,4 @@ 
-// Specific definitions for Solaris 8  -*- C++ -*-
+// Specific definitions for Solaris 8+  -*- C++ -*-
 
 // Copyright (C) 2000, 2002, 2005, 2009, 2011 Free Software Foundation, Inc.
 //
@@ -28,9 +28,12 @@ 
 // System-specific #define, typedefs, corrections, etc, go here.  This
 // file will come before all others.
 
-// FIXME: Autoconf if possible.
 #if __cplusplus >= 199711L
-#define __CORRECT_ISO_CPP_MATH_H_PROTO2
+// Overloads in <iso/math_iso.h> and <iso/stdlib_iso.h> changed with
+// Solaris 8 patches.  Since <bits/c++config.h> includes
+// <bits/os_defines.h> before configure results,
+// __CORRECT_ISO_CPP_MATH_H_PROTO[12] and __CORRECT_ISO_CPP_STDLIB_H_PROTO
+// must be defined via acinclude.m4.
 #define __CORRECT_ISO_CPP_STRING_H_PROTO
 #define __CORRECT_ISO_CPP_WCHAR_H_PROTO
 #endif
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -135,6 +135,8 @@  GLIBCXX_ENABLE_FULLY_DYNAMIC_STRING([no]
 GLIBCXX_ENABLE_EXTERN_TEMPLATE([yes])
 
 # Checks for operating systems support that doesn't require linking.
+GLIBCXX_CHECK_MATH_PROTO
+GLIBCXX_CHECK_STDLIB_PROTO
 GLIBCXX_CHECK_SYSTEM_ERROR
 
 # For the streamoff typedef.
diff --git a/libstdc++-v3/configure.host b/libstdc++-v3/configure.host
--- a/libstdc++-v3/configure.host
+++ b/libstdc++-v3/configure.host
@@ -288,12 +288,9 @@  case "${host_os}" in
     echo "Please specify the full version of Solaris, ie. solaris2.9 " 1>&2
     exit 1
     ;;
-  solaris2.8)
+  solaris2.[89] | solaris2.1[0-9])
     os_include_dir="os/solaris/solaris2.8"
     ;;
-  solaris2.9 | solaris2.1[0-9])
-    os_include_dir="os/solaris/solaris2.9"
-    ;;
   tpf)
     os_include_dir="os/tpf"
     ;;