Since glibc 2.26, <math.h> uses __builtin_types_compatible_p and __typeof for isinf, which are GCC extensions that GCC 6.3 only provides in C mode. This means <math.h> cannot be used from C++ programs. Whether this in itself is harmful is questionable, but it certainly is a regression and implies at least one serious implication: the GCC 6.3 libstdc++v3 configure check "for ISO C99 support in <math.h> for C++98" now fails, resulting in a libstdc++ <cmath> header that simply includes <math.h>, which makes every C++ program fail to compile that correctly uses <cmath>.
Gabriel, I had thought this usage - and the definition of __HAVE_GENERIC_SELECTION, which relates to a C-only feature but is not conditional on C in any way - was harmless because C++ doesn't actually use these macros (libstdc++ undefines them in its headers and defines its own function overloads). Or, at least, that it was harmless except for issignaling, which libstdc++ doesn't handle. But given the configure test, maybe that needs revisiting. (a) Don't define __HAVE_GENERIC_SELECTION for C++. (b) The use of __builtin_choose_expr is unnecessary, a conditional expression should suffice. (__typeof *is* supported for C++, it's __builtin_choose_expr and __builtin_types_compatible_p that aren't.) (c) Use some suitable C++ magic in place of __builtin_types_compatible_p (remembering that __builtin_types_compatible_p explicitly removes type qualifiers, and alternative C++ magic needs to do likewise).
(In reply to Joseph Myers from comment #1) > (a) Don't define __HAVE_GENERIC_SELECTION for C++. OK. > (b) The use of __builtin_choose_expr is unnecessary, a conditional > expression should suffice. (__typeof *is* supported for C++, it's > __builtin_choose_expr and __builtin_types_compatible_p that aren't.) OK. > (c) Use some suitable C++ magic in place of __builtin_types_compatible_p > (remembering that __builtin_types_compatible_p explicitly removes type > qualifiers, and alternative C++ magic needs to do likewise). Since typeid also removes qualifiers implicitly (http://en.cppreference.com/w/cpp/language/typeid), would something in the lines of the following be acceptable for c++98: if (typeid(f) == typeid(float)) isinff (f) Likewise for the other types and for the other functions (defined through __MATH_TG)? For C++11, we could use is_same, but we would need to remove the qualifiers explicitly... Something in the lines of: if (std::is_same<std::remove_cv<typeof(f)>::type, float>::value) This seems more complex, so I wonder if I'm missing something in the proposed test for c++98.
On Wed, 9 Aug 2017, gftg at linux dot vnet.ibm.com wrote: > > (c) Use some suitable C++ magic in place of __builtin_types_compatible_p > > (remembering that __builtin_types_compatible_p explicitly removes type > > qualifiers, and alternative C++ magic needs to do likewise). > > Since typeid also removes qualifiers implicitly > (http://en.cppreference.com/w/cpp/language/typeid), would something in the > lines of the following be acceptable for c++98: > > if (typeid(f) == typeid(float)) isinff (f) No, because that involves a runtime comparison of type_info objects. We want something (to go in a __MATH_TG macro definition for the "C++ and distinct float128" case - and, similarly, an isinf definition for when C++ is combined with the present conditions for handling GCC < 7 with _Float128) that's optimized away at compile time - something where the conditionals are always folded to true or false. (And preferably not depending on including any C++ headers from <math.h>; I don't know what the rules are for such header inclusion in the C++ standard library.) Maybe we need to understand further what cases are the problem. Various macros may be used in that configure test, but most of them are using type-generic built-in functions which work fine for C++ (given that libstdc++ is not built with sNaNs enabled). The problem for the configure test might only be the "__builtin_isinf_sign is broken for float128 only before GCC 7.0." definition of isinf, which uses __builtin_types_compatible_p. Separately, with the installed compiler, most of the macros should be undefined in the libstdc++ headers, leaving issignaling as the likely problem case using __MATH_TG (issubnormal is defined using fpclassify; iszero has a separate C++ template definition, to avoid problems with macros). Possible solutions for isinf include: * Do not use the special-case isinf definition with __builtin_types_compatible_p for C++. It's possible this suffices to make the configure test work (and exactly what the isinf macro does doesn't matter when using an installed C++ compiler because the libstdc++ headers undefine that macro, and the configure test doesn't use it with float128). * Add a C++ variant of that definition, which would only be used for the configure test but which still has the right semantics. And for issignaling: * Change __MATH_TG to have a fully functional C++ variant. float and double can be distinguished with sizeof; it's only long double / float128 where you need a replacement for __builtin_types_compatible_p (__typeof (TG_ARG), long double) using C++ features. * Add template versions of issignaling for C++, like iszero.
(In reply to joseph@codesourcery.com from comment #3) > * Do not use the special-case isinf definition with > __builtin_types_compatible_p for C++. It's possible this suffices to make > the configure test work Indeed, this is enough for the configure test to work. I sent this change as: https://sourceware.org/ml/libc-alpha/2017-08/msg00588.html The other emails in the thread address the other concerns raised in this bug report.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, master has been updated via 6913ad65e00bb32417ad39c41d292b976171e27e (commit) via 47a67213a9f51c5f8816d240500b10db605d8b77 (commit) from c647fb885cb678471f6b6a66f394b4ca5515a016 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=6913ad65e00bb32417ad39c41d292b976171e27e commit 6913ad65e00bb32417ad39c41d292b976171e27e Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Mon Aug 14 17:51:51 2017 -0300 Do not use generic selection in C++ mode The logic to protect the use of generic selection (_Generic) does not check for C or C++ mode, however, generic selection is a C-only feature. Tested for powerpc64le. * misc/sys/cdefs.h (__HAVE_GENERIC_SELECTION): Define to 0, if in C++ mode. https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=47a67213a9f51c5f8816d240500b10db605d8b77 commit 47a67213a9f51c5f8816d240500b10db605d8b77 Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Fri Aug 11 14:29:06 2017 -0300 Do not use __builtin_types_compatible_p in C++ mode (bug 21930) The logic to define isinf for float128 depends on the availability of __builtin_types_compatible_p, which is only available in C mode, however, the conditionals do not check for C or C++ mode. This lead to an error in libstdc++ configure, as reported by bug 21930. This patch adds a conditional for C mode in the definition of isinf for float128. No definition is provided in C++ mode, since libstdc++ headers undefine isinf. Tested for powerpc64le (glibc test suite and libstdc++-v3 configure). [BZ #21930] * math/math.h (isinf): Check if in C or C++ mode before using __builtin_types_compatible_p, since this is a C mode feature. ----------------------------------------------------------------------- Summary of changes: ChangeLog | 11 +++++++++++ math/math.h | 8 ++++++-- misc/sys/cdefs.h | 19 ++++++++++--------- 3 files changed, 27 insertions(+), 11 deletions(-)
*** Bug 21983 has been marked as a duplicate of this bug. ***
There are more uses of __builtin_types_compatible_p, enabled with -fsignaling-nans.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, release/2.26/master has been updated via 5e989c36934d0f0cf13b7a53ef2fa440bce39210 (commit) via c2921b17a37e887b8a5ca9d84b875b9ba702b79c (commit) from 645b7635ba8fd58062245419e8bb668ab90cd3ec (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5e989c36934d0f0cf13b7a53ef2fa440bce39210 commit 5e989c36934d0f0cf13b7a53ef2fa440bce39210 Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Mon Aug 14 17:51:51 2017 -0300 Do not use generic selection in C++ mode The logic to protect the use of generic selection (_Generic) does not check for C or C++ mode, however, generic selection is a C-only feature. Tested for powerpc64le. * misc/sys/cdefs.h (__HAVE_GENERIC_SELECTION): Define to 0, if in C++ mode. (cherry picked from commit 6913ad65e00bb32417ad39c41d292b976171e27e) https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c2921b17a37e887b8a5ca9d84b875b9ba702b79c commit c2921b17a37e887b8a5ca9d84b875b9ba702b79c Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Mon Aug 21 14:23:27 2017 +0200 Do not use __builtin_types_compatible_p in C++ mode (bug 21930) The logic to define isinf for float128 depends on the availability of __builtin_types_compatible_p, which is only available in C mode, however, the conditionals do not check for C or C++ mode. This lead to an error in libstdc++ configure, as reported by bug 21930. This patch adds a conditional for C mode in the definition of isinf for float128. No definition is provided in C++ mode, since libstdc++ headers undefine isinf. Tested for powerpc64le (glibc test suite and libstdc++-v3 configure). [BZ #21930] * math/math.h (isinf): Check if in C or C++ mode before using __builtin_types_compatible_p, since this is a C mode feature. (cherry picked from commit 47a67213a9f51c5f8816d240500b10db605d8b77) ----------------------------------------------------------------------- Summary of changes: ChangeLog | 11 +++++++++++ NEWS | 1 + math/math.h | 8 ++++++-- misc/sys/cdefs.h | 19 ++++++++++--------- 4 files changed, 28 insertions(+), 11 deletions(-)
Most of the macros, including fpclassify, should be irrelevant because libstdc++ headers undefine the macros and replace them with function overloads (as required by the C++ standard). The problem with iszero is specifically that the C++ iszero definition in the C math.h is using the macro fpclassify definition (this is before libstdc++ gets a chance to undefine it), and, in the case (__SUPPORT_SNAN__ and float128 supported), that definition is using __builtin_types_compatible_p. So some approach is needed for iszero to work in that place - without conflicting with libstdc++'s own fpclassify overloads.
I will add, for completeness: I don't consider the uses of __builtin_types_compatible_p in tgmath.h for float128 to be a bug. For C++11 and later libstdc++'s tgmath.h does not include the C library header, and the C macros in tgmath.h rely on details of C type conversions which don't work for C++ anyway.
For iszero, I could write a patch similar to the one I wrote (and that is still under review) for issignaling (https://sourceware.org/ml/libc-alpha/2017-08/msg00859.html). That would make iszero use the correct __fpclassify* function for each type, instead of relying on the type-generic fpclassify macro. Does that sound like a good approach for this problem?
That approach to iszero seems reasonable (provided it's only for the __SUPPORT_SNAN__ case, the case where iszero just compares with zero can stay with the template).
I posted a patch [1] to libc-alpha that addesses the problem with iszero and -fsignaling-nans (if I'm not mistaken, this is the only problem mentioned in this bug report that has not yet being dealt with). [1] https://sourceware.org/ml/libc-alpha/2017-08/msg01163.html
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, master has been updated via 42496114ec0eb7d6d039d05d4262e109951c600c (commit) from 56bc7f43603b5d28437496efb32df40997c62cb4 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=42496114ec0eb7d6d039d05d4262e109951c600c commit 42496114ec0eb7d6d039d05d4262e109951c600c Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Tue Aug 22 16:34:42 2017 -0300 Provide a C++ version of iszero that does not use __MATH_TG (bug 21930) When signaling nans are enabled (with -fsignaling-nans), the C++ version of iszero uses the fpclassify macro, which is defined with __MATH_TG. However, when support for float128 is available, __MATH_TG uses the builtin __builtin_types_compatible_p, which is only available in C mode. This patch refactors the C++ version of iszero so that it uses function overloading to select between the floating-point types, instead of relying on fpclassify and __MATH_TG. Tested for powerpc64le, s390x, x86_64, and with build-many-glibcs.py. [BZ #21930] * math/math.h [defined __cplusplus && defined __SUPPORT_SNAN__] (iszero): New C++ implementation that does not use fpclassify/__MATH_TG/__builtin_types_compatible_p, when signaling nans are enabled, since __builtin_types_compatible_p is a C-only feature. * math/test-math-iszero.cc: When __HAVE_DISTINCT_FLOAT128 is defined, include ieee754_float128.h for access to the union and member ieee854_float128.ieee. [__HAVE_DISTINCT_FLOAT128] (do_test): Call check_float128. [__HAVE_DISTINCT_FLOAT128] (check_float128): New function. * sysdeps/powerpc/powerpc64le/Makefile [subdir == math] (CXXFLAGS-test-math-iszero.cc): Add -mfloat128 to the build options of test-math-zero on powerpc64le. ----------------------------------------------------------------------- Summary of changes: ChangeLog | 17 +++++++ math/math.h | 33 ++++++++++++-- math/test-math-iszero.cc | 79 ++++++++++++++++++++++++++++++++++ sysdeps/powerpc/powerpc64le/Makefile | 3 +- 4 files changed, 127 insertions(+), 5 deletions(-)
Fixed for 2.27.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, release/2.26/master has been updated via 58270c0049404ef2f878fdd45df55f17f0b8c1f7 (commit) via 35dded99a89db873b06270ca7f21245a0faf712a (commit) via ef8566d72af5e03c1b82cf02efb794268a347f8c (commit) from 6043d77a47de297b62084c1c261cdada082bf09c (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=58270c0049404ef2f878fdd45df55f17f0b8c1f7 commit 58270c0049404ef2f878fdd45df55f17f0b8c1f7 Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Tue Aug 22 16:34:42 2017 -0300 Provide a C++ version of iszero that does not use __MATH_TG (bug 21930) When signaling nans are enabled (with -fsignaling-nans), the C++ version of iszero uses the fpclassify macro, which is defined with __MATH_TG. However, when support for float128 is available, __MATH_TG uses the builtin __builtin_types_compatible_p, which is only available in C mode. This patch refactors the C++ version of iszero so that it uses function overloading to select between the floating-point types, instead of relying on fpclassify and __MATH_TG. Tested for powerpc64le, s390x, x86_64, and with build-many-glibcs.py. [BZ #21930] * math/math.h [defined __cplusplus && defined __SUPPORT_SNAN__] (iszero): New C++ implementation that does not use fpclassify/__MATH_TG/__builtin_types_compatible_p, when signaling nans are enabled, since __builtin_types_compatible_p is a C-only feature. * math/test-math-iszero.cc: When __HAVE_DISTINCT_FLOAT128 is defined, include ieee754_float128.h for access to the union and member ieee854_float128.ieee. [__HAVE_DISTINCT_FLOAT128] (do_test): Call check_float128. [__HAVE_DISTINCT_FLOAT128] (check_float128): New function. * sysdeps/powerpc/powerpc64le/Makefile [subdir == math] (CXXFLAGS-test-math-iszero.cc): Add -mfloat128 to the build options of test-math-zero on powerpc64le. (cherry picked from commit 42496114ec0eb7d6d039d05d4262e109951c600c) https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=35dded99a89db873b06270ca7f21245a0faf712a commit 35dded99a89db873b06270ca7f21245a0faf712a Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Wed Aug 23 10:16:54 2017 -0300 Fix the C++ version of issignaling when __NO_LONG_DOUBLE_MATH is defined When __NO_LONG_DOUBLE_MATH is defined, __issignalingl is not available, thus issignaling with long double argument should call __issignaling, instead. Tested for powerpc64le. * math/math.h [defined __cplusplus] (issignaling): In the long double case, call __issignalingl only if __NO_LONG_DOUBLE_MATH is not defined. Call __issignaling, otherwise. (cherry picked from commit 3d7b66f66cb223e899a7ebc0f4c20f13e711c9e0) https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=ef8566d72af5e03c1b82cf02efb794268a347f8c commit ef8566d72af5e03c1b82cf02efb794268a347f8c Author: Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> Date: Mon Aug 14 13:46:15 2017 -0300 Provide a C++ version of issignaling that does not use __MATH_TG The macro __MATH_TG contains the logic to select between long double and _Float128, when these types are ABI-distinct. This logic relies on __builtin_types_compatible_p, which is not available in C++ mode. On the other hand, C++ function overloading provides the means to distinguish between the floating-point types. The overloading resolution will match the correct parameter regardless of type qualifiers, i.e.: const and volatile. Tested for powerpc64le, s390x, and x86_64. * math/math.h [defined __cplusplus] (issignaling): Provide a C++ definition for issignaling that does not rely on __MATH_TG, since __MATH_TG uses __builtin_types_compatible_p, which is only available in C mode. (CFLAGS-test-math-issignaling.cc): New variable. * math/Makefile [CXX] (tests): Add test-math-issignaling. * math/test-math-issignaling.cc: New test for C++ implementation of type-generic issignaling. * sysdeps/powerpc/powerpc64le/Makefile [subdir == math] (CXXFLAGS-test-math-issignaling.cc): Add -mfloat128 to the build options of test-math-issignaling on powerpc64le. (cherry picked from commit a16e8bc08edca84d507715c66d6cddbbc7ed3b62) ----------------------------------------------------------------------- Summary of changes: ChangeLog | 37 +++++++++ math/Makefile | 3 +- math/math.h | 60 ++++++++++++++- ...est-math-iszero.cc => test-math-issignaling.cc} | 78 +++++++++++++------ math/test-math-iszero.cc | 79 ++++++++++++++++++++ sysdeps/powerpc/powerpc64le/Makefile | 4 +- 6 files changed, 229 insertions(+), 32 deletions(-) copy math/{test-math-iszero.cc => test-math-issignaling.cc} (52%)
*** Bug 22650 has been marked as a duplicate of this bug. ***