diff mbox series

[v2,01/10] ldbl-128ibm-compat: workaround C++ redirect limitations

Message ID 20200327211801.31234-2-murphyp@linux.vnet.ibm.com
State Superseded
Delegated to: Joseph Myers
Headers show
Series IEEE binary128 long double on powerpc64le | expand

Commit Message

Paul E. Murphy March 27, 2020, 9:17 p.m. UTC
GCC 8+ is more pedantic about type checking the redirect declarations
for non-system headers.  I am not sure if there is less obtrusive way
to dodge these warnings when building C++ tests using the headers from
the glibc under construction.
---
 include/monetary.h |  8 ++++++++
 include/printf.h   |  8 ++++++++
 include/stdio.h    | 10 ++++++++++
 include/stdlib.h   |  9 +++++++++
 include/wchar.h    |  9 +++++++++
 5 files changed, 44 insertions(+)

Comments

Joseph Myers March 27, 2020, 9:34 p.m. UTC | #1
On Fri, 27 Mar 2020, Paul E. Murphy via Libc-alpha wrote:

> GCC 8+ is more pedantic about type checking the redirect declarations
> for non-system headers.  I am not sure if there is less obtrusive way
> to dodge these warnings when building C++ tests using the headers from
> the glibc under construction.

The first thing you need to include in the commit message for such a 
change is *the exact diagnostics you're trying to avoid*.  Only given 
those diagnostics can we try to work out a better fix.
Paul E Murphy April 1, 2020, 8:29 p.m. UTC | #2
On 3/27/20 4:34 PM, Joseph Myers wrote:
> On Fri, 27 Mar 2020, Paul E. Murphy via Libc-alpha wrote:
> 
>> GCC 8+ is more pedantic about type checking the redirect declarations
>> for non-system headers.  I am not sure if there is less obtrusive way
>> to dodge these warnings when building C++ tests using the headers from
>> the glibc under construction.

How about the following commit message body?

GCC 9.2 is more pedantic about type checking the redirect declarations
for non-system headers.  I am not sure if there is less obtrusive way
to dodge these warnings when building C++ tests using the headers from
the glibc under construction.

Many errors of the following form appear:

In file included from ../include/sys/cdefs.h:3,
                  from ../include/features.h:465,
                  from ../bits/libc-header-start.h:33,
                  from ../math/math.h:27,
                  from ../include/math.h:7,
                  from test-math-isinff.cc:21:
../libio/bits/stdio-ldbl.h:25:20: error: declaration of ‘int 
sprintf(char*, const char*, ...)’ has a different exception specifier
    25 | __LDBL_REDIR_DECL (sprintf)
       |                    ^~~~~~~
../misc/sys/cdefs.h:461:26: note: in definition of macro ‘__LDBL_REDIR_DECL’
   461 |   extern __typeof (name) name __asm (__ASMNAME ("__" #name 
"ieee128"));
       |                          ^~~~
In file included from ../include/stdio.h:5,
                  from test-math-isinff.cc:22:
