diff mbox

libstdc++/48891 Use ::isinf and ::isnan if libc defines them

Message ID 20160108135923.GK30323@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Jan. 8, 2016, 1:59 p.m. UTC 
Glibc defines obsolete isinf(double) and isnan(double) functions from
Unix98. For C99 these are hidden by the type-generic isinf and isnan
macros, but in C++11 <cmath> #undefs those macros, exposing the Unix98
functions. They then conflict with our own std::isinf(double) and
std::isnan(double) if users add them to the global namespace:

#include <cmath>
using std::isinf;

d.cc:3:12: error: ‘constexpr bool std::isinf(double)’ conflicts with a previous declaration
 using std::isinf;
            ^

This has been reported three times now, and with the fix I'm planning
for PR14608 this becomes essential to fix, because I'm adding a
<math.h> wrapper containing exactly the code above.

At https://sourceware.org/bugzilla/show_bug.cgi?id=19439 I've proposed
a glibc patch to suppress those functions for C++11 and later, but we
still need to make libstdc++ work with existing glibc releases.

This patch adds a configure check for non-macro isinf and isnan
declarations, and pulls them into namespace std when they're found.
This means we define the functions with the wrong return type (int not
bool) but that's better than having conflicting declarations that
don't compile at all!

I'm only checking for those functions for *-*-*gnu* targets, as I
don't know of any other targets where it's an issue. Solaris and
the BSDs don't define those functions. If it affects other targets we
can extend the check to cover them too.

With a fixed glibc (or other C libraries that don't declare those
functions) we continue to define extern "C++" bool std::isinf(double),
which is the correct signature and mangles differently to the glibc
functions.

I've tested this on x86_64-linux with and without the glibc fix, and
on powerpc64-linux and x86_64-freebsd10.2 and although the interaction
between libc and libstdc++ is always tricky, I'm confident it's safe
and certainly more correct than what we have now. We could potentially
even backport it to the branches.

Does anyone see any problem with this patch, or the proposed glibc
change?

Comments

Jonathan Wakely Jan. 13, 2016, 4:26 p.m. UTC | #1 
On 08/01/16 13:59 +0000, Jonathan Wakely wrote:
>Glibc defines obsolete isinf(double) and isnan(double) functions from
>Unix98. For C99 these are hidden by the type-generic isinf and isnan
>macros, but in C++11 <cmath> #undefs those macros, exposing the Unix98
>functions. They then conflict with our own std::isinf(double) and
>std::isnan(double) if users add them to the global namespace:
>
>#include <cmath>
>using std::isinf;
>
>d.cc:3:12: error: ‘constexpr bool std::isinf(double)’ conflicts with a previous declaration
>using std::isinf;
>           ^
>
>This has been reported three times now, and with the fix I'm planning
>for PR14608 this becomes essential to fix, because I'm adding a
><math.h> wrapper containing exactly the code above.
>
>At https://sourceware.org/bugzilla/show_bug.cgi?id=19439 I've proposed
>a glibc patch to suppress those functions for C++11 and later, but we
>still need to make libstdc++ work with existing glibc releases.
>
>This patch adds a configure check for non-macro isinf and isnan
>declarations, and pulls them into namespace std when they're found.
>This means we define the functions with the wrong return type (int not
>bool) but that's better than having conflicting declarations that
>don't compile at all!
>
>I'm only checking for those functions for *-*-*gnu* targets, as I
>don't know of any other targets where it's an issue. Solaris and
>the BSDs don't define those functions. If it affects other targets we
>can extend the check to cover them too.
>
>With a fixed glibc (or other C libraries that don't declare those
>functions) we continue to define extern "C++" bool std::isinf(double),
>which is the correct signature and mangles differently to the glibc
>functions.
>
>I've tested this on x86_64-linux with and without the glibc fix, and
>on powerpc64-linux and x86_64-freebsd10.2 and although the interaction
>between libc and libstdc++ is always tricky, I'm confident it's safe
>and certainly more correct than what we have now. We could potentially
>even backport it to the branches.
>
>Does anyone see any problem with this patch, or the proposed glibc
>change?
>

>commit e5a8514077918b7885266ab3e136ae7b8ad65f99
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Thu Jan 7 16:01:09 2016 +0000
>
>    Use ::isinf and ::isnan if libc defines them
>    
>    	PR libstdc++/48891
>    	* acinclude.m4 (GLIBCXX_CHECK_MATH11_PROTO): Check for obsolete isinf
>    	and isnan functions.
>    	* config.h.in: Regenerate.
>    	* configure: Regenerate.
>    	* include/c_global/cmath (isinf(double), isnan(double))
>    	[_GLIBCXX_HAVE_OBSOLETE_ISINF_ISNAN]: Import via using-directive.
>    	* testsuite/26_numerics/headers/cmath/48891.cc: New.

