[v2,2/3] cdefs.h: Define __glibc_warn_system_headers_{begin, end}

Message ID 20230528172013.73111-3-bugaevc@gmail.com
State Superseded
Headers
Series fcntl fortification |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 pending Patch applied

Commit Message

Sergey Bugaev May 28, 2023, 5:20 p.m. UTC
  By default, GCC supresses warnings inside "system headers". Moreover, it
also supresses warnings resulting from expansion of macros defined in
system headers, even in the expansion itself happens in user code. This
may be desirable most of the time because the user cannot do anything
about the mistakes of the system headers, but sometimes causing a
warning is an intentional effect that a macro has, in which case this
GCC feature gets in a way.

GCC allows turning off the warning suspension feature by passing
-Wsystem-headers; however, that turns it off globally. But by using
"#pragma GCC diagnostic" it can be made to only apply to the relevant
macro definition, in which case GCC only does not supress warnings
resulting from expansion of that one macro.

To that end, introduce __glibc_warn_system_headers_begin and
__glibc_warn_system_headers_end macros that can be used to surround a
macro definition and ensure that warnings inside the macro are not
supressed:

    __glibc_warn_system_headers_begin
    #define foo(x) bar_warn (x)
    __glibc_warn_system_headers_end

This will be used in a following patch which turns fcntl and fcntl64
into macros that cause warnings on argument type mismatch.

Note that "#pragma GCC diagnostic warning" normally overrides any
diagnostic options specified on the command line, and so can even
downgrade severity of a diagnostic from an error to a warning (if, for
instance, -Werror is in effect). But this is not a concern here, since
the actual warning that gets emitted is not "-Wsystem-headers", but some
other specific warning; "-Wsystem-headers" is only used to disable its
supression. So passing -Werror still causes the specific warning to be
treated as an error, and to fail the compilation.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---

I'm not very sure about the prereq check. I stole it from the
__glibc_macro_warning above (which also uses _Pragma), and according to
the online docs GCC 4 has already supported -Wsystem-headers.

 misc/sys/cdefs.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
  

Comments

Adhemerval Zanella Netto May 29, 2023, 2:50 p.m. UTC | #1
On 28/05/23 14:20, Sergey Bugaev via Libc-alpha wrote:
> By default, GCC supresses warnings inside "system headers". Moreover, it
> also supresses warnings resulting from expansion of macros defined in
> system headers, even in the expansion itself happens in user code. This
> may be desirable most of the time because the user cannot do anything
> about the mistakes of the system headers, but sometimes causing a
> warning is an intentional effect that a macro has, in which case this
> GCC feature gets in a way.
> 
> GCC allows turning off the warning suspension feature by passing
> -Wsystem-headers; however, that turns it off globally. But by using
> "#pragma GCC diagnostic" it can be made to only apply to the relevant
> macro definition, in which case GCC only does not supress warnings
> resulting from expansion of that one macro.
> 
> To that end, introduce __glibc_warn_system_headers_begin and
> __glibc_warn_system_headers_end macros that can be used to surround a
> macro definition and ensure that warnings inside the macro are not
> supressed:
> 
>     __glibc_warn_system_headers_begin
>     #define foo(x) bar_warn (x)
>     __glibc_warn_system_headers_end
> 
> This will be used in a following patch which turns fcntl and fcntl64
> into macros that cause warnings on argument type mismatch.
> 
> Note that "#pragma GCC diagnostic warning" normally overrides any
> diagnostic options specified on the command line, and so can even
> downgrade severity of a diagnostic from an error to a warning (if, for
> instance, -Werror is in effect). But this is not a concern here, since
> the actual warning that gets emitted is not "-Wsystem-headers", but some
> other specific warning; "-Wsystem-headers" is only used to disable its
> supression. So passing -Werror still causes the specific warning to be
> treated as an error, and to fail the compilation.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> 
> I'm not very sure about the prereq check. I stole it from the
> __glibc_macro_warning above (which also uses _Pragma), and according to
> the online docs GCC 4 has already supported -Wsystem-headers.

I think it should be OK to limit to GCC 4.8 and clang 3.5 (both support
-Wsystem-headers).

The patch look good to me, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
>  misc/sys/cdefs.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index 393d9091..d832d5c3 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -649,6 +649,21 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
>  # define __glibc_macro_warning(msg)
>  #endif
>  
> +/* __glibc_warn_system_headers_begin starts a block of code where warnings
> +   produced by expanding macros defined in system headers will *not* be
> +   supressed.  __glibc_warn_system_headers_end ends such a block.  */
> +#if __GNUC_PREREQ (4,8) || __glibc_clang_prereq (3,5)
> +# define __glibc_warn_system_headers1(message) _Pragma (#message)
> +# define __glibc_warn_system_headers_begin \
> +  __glibc_warn_system_headers1 (GCC diagnostic push) \
> +  __glibc_warn_system_headers1 (GCC diagnostic warning "-Wsystem-headers")
> +# define __glibc_warn_system_headers_end \
> +  __glibc_warn_system_headers1 (GCC diagnostic pop)
> +#else
> +# define __glibc_warn_system_headers_begin
> +# define __glibc_warn_system_headers_end
> +#endif
> +
>  /* Generic selection (ISO C11) is a C-only feature, available in GCC
>     since version 4.9.  Previous versions do not provide generic
>     selection, even though they might set __STDC_VERSION__ to 201112L,
  

Patch

diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index 393d9091..d832d5c3 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -649,6 +649,21 @@  _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 # define __glibc_macro_warning(msg)
 #endif
 
+/* __glibc_warn_system_headers_begin starts a block of code where warnings
+   produced by expanding macros defined in system headers will *not* be
+   supressed.  __glibc_warn_system_headers_end ends such a block.  */
+#if __GNUC_PREREQ (4,8) || __glibc_clang_prereq (3,5)
+# define __glibc_warn_system_headers1(message) _Pragma (#message)
+# define __glibc_warn_system_headers_begin \
+  __glibc_warn_system_headers1 (GCC diagnostic push) \
+  __glibc_warn_system_headers1 (GCC diagnostic warning "-Wsystem-headers")
+# define __glibc_warn_system_headers_end \
+  __glibc_warn_system_headers1 (GCC diagnostic pop)
+#else
+# define __glibc_warn_system_headers_begin
+# define __glibc_warn_system_headers_end
+#endif
+
 /* Generic selection (ISO C11) is a C-only feature, available in GCC
    since version 4.9.  Previous versions do not provide generic
    selection, even though they might set __STDC_VERSION__ to 201112L,