../libio/stdio.h:334:12: note: from previous declaration ‘int 
sprintf(char*, const char*, ...) throw ()’
   334 | extern int sprintf (char *__restrict __s,
       |            ^~~~~~~


> 
> The first thing you need to include in the commit message for such a
> change is *the exact diagnostics you're trying to avoid*.  Only given
> those diagnostics can we try to work out a better fix.
> 
I noticed this occurs on FSF GCC 9.2, but not 9.3 and newer.  The 
documentation is vague about which warnings and errors are disabled by 
system_header pragma.
Joseph Myers April 1, 2020, 11:07 p.m. UTC | #3
On Wed, 1 Apr 2020, Paul E Murphy via Libc-alpha wrote:

> GCC 9.2 is more pedantic about type checking the redirect declarations
> for non-system headers.  I am not sure if there is less obtrusive way
> to dodge these warnings when building C++ tests using the headers from
> the glibc under construction.

Warnings about inconsistency of attributes for aliases were addressed by 
using __attribute_copy__ in alias macros.  Those warnings typically 
indicated that code using the aliases would be suboptimal in some way (not 
fully optimized based on attributes, or with diagnostics based on those 
attributes potentially missing).

This seems like something similar, but about C++ exception specifications 
instead of attributes.  If there isn't some way to copy exception 
specifications, maybe there needs to be a variant of __LDBL_REDIR_DECL 
where attributes / exception specifications / ... can be passed in 
explicitly.  Then you can include __THROWNL in that call to 
__LDBL_REDIR_DECL and similarly for all other such redirected functions 
where __THROWNL is part of the declaration of the original function.

> I noticed this occurs on FSF GCC 9.2, but not 9.3 and newer.  The
> documentation is vague about which warnings and errors are disabled by
> system_header pragma.

Or if it's a GCC bug it would be useful to identify exactly which bug in 
GCC Bugzilla it is / which commit fixed it (and potentially have a 
workaround that is explicitly conditioned on the buggy GCC versions with a 
comment referencing the exact bug and fixed versions).
Paul E Murphy April 2, 2020, 4:48 p.m. UTC | #4
On 4/1/20 6:07 PM, Joseph Myers wrote:
> On Wed, 1 Apr 2020, Paul E Murphy via Libc-alpha wrote:
> 
>> GCC 9.2 is more pedantic about type checking the redirect declarations
>> for non-system headers.  I am not sure if there is less obtrusive way
>> to dodge these warnings when building C++ tests using the headers from
>> the glibc under construction.
> 
> Warnings about inconsistency of attributes for aliases were addressed by
> using __attribute_copy__ in alias macros.  Those warnings typically
> indicated that code using the aliases would be suboptimal in some way (not
> fully optimized based on attributes, or with diagnostics based on those
> attributes potentially missing).
> 
> This seems like something similar, but about C++ exception specifications
> instead of attributes.  If there isn't some way to copy exception
> specifications, maybe there needs to be a variant of __LDBL_REDIR_DECL
> where attributes / exception specifications / ... can be passed in
> explicitly.  Then you can include __THROWNL in that call to
> __LDBL_REDIR_DECL and similarly for all other such redirected functions
> where __THROWNL is part of the declaration of the original function.
> 
>> I noticed this occurs on FSF GCC 9.2, but not 9.3 and newer.  The
>> documentation is vague about which warnings and errors are disabled by
>> system_header pragma.
> 
> Or if it's a GCC bug it would be useful to identify exactly which bug in
> GCC Bugzilla it is / which commit fixed it (and potentially have a
> workaround that is explicitly conditioned on the buggy GCC versions with a
> comment referencing the exact bug and fixed versions).
> 

On second thought, this is surely a bug.  With some exploration, I found 
GCC PR 90731 fixed it between 9.2 and 9.3.  I will seek a lower bound 
and update the patch.
diff mbox series

Patch

diff --git a/include/monetary.h b/include/monetary.h
index 240925e87d..81933a2c5e 100644
--- a/include/monetary.h
+++ b/include/monetary.h
@@ -1,3 +1,11 @@ 
+/* This is needed to keep GCC >= 9 happy when using redirects inside system
+   headers.   glibc builds some C++ tests which use these headers which do
+   not get marked as system headers.  */
+#include <bits/floatn.h>
+#if defined __cplusplus && __LONG_DOUBLE_USES_FLOAT128 == 1
+# pragma GCC system_header
+#endif
+
 #include <stdlib/monetary.h>
 #ifndef _ISOMAC
 #include <stdarg.h>
diff --git a/include/printf.h b/include/printf.h
index d051514119..9e74e35678 100644
--- a/include/printf.h
+++ b/include/printf.h
@@ -1,5 +1,13 @@ 
 #ifndef	_PRINTF_H
 
+/* This is needed to keep GCC >= 9 happy when using redirects inside system
+   headers.   glibc builds some C++ tests which use these headers which do
+   not get marked as system headers.  */
+#include <bits/floatn.h>
+#if defined __cplusplus && __LONG_DOUBLE_USES_FLOAT128 == 1
+# pragma GCC system_header
+#endif
+
 #include <stdio-common/printf.h>
 
 # ifndef _ISOMAC
diff --git a/include/stdio.h b/include/stdio.h
index 6718af4108..517d53837f 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -2,8 +2,18 @@ 
 # if !defined _ISOMAC && defined _IO_MTSAFE_IO
 #  include <stdio-lock.h>
 # endif
+
+/* This is needed to keep GCC >= 9 happy when using redirects inside system
+   headers.   glibc builds some C++ tests which use these headers which do
+   not get marked as system headers.  */
+# include <bits/floatn.h>
+# if defined __cplusplus && __LONG_DOUBLE_USES_FLOAT128 == 1
+#  pragma GCC system_header
+# endif
+
 # include <libio/stdio.h>
 # ifndef _ISOMAC
+
 #  define _LIBC_STDIO_H 1
 #  include <libio/libio.h>
 
diff --git a/include/stdlib.h b/include/stdlib.h
index 926f965f69..7929e45386 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -3,6 +3,15 @@ 
 #ifndef _ISOMAC
 # include <stddef.h>
 #endif
+
+/* This is needed to keep GCC >= 9 happy when using redirects inside system
+   headers.   glibc builds some C++ tests which use these headers which do
+   not get marked as system headers.  */
+#include <bits/floatn.h>
+#if defined __cplusplus && __LONG_DOUBLE_USES_FLOAT128 == 1
+# pragma GCC system_header
+#endif
+
 #include <stdlib/stdlib.h>
 
 /* Now define the internal interfaces.  */
diff --git a/include/wchar.h b/include/wchar.h
index 617906eb14..64355dd6fb 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -1,4 +1,13 @@ 
 #ifndef _WCHAR_H
+
+/* This is needed to keep GCC >= 9 happy when using redirects inside system
+   headers.   glibc builds some C++ tests which use these headers which do
+   not get marked as system headers.  */
+# include <bits/floatn.h>
+# if defined __cplusplus && __LONG_DOUBLE_USES_FLOAT128 == 1
+#  pragma GCC system_header
+# endif
+
 # include <wcsmbs/wchar.h>
 # ifndef _ISOMAC