Committed to trunk.

My patch to suppress those functions for >= C++11 has also been
committed to glibc.


Should we backport the libstdc++ fix to the branches too?
diff mbox

Patch

commit e5a8514077918b7885266ab3e136ae7b8ad65f99
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 7 16:01:09 2016 +0000

    Use ::isinf and ::isnan if libc defines them
    
    	PR libstdc++/48891
    	* acinclude.m4 (GLIBCXX_CHECK_MATH11_PROTO): Check for obsolete isinf
    	and isnan functions.
    	* config.h.in: Regenerate.
    	* configure: Regenerate.
    	* include/c_global/cmath (isinf(double), isnan(double))
    	[_GLIBCXX_HAVE_OBSOLETE_ISINF_ISNAN]: Import via using-directive.
    	* testsuite/26_numerics/headers/cmath/48891.cc: New.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 75e4667..b76e8d5 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2186,6 +2186,40 @@  AC_DEFUN([GLIBCXX_CHECK_MATH11_PROTO], [
       fi
       AC_MSG_RESULT([$glibcxx_cv_math11_overload])
       ;;
+    *-*-*gnu*)
+      # If <math.h> defines the obsolete isinf(double) and isnan(double)
+      # functions (instead of or as well as the C99 generic macros) then we
+      # can't define std::isinf(double) and std::isnan(double) in <cmath>
+      # and must use the ones from <math.h> instead.
+      AC_MSG_CHECKING([for obsolete isinf and isnan functions in <math.h>])
+        AC_CACHE_VAL(glibcxx_cv_obsolete_isinf_isnan, [
+          AC_COMPILE_IFELSE([AC_LANG_SOURCE(
+            [#include <math.h>
+             #undef isinf
+             #undef isnan
+             namespace std {
+               using ::isinf;
+               bool isinf(float);
+               bool isinf(long double);
+               using ::isnan;
+               bool isnan(float);
+               bool isnan(long double);
+             }
+             using std::isinf;
+             using std::isnan;
+             bool b = isinf(0.0) || isnan(0.0);
+          ])],
+          [glibcxx_cv_obsolete_isinf_isnan=yes],
+          [glibcxx_cv_obsolete_isinf_isnan=no]
+        )])
+
+
+      if test $glibcxx_cv_obsolete_isinf_isnan = yes; then
+        AC_DEFINE(HAVE_OBSOLETE_ISINF_ISNAN, 1,
+                  [Define if <math.h> defines obsolete isinf and isnan functions.])
+      fi
+      AC_MSG_RESULT([$glibcxx_cv_obsolete_isinf_isnan])
+      ;;
   esac
 
   CXXFLAGS="$ac_save_CXXFLAGS"
diff --git a/libstdc++-v3/include/c_global/cmath b/libstdc++-v3/include/c_global/cmath
index da7ec28..6269e32 100644
--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -606,9 +606,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   isinf(float __x)
   { return __builtin_isinf(__x); }
 
+#ifdef _GLIBCXX_HAVE_OBSOLETE_ISINF_ISNAN
+  using ::isinf;
+#else
   constexpr bool
   isinf(double __x)
   { return __builtin_isinf(__x); }
+#endif
 
   constexpr bool
   isinf(long double __x)
@@ -626,9 +630,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   isnan(float __x)
   { return __builtin_isnan(__x); }
 
+#ifdef _GLIBCXX_HAVE_OBSOLETE_ISINF_ISNAN
+  using ::isnan;
+#else
   constexpr bool
   isnan(double __x)
   { return __builtin_isnan(__x); }
+#endif
 
   constexpr bool
   isnan(long double __x)
diff --git a/libstdc++-v3/testsuite/26_numerics/headers/cmath/48891.cc b/libstdc++-v3/testsuite/26_numerics/headers/cmath/48891.cc
new file mode 100644
index 0000000..ef94fe1
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/headers/cmath/48891.cc
@@ -0,0 +1,30 @@ 
+// Copyright (C) 2016 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/>.
+
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+// PR libstdc++/48891
+
+#include <math.h>
+#include <cmath>
+
+using std::isinf;
+using std::isnan;
+
+bool d1 = isinf(1.0);
+bool d2 = isnan(1.0);