I have occasionally run into a situation where it would make sense to
control a compiler warning from a source file rather than doing so from
a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
helpers.
The approach here is similar to what glibc uses, using __diag() and
related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
that gets turned into the respective "#pragma GCC diagnostic ..." by
the preprocessor when the macro gets expanded.
Like glibc, I also have an argument to pass the affected compiler
version, but decided to actually evaluate that one. For now, this
supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
versions is straightforward here. GNU compilers starting with gcc-4.2
could support it in principle, but "#pragma GCC diagnostic push"
was only added in gcc-4.6, so it seems simpler to not deal with those
at all. The same versions show a large number of warnings already,
so it seems easier to just leave it at that and not do a more
fine-grained control for them.
The use cases I found so far include:
- turning off the gcc-8 -Wattribute-alias warning inside of the
SYSCALL_DEFINEx() macro without having to do it globally.
- Reducing the build time for a simple re-make after a change,
once we move the warnings from ./Makefile and
./scripts/Makefile.extrawarn into linux/compiler.h
- More control over the warnings based on other configurations,
using preprocessor syntax instead of Makefile syntax. This should make
it easier for the average developer to understand and change things.
- Adding an easy way to turn the W=1 option on unconditionally
for a subdirectory or a specific file. This has been requested
by several developers in the past that want to have their subsystems
W=1 clean.
- Integrating clang better into the build systems. Clang supports
more warnings than GCC, and we probably want to classify them
as default, W=1, W=2 etc, but there are cases in which the
warnings should be classified differently due to excessive false
positives from one or the other compiler.
- Adding a way to turn the default warnings into errors (e.g. using
a new "make E=0" tag) while not also turning the W=1 warnings into
errors.
This patch for now just adds the minimal infrastructure in order to
do the first of the list above. As the #pragma GCC diagnostic
takes precedence over command line options, the next step would be
to convert a lot of the individual Makefiles that set nonstandard
options to use __diag() instead.
Signed-off-by: Arnd Bergmann <[email protected]>
---
I'm marking this RFC for now, as I'd like to get consensus about
whether we want to go down this particular route first, and maybe
if someone can come up with better naming for the macros.
---
include/linux/compiler-gcc.h | 64 ++++++++++++++++++++++++++++++++++++++++++
include/linux/compiler_types.h | 10 +++++++
2 files changed, 74 insertions(+)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 2272ded07496..5d595cfdb2c4 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -329,3 +329,67 @@
* code
*/
#define uninitialized_var(x) x = x
+
+/*
+ * turn individual warnings and errors on and off locally, depending
+ * on version.
+ */
+#if GCC_VERSION >= 40600
+#define __diag_str1(s) #s
+#define __diag_str(s) __diag_str1(s)
+#define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
+
+/* compilers before gcc-4.6 do not understand "#pragma GCC diagnostic push" */
+#define __diag_GCC_4_6(s) __diag(s)
+#else
+#define __diag(s)
+#define __diag_GCC_4_6(s)
+#endif
+
+#if GCC_VERSION >= 40700
+#define __diag_GCC_4_7(s) __diag(s)
+#else
+#define __diag_GCC_4_7(s)
+#endif
+
+#if GCC_VERSION >= 40800
+#define __diag_GCC_4_8(s) __diag(s)
+#else
+#define __diag_GCC_4_8(s)
+#endif
+
+#if GCC_VERSION >= 40900
+#define __diag_GCC_4_9(s) __diag(s)
+#else
+#define __diag_GCC_4_9(s)
+#endif
+
+#if GCC_VERSION >= 50000
+#define __diag_GCC_5(s) __diag(s)
+#else
+#define __diag_GCC_5(s)
+#endif
+
+#if GCC_VERSION >= 60000
+#define __diag_GCC_6(s) __diag(s)
+#else
+#define __diag_GCC_6(s)
+#endif
+
+#if GCC_VERSION >= 70000
+#define __diag_GCC_7(s) __diag(s)
+#else
+#define __diag_GCC_7(s)
+#endif
+
+#if GCC_VERSION >= 80000
+#define __diag_GCC_8(s) __diag(s)
+#else
+#define __diag_GCC_8(s)
+#endif
+
+#if GCC_VERSION >= 90000
+#define __diag_GCC_9(s) __diag(s)
+#else
+#define __diag_GCC_9(s)
+#endif
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 6b79a9bba9a7..7e7664d57adb 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -271,4 +271,14 @@ struct ftrace_likely_data {
# define __native_word(t) (sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
#endif
+#ifndef __diag
+#define __diag(string)
+#endif
+
+#define __diag_push() __diag(push)
+#define __diag_ignore(version, option) __diag_ ## version (ignored option)
+#define __diag_warn(version, option) __diag_ ## version (warning option)
+#define __diag_error(version, option) __diag_ ## version (error option)
+#define __diag_pop() __diag(pop)
+
#endif /* __LINUX_COMPILER_TYPES_H */
--
2.9.0
gcc-8 warns for every single definition of a system call entry
point, e.g.:
include/linux/compat.h:56:18: error: 'compat_sys_rt_sigprocmask' alias between functions of incompatible types 'long int(int, compat_sigset_t *, compat_sigset_t *, compat_size_t)' {aka 'long int(int, struct <anonymous> *, struct <anonymous> *, unsigned int)'} and 'long int(long int, long int, long int, long int)' [-Werror=attribute-alias]
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
^~~~~~~~~~
include/linux/compat.h:45:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
^~~~~~~~~~~~~~~~~~~~~~
kernel/signal.c:2601:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE4'
COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, int, how, compat_sigset_t __user *, nset,
^~~~~~~~~~~~~~~~~~~~~~
include/linux/compat.h:60:18: note: aliased declaration here
asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
^~~~~~~~~~
The new warning seems reasonable in principle, but it doesn't
help us here, since we rely on the type mismatch to sanitize the
system call arguments. After I reported this as GCC PR82435, a new
-Wno-attribute-alias option was added that could be used to turn the
warning off globally on the command line, but I'd prefer to do it a
little more fine-grained.
Interestingly, turning a warning off and on again inside of
a single macro doesn't always work, in this case I had to add
an extra statement inbetween and decided to copy the __SC_TEST
one from the native syscall to the compat syscall macro. See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 for more details
about this.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82435
Signed-off-by: Arnd Bergmann <[email protected]>
---
include/linux/compat.h | 7 ++++++-
include/linux/syscalls.h | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 1165036d091f..27b4a429df77 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -49,14 +49,19 @@
COMPAT_SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
#define COMPAT_SYSCALL_DEFINEx(x, name, ...) \
+ __diag_push(); \
+ __diag_ignore(GCC_8, "-Wattribute-alias"); \
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
__attribute__((alias(__stringify(compat_SyS##name)))); \
static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));\
asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
{ \
- return C_SYSC##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__)); \
+ long ret = C_SYSC##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
+ __MAP(x,__SC_TEST,__VA_ARGS__); \
+ return ret; \
} \
+ __diag_pop(); \
static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))
#ifdef CONFIG_COMPAT
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 4df16a70b0d7..dcf6ceabda44 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -208,6 +208,8 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
#define __PROTECT(...) asmlinkage_protect(__VA_ARGS__)
#define __SYSCALL_DEFINEx(x, name, ...) \
+ __diag_push(); \
+ __diag_ignore(GCC_8, "-Wattribute-alias"); \
asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
__attribute__((alias(__stringify(SyS##name)))); \
static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__)); \
@@ -219,6 +221,7 @@ static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
__PROTECT(x, ret,__MAP(x,__SC_ARGS,__VA_ARGS__)); \
return ret; \
} \
+ __diag_pop(); \
static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))
/*
--
2.9.0
On Tue, Dec 5, 2017 at 7:32 AM, Arnd Bergmann <[email protected]> wrote:
> I have occasionally run into a situation where it would make sense to
> control a compiler warning from a source file rather than doing so from
> a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
> helpers.
>
> The approach here is similar to what glibc uses, using __diag() and
> related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
> that gets turned into the respective "#pragma GCC diagnostic ..." by
> the preprocessor when the macro gets expanded.
>
> Like glibc, I also have an argument to pass the affected compiler
> version, but decided to actually evaluate that one. For now, this
> supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
> GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
> versions is straightforward here. GNU compilers starting with gcc-4.2
> could support it in principle, but "#pragma GCC diagnostic push"
> was only added in gcc-4.6, so it seems simpler to not deal with those
> at all. The same versions show a large number of warnings already,
> so it seems easier to just leave it at that and not do a more
> fine-grained control for them.
>
> The use cases I found so far include:
>
> - turning off the gcc-8 -Wattribute-alias warning inside of the
> SYSCALL_DEFINEx() macro without having to do it globally.
>
> - Reducing the build time for a simple re-make after a change,
> once we move the warnings from ./Makefile and
> ./scripts/Makefile.extrawarn into linux/compiler.h
>
> - More control over the warnings based on other configurations,
> using preprocessor syntax instead of Makefile syntax. This should make
> it easier for the average developer to understand and change things.
>
> - Adding an easy way to turn the W=1 option on unconditionally
> for a subdirectory or a specific file. This has been requested
> by several developers in the past that want to have their subsystems
> W=1 clean.
>
> - Integrating clang better into the build systems. Clang supports
> more warnings than GCC, and we probably want to classify them
> as default, W=1, W=2 etc, but there are cases in which the
> warnings should be classified differently due to excessive false
> positives from one or the other compiler.
>
> - Adding a way to turn the default warnings into errors (e.g. using
> a new "make E=0" tag) while not also turning the W=1 warnings into
> errors.
>
> This patch for now just adds the minimal infrastructure in order to
> do the first of the list above. As the #pragma GCC diagnostic
> takes precedence over command line options, the next step would be
> to convert a lot of the individual Makefiles that set nonstandard
> options to use __diag() instead.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> I'm marking this RFC for now, as I'd like to get consensus about
> whether we want to go down this particular route first, and maybe
> if someone can come up with better naming for the macros.
I like this. I wonder if it would be a good idea to add an additional
argument that forces documentation of the reason for adding a diag
marking? Something like:
__diag_warn(GCC_7, vla, "No VLAs should be used in this code");
-Kees
> ---
> include/linux/compiler-gcc.h | 64 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/compiler_types.h | 10 +++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 2272ded07496..5d595cfdb2c4 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -329,3 +329,67 @@
> * code
> */
> #define uninitialized_var(x) x = x
> +
> +/*
> + * turn individual warnings and errors on and off locally, depending
> + * on version.
> + */
> +#if GCC_VERSION >= 40600
> +#define __diag_str1(s) #s
> +#define __diag_str(s) __diag_str1(s)
> +#define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
> +
> +/* compilers before gcc-4.6 do not understand "#pragma GCC diagnostic push" */
> +#define __diag_GCC_4_6(s) __diag(s)
> +#else
> +#define __diag(s)
> +#define __diag_GCC_4_6(s)
> +#endif
> +
> +#if GCC_VERSION >= 40700
> +#define __diag_GCC_4_7(s) __diag(s)
> +#else
> +#define __diag_GCC_4_7(s)
> +#endif
> +
> +#if GCC_VERSION >= 40800
> +#define __diag_GCC_4_8(s) __diag(s)
> +#else
> +#define __diag_GCC_4_8(s)
> +#endif
> +
> +#if GCC_VERSION >= 40900
> +#define __diag_GCC_4_9(s) __diag(s)
> +#else
> +#define __diag_GCC_4_9(s)
> +#endif
> +
> +#if GCC_VERSION >= 50000
> +#define __diag_GCC_5(s) __diag(s)
> +#else
> +#define __diag_GCC_5(s)
> +#endif
> +
> +#if GCC_VERSION >= 60000
> +#define __diag_GCC_6(s) __diag(s)
> +#else
> +#define __diag_GCC_6(s)
> +#endif
> +
> +#if GCC_VERSION >= 70000
> +#define __diag_GCC_7(s) __diag(s)
> +#else
> +#define __diag_GCC_7(s)
> +#endif
> +
> +#if GCC_VERSION >= 80000
> +#define __diag_GCC_8(s) __diag(s)
> +#else
> +#define __diag_GCC_8(s)
> +#endif
> +
> +#if GCC_VERSION >= 90000
> +#define __diag_GCC_9(s) __diag(s)
> +#else
> +#define __diag_GCC_9(s)
> +#endif
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 6b79a9bba9a7..7e7664d57adb 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,4 +271,14 @@ struct ftrace_likely_data {
> # define __native_word(t) (sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
> #endif
>
> +#ifndef __diag
> +#define __diag(string)
> +#endif
> +
> +#define __diag_push() __diag(push)
> +#define __diag_ignore(version, option) __diag_ ## version (ignored option)
> +#define __diag_warn(version, option) __diag_ ## version (warning option)
> +#define __diag_error(version, option) __diag_ ## version (error option)
> +#define __diag_pop() __diag(pop)
> +
> #endif /* __LINUX_COMPILER_TYPES_H */
> --
> 2.9.0
>
--
Kees Cook
Pixel Security
On Tue, Dec 5, 2017 at 8:25 PM, Kees Cook <[email protected]> wrote:
> I like this. I wonder if it would be a good idea to add an additional
> argument that forces documentation of the reason for adding a diag
> marking? Something like:
>
> __diag_warn(GCC_7, vla, "No VLAs should be used in this code");
This would be similar to what glibc does, it names the (ignore) macro
DIAG_IGNORE_NEEDS_COMMENT(), and by convention requires
a comment block in front of it. Not sure if it will actually work in the kernel
where the reviews are much more scattered across subsystem maintainers,
but we could try it.
Arnd
Hi Arnd,
2017-12-06 0:32 GMT+09:00 Arnd Bergmann <[email protected]>:
> I have occasionally run into a situation where it would make sense to
> control a compiler warning from a source file rather than doing so from
> a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
> helpers.
>
> The approach here is similar to what glibc uses, using __diag() and
> related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
> that gets turned into the respective "#pragma GCC diagnostic ..." by
> the preprocessor when the macro gets expanded.
>
> Like glibc, I also have an argument to pass the affected compiler
> version, but decided to actually evaluate that one. For now, this
> supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
> GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
> versions is straightforward here. GNU compilers starting with gcc-4.2
> could support it in principle, but "#pragma GCC diagnostic push"
> was only added in gcc-4.6, so it seems simpler to not deal with those
> at all. The same versions show a large number of warnings already,
> so it seems easier to just leave it at that and not do a more
> fine-grained control for them.
>
> The use cases I found so far include:
>
> - turning off the gcc-8 -Wattribute-alias warning inside of the
> SYSCALL_DEFINEx() macro without having to do it globally.
>
> - Reducing the build time for a simple re-make after a change,
> once we move the warnings from ./Makefile and
> ./scripts/Makefile.extrawarn into linux/compiler.h
Do you mean, list default warnings in linux/compiler.h
like
__diag_ignore(GCC_*, "-Wunused-but-set-variable");
__diag_ignore(GCC_*, "-Wunused-const-variable");
instead of
KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable)
KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
Is this what you mean?
I wonder if we should for all files to include <linux/compiler.h>...
> - More control over the warnings based on other configurations,
> using preprocessor syntax instead of Makefile syntax. This should make
> it easier for the average developer to understand and change things.
>
> - Adding an easy way to turn the W=1 option on unconditionally
> for a subdirectory or a specific file. This has been requested
> by several developers in the past that want to have their subsystems
> W=1 clean.
>
> - Integrating clang better into the build systems. Clang supports
> more warnings than GCC, and we probably want to classify them
> as default, W=1, W=2 etc, but there are cases in which the
> warnings should be classified differently due to excessive false
> positives from one or the other compiler.
>
> - Adding a way to turn the default warnings into errors (e.g. using
> a new "make E=0" tag) while not also turning the W=1 warnings into
> errors.
>
> This patch for now just adds the minimal infrastructure in order to
> do the first of the list above. As the #pragma GCC diagnostic
> takes precedence over command line options, the next step would be
> to convert a lot of the individual Makefiles that set nonstandard
> options to use __diag() instead.
GCC manual says:
Note that not all diagnostics are modifiable; at the moment only warnings
(normally controlled by ‘-W…’) can be controlled, and not all of them.
https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
Actually, my compiler does not react
to __diag_*(GCC_*, "-Wunused-but-set-variable") at all.
Is it possible to replace command line flags?
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> I'm marking this RFC for now, as I'd like to get consensus about
> whether we want to go down this particular route first, and maybe
> if someone can come up with better naming for the macros.
> ---
> include/linux/compiler-gcc.h | 64 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/compiler_types.h | 10 +++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 2272ded07496..5d595cfdb2c4 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -329,3 +329,67 @@
> * code
> */
> #define uninitialized_var(x) x = x
> +
> +/*
> + * turn individual warnings and errors on and off locally, depending
> + * on version.
> + */
> +#if GCC_VERSION >= 40600
> +#define __diag_str1(s) #s
> +#define __diag_str(s) __diag_str1(s)
> +#define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
> +
> +/* compilers before gcc-4.6 do not understand "#pragma GCC diagnostic push" */
> +#define __diag_GCC_4_6(s) __diag(s)
> +#else
> +#define __diag(s)
> +#define __diag_GCC_4_6(s)
> +#endif
> +
> +#if GCC_VERSION >= 40700
> +#define __diag_GCC_4_7(s) __diag(s)
> +#else
> +#define __diag_GCC_4_7(s)
> +#endif
> +
> +#if GCC_VERSION >= 40800
> +#define __diag_GCC_4_8(s) __diag(s)
> +#else
> +#define __diag_GCC_4_8(s)
> +#endif
> +
> +#if GCC_VERSION >= 40900
> +#define __diag_GCC_4_9(s) __diag(s)
> +#else
> +#define __diag_GCC_4_9(s)
> +#endif
> +
> +#if GCC_VERSION >= 50000
> +#define __diag_GCC_5(s) __diag(s)
> +#else
> +#define __diag_GCC_5(s)
> +#endif
> +
> +#if GCC_VERSION >= 60000
> +#define __diag_GCC_6(s) __diag(s)
> +#else
> +#define __diag_GCC_6(s)
> +#endif
> +
> +#if GCC_VERSION >= 70000
> +#define __diag_GCC_7(s) __diag(s)
> +#else
> +#define __diag_GCC_7(s)
> +#endif
> +
> +#if GCC_VERSION >= 80000
> +#define __diag_GCC_8(s) __diag(s)
> +#else
> +#define __diag_GCC_8(s)
> +#endif
> +
> +#if GCC_VERSION >= 90000
> +#define __diag_GCC_9(s) __diag(s)
> +#else
> +#define __diag_GCC_9(s)
> +#endif
For linux/compiler-intel.h case,
these macros are not defined at all,
so __diag_ignore(GCC_*, ...) will cause compile error.
Clang includes linux/compiler-gcc.h as well,
so it should be OK.
Interestingly, clang also defines GCC version,
but I have no idea the correspondence between
gcc version vs. clang version.
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 6b79a9bba9a7..7e7664d57adb 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,4 +271,14 @@ struct ftrace_likely_data {
> # define __native_word(t) (sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
> #endif
>
> +#ifndef __diag
> +#define __diag(string)
> +#endif
> +
> +#define __diag_push() __diag(push)
> +#define __diag_ignore(version, option) __diag_ ## version (ignored option)
> +#define __diag_warn(version, option) __diag_ ## version (warning option)
> +#define __diag_error(version, option) __diag_ ## version (error option)
> +#define __diag_pop() __diag(pop)
> +
> #endif /* __LINUX_COMPILER_TYPES_H */
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
On Mon, Dec 18, 2017 at 10:48 AM, Masahiro Yamada
<[email protected]> wrote:
> 2017-12-06 0:32 GMT+09:00 Arnd Bergmann <[email protected]>:
>> I have occasionally run into a situation where it would make sense to
>> control a compiler warning from a source file rather than doing so from
>> a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
>> helpers.
>>
>> The approach here is similar to what glibc uses, using __diag() and
>> related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
>> that gets turned into the respective "#pragma GCC diagnostic ..." by
>> the preprocessor when the macro gets expanded.
>>
>> Like glibc, I also have an argument to pass the affected compiler
>> version, but decided to actually evaluate that one. For now, this
>> supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
>> GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
>> versions is straightforward here. GNU compilers starting with gcc-4.2
>> could support it in principle, but "#pragma GCC diagnostic push"
>> was only added in gcc-4.6, so it seems simpler to not deal with those
>> at all. The same versions show a large number of warnings already,
>> so it seems easier to just leave it at that and not do a more
>> fine-grained control for them.
>>
>> The use cases I found so far include:
>>
>> - turning off the gcc-8 -Wattribute-alias warning inside of the
>> SYSCALL_DEFINEx() macro without having to do it globally.
>>
>> - Reducing the build time for a simple re-make after a change,
>> once we move the warnings from ./Makefile and
>> ./scripts/Makefile.extrawarn into linux/compiler.h
>
> Do you mean, list default warnings in linux/compiler.h
>
> like
>
> __diag_ignore(GCC_*, "-Wunused-but-set-variable");
> __diag_ignore(GCC_*, "-Wunused-const-variable");
>
> instead of
>
> KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable)
> KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
>
> Is this what you mean?
In principle yes, though my plan was a bit more sophisticated than this:
I want to make those definitions specific to both the W= level and the compiler
version, so you can say things like
KBUILD_WARN(0, GCC_4_6, "-Wall")
KBUILD_WARN(0, CLANG5, "-Wall")
KBUILD_WARN(1, GCC_4_6, "-Wextra")
KBUILD_WARN(1, CLANG5, "-Wextra")
KBUILD_WARN(2, CLANG5, "-Wmost")
KBUILD_WARN(3, CLANG5, "-Weverything")
With the above, "make W=12 E=01" should would lead to
-Wall and -Wextra warnings to become errors on all compilers,
but also enable -Wmost as warnings rather than errors on clang.
> I wonder if we should for all files to include <linux/compiler.h>...
I assumed we already did that, but maybe I was wrong here. I see
that we include include/linux/kconfig.h everywhere, but I don't see
that for compiler.h. This would obviously be a requirement to do first.
>> This patch for now just adds the minimal infrastructure in order to
>> do the first of the list above. As the #pragma GCC diagnostic
>> takes precedence over command line options, the next step would be
>> to convert a lot of the individual Makefiles that set nonstandard
>> options to use __diag() instead.
>
>
> GCC manual says:
> Note that not all diagnostics are modifiable; at the moment only warnings
> (normally controlled by ‘-W…’) can be controlled, and not all of them.
>
> https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
>
>
> Actually, my compiler does not react
> to __diag_*(GCC_*, "-Wunused-but-set-variable") at all.
>
> Is it possible to replace command line flags?
My interpretation was that everything that had a -W option could be controlled
with a pragma as well. While we can turn any warning into an error, we cannot
turn most errors into warnings.
When I experimented with this a couple of months ago, I also saw that
"gcc -Q --help=warnings" reports some warnings that are language
specific, and it refuses to enable e.g. a fortran specific warning when
building a C file.
> For linux/compiler-intel.h case,
> these macros are not defined at all,
> so __diag_ignore(GCC_*, ...) will cause compile error.
Right, that would need to be fixed. I think it's ok to just ignore all
those statements and just continue with the default warnings.
> Clang includes linux/compiler-gcc.h as well,
> so it should be OK.
>
> Interestingly, clang also defines GCC version,
> but I have no idea the correspondence between
> gcc version vs. clang version.
I tried clang-3.9 and clang-5, both of which report compatibility with
gcc-4.2.1. It seems that for the most part, clang tries to implement
the gcc warning options, but there are always some it does not have.
I can probably come up with a list of warning options that all
supported versions of either gcc or clang understand, and have
those set independent of the version, but for all others require
listing the specific version that introduces it. In some cases a
warning gets introduced in one version but isn't actually usable
until a later version.
Arnd
On Tue, Dec 05, 2017 at 05:06:31PM +0100, Arnd Bergmann wrote:
> gcc-8 warns for every single definition of a system call entry
> point, e.g.:
>
> include/linux/compat.h:56:18: error: 'compat_sys_rt_sigprocmask' alias between functions of incompatible types 'long int(int, compat_sigset_t *, compat_sigset_t *, compat_size_t)' {aka 'long int(int, struct <anonymous> *, struct <anonymous> *, unsigned int)'} and 'long int(long int, long int, long int, long int)' [-Werror=attribute-alias]
> asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
> ^~~~~~~~~~
> include/linux/compat.h:45:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
> COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
> ^~~~~~~~~~~~~~~~~~~~~~
> kernel/signal.c:2601:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE4'
> COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, int, how, compat_sigset_t __user *, nset,
> ^~~~~~~~~~~~~~~~~~~~~~
> include/linux/compat.h:60:18: note: aliased declaration here
> asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
> ^~~~~~~~~~
>
> The new warning seems reasonable in principle, but it doesn't
> help us here, since we rely on the type mismatch to sanitize the
> system call arguments. After I reported this as GCC PR82435, a new
> -Wno-attribute-alias option was added that could be used to turn the
> warning off globally on the command line, but I'd prefer to do it a
> little more fine-grained.
>
> Interestingly, turning a warning off and on again inside of
> a single macro doesn't always work, in this case I had to add
> an extra statement inbetween and decided to copy the __SC_TEST
> one from the native syscall to the compat syscall macro. See
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 for more details
> about this.
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82435
> Signed-off-by: Arnd Bergmann <[email protected]>
Hi Arnd,
What ever happened to this patch? I am compiline the kernel with the new
OpenRISC GCC backend port https://github.com/stffrdhrn/gcc/commits/or1k-port,
which is based on gcc 9.0.0 and seeing similar 'alias between functions of
incompatible types' warnings.
I found this old patch which seems will fix the issue, but it seems to not have
been merged. Did we find another solution?
Case in point, when I build x86 64 on gcc 8.1.1, I am not seeing the warnings.
Is this not an issue for x86?
-Stafford