[1/3] kbuild: add macro for controlling warnings to linux/compiler.h

Commit Message

Masahiro Yamada June 19, 2018, 5:34 p.m. UTC

Hi.

2018-06-16 9:53 GMT+09:00 Paul Burton <[e-mail]>:
> From: Arnd Bergmann <[e-mail]>
>
> 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.
>
> [[e-mail]:
>   - Rebase atop current master.
>   - Add __diag_GCC, or more generally __diag_<compiler>, abstraction to
>     avoid code outside of linux/compiler-gcc.h needing to duplicate
>     knowledge about different GCC versions.
>   - Add a comment argument to __diag_{ignore,warn,error} which isn't
>     used in the expansion of the macros but serves to push people to
>     document the reason for using them - per feedback from Kees Cook.]
>
> Signed-off-by: Arnd Bergmann <[e-mail]>
> Signed-off-by: Paul Burton <[e-mail]>
> Cc: Michal Marek <[e-mail]>
> Cc: Masahiro Yamada <[e-mail]>
> Cc: Douglas Anderson <[e-mail]>
> Cc: Al Viro <[e-mail]>
> Cc: Heiko Carstens <[e-mail]>
> Cc: Mauro Carvalho Chehab <[e-mail]>
> Cc: Matthew Wilcox <[e-mail]>
> Cc: Matthias Kaehlcke <[e-mail]>
> Cc: Arnd Bergmann <[e-mail]>
> Cc: Ingo Molnar <[e-mail]>
> Cc: Josh Poimboeuf <[e-mail]>
> Cc: Kees Cook <[e-mail]>
> Cc: Andrew Morton <[e-mail]>
> Cc: Thomas Gleixner <[e-mail]>
> Cc: Gideon Israel Dsouza <[e-mail]>
> Cc: Christophe Leroy <[e-mail]>
> Cc: Benjamin Herrenschmidt <[e-mail]>
> Cc: Paul Mackerras <[e-mail]>
> Cc: Michael Ellerman <[e-mail]>
> Cc: Stafford Horne <[e-mail]>
> Cc: Khem Raj <[e-mail]>
> Cc: He Zhe <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
>
>  include/linux/compiler-gcc.h   | 66 ++++++++++++++++++++++++++++++++++
>  include/linux/compiler_types.h | 18 ++++++++++
>  2 files changed, 84 insertions(+)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index f1a7492a5cc8..aba64a2912d8 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -347,3 +347,69 @@
>  #if GCC_VERSION >= 50100
>  #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
>  #endif
> +
> +/*
> + * turn individual warnings and errors on and off locally, depending
> + * on version.
> + */
> +#define __diag_GCC(version, s) __diag_GCC_ ## version(s)
> +
> +#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


Hmm, we would have to add this for every release.



> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 6b79a9bba9a7..313a2ad884e0 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,4 +271,22 @@ 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
> +
> +#ifndef __diag_GCC
> +#define __diag_GCC(string)
> +#endif




__diag_GCC() takes two arguments,
so it should be:

#ifndef __diag_GCC
#define __diag_GCC(version, s)
#endif


Otherwise, this would cause warning like this:


arch/arm64/kernel/sys.c:40:1: error: macro "__diag_GCC" passed 2
arguments, but takes just 1
 SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
 ^~~~~~~~~~







> +#define __diag_push()  __diag(push)
> +#define __diag_pop()   __diag(pop)
> +
> +#define __diag_ignore(compiler, version, option, comment) \
> +       __diag_ ## compiler(version, ignored option)
> +#define __diag_warn(compiler, version, option, comment) \
> +       __diag_ ## compiler(version, warning option)
> +#define __diag_error(compiler, version, option, comment) \
> +       __diag_ ## compiler(version, error   option)
> +


To me, it looks like this is putting GCC/Clang specific things
in the common file, <linux/compiler_types.h> .

All compilers must use "ignored", "warning", "error",
not allowed to use "ignore".



I also wonder if we could avoid proliferating __diag_GCC_*.



>  #endif /* __LINUX_COMPILER_TYPES_H */
> --
> 2.17.1
>


I attached a bit different implementation below.

I used -Wno-pragmas to avoid unknown option warnings.




+
 #endif /* __LINUX_COMPILER_TYPES_H */





Usage is

       __diag_push();
       __diag_ignore(-Wattribute-alias,
                     "Type aliasing is used to sanitize syscall arguments");
              ...
       __diag_pop();




Comments, ideas are appreciated.

Comments

Paul Burton June 19, 2018, 7:02 p.m. UTC | #1

Hi Masahiro,

On Wed, Jun 20, 2018 at 02:34:35AM +0900, Masahiro Yamada wrote:
> 2018-06-16 9:53 GMT+09:00 Paul Burton <[e-mail]>:
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index f1a7492a5cc8..aba64a2912d8 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -347,3 +347,69 @@
> >  #if GCC_VERSION >= 50100
> >  #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> >  #endif
> > +
> > +/*
> > + * turn individual warnings and errors on and off locally, depending
> > + * on version.
> > + */
> > +#define __diag_GCC(version, s) __diag_GCC_ ## version(s)
> > +
> > +#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
> 
> 
> Hmm, we would have to add this for every release.

Well, strictly speaking only ones that we need to modify diags for - ie.
in this series we could get away with only adding the GCC 8 macro if we
wanted.

> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index 6b79a9bba9a7..313a2ad884e0 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -271,4 +271,22 @@ 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
> > +
> > +#ifndef __diag_GCC
> > +#define __diag_GCC(string)
> > +#endif
> 
> __diag_GCC() takes two arguments,
> so it should be:
> 
> #ifndef __diag_GCC
> #define __diag_GCC(version, s)
> #endif
> 
> 
> Otherwise, this would cause warning like this:
> 
> 
> arch/arm64/kernel/sys.c:40:1: error: macro "__diag_GCC" passed 2
> arguments, but takes just 1
>  SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
>  ^~~~~~~~~~

Yes, good catch.

> > +#define __diag_push()  __diag(push)
> > +#define __diag_pop()   __diag(pop)
> > +
> > +#define __diag_ignore(compiler, version, option, comment) \
> > +       __diag_ ## compiler(version, ignored option)
> > +#define __diag_warn(compiler, version, option, comment) \
> > +       __diag_ ## compiler(version, warning option)
> > +#define __diag_error(compiler, version, option, comment) \
> > +       __diag_ ## compiler(version, error   option)
> > +
> 
> To me, it looks like this is putting GCC/Clang specific things
> in the common file, <linux/compiler_types.h> .
> 
> All compilers must use "ignored", "warning", "error",
> not allowed to use "ignore".

We could move that to linux/compiler-gcc.h pretty easily.

> I also wonder if we could avoid proliferating __diag_GCC_*.

My thought is that it's unlikely we'll ever support enough different
compilers for it to become problematic to list the ones we modify
warnings for in linux/compiler_types.h.

> I attached a bit different implementation below.
> 
> I used -Wno-pragmas to avoid unknown option warnings.

That doesn't seem very clean to me because it will hide typos or other
mistakes. One advantage of Arnd's patch is that by specifying the
compiler & version we only attempt to use pragmas that are appropriate
so we don't need to ignore unknown ones.

> Usage is
> 
>        __diag_push();
>        __diag_ignore(-Wattribute-alias,
>                      "Type aliasing is used to sanitize syscall arguments");
>               ...
>        __diag_pop();
> 
> Comments, ideas are appreciated.

By removing the compiler & version arguments you're enforcing that if we
ignore a warning for one compiler we must ignore it for all, regardless
of whether it's problematic. This essentially presumes that warnings
with the same name will behave the same across compilers, which feels
worse to me than having users of this explicitly state which compilers
need the pragmas.

Thanks,
    Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Burton June 19, 2018, 8:14 p.m. UTC | #2

This series introduces infrastructure allowing compiler diagnostics to
be disabled or their severity modified for specific pieces of code, with
suitable abstractions to prevent that code from becoming tied to a
specific compiler.

This infrastructure is then used to disable the -Wattribute-alias
warning around syscall definitions, which rely on type mismatches to
sanitize arguments.

Finally PowerPC-specific #pragma's are removed now that the generic code
is handling this.

The series takes Arnd's RFC patches & addresses the review comments they
received. The most notable effect of this series to to avoid warnings &
build failures caused by -Wattribute-alias when compiling the kernel
with GCC 8.

Applies cleanly atop v4.18-rc1.

Thanks,
    Paul

Arnd Bergmann (2):
  kbuild: add macro for controlling warnings to linux/compiler.h
  disable -Wattribute-alias warning for SYSCALL_DEFINEx()

Paul Burton (1):
  powerpc: Remove -Wattribute-alias pragmas

 arch/powerpc/kernel/pci_32.c    |  4 ----
 arch/powerpc/kernel/pci_64.c    |  4 ----
 arch/powerpc/kernel/rtas.c      |  4 ----
 arch/powerpc/kernel/signal_32.c |  8 --------
 arch/powerpc/kernel/signal_64.c |  4 ----
 arch/powerpc/kernel/syscalls.c  |  4 ----
 arch/powerpc/mm/subpage-prot.c  |  4 ----
 include/linux/compat.h          |  8 +++++++-
 include/linux/compiler-gcc.h    | 27 +++++++++++++++++++++++++++
 include/linux/compiler_types.h  | 18 ++++++++++++++++++
 include/linux/syscalls.h        |  4 ++++
 11 files changed, 56 insertions(+), 33 deletions(-)

Masahiro Yamada June 20, 2018, 11:21 p.m. UTC | #3

2018-06-20 4:02 GMT+09:00 Paul Burton <[e-mail]>:
> Hi Masahiro,
>
> On Wed, Jun 20, 2018 at 02:34:35AM +0900, Masahiro Yamada wrote:
>> 2018-06-16 9:53 GMT+09:00 Paul Burton <[e-mail]>:
>> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> > index f1a7492a5cc8..aba64a2912d8 100644
>> > --- a/include/linux/compiler-gcc.h
>> > +++ b/include/linux/compiler-gcc.h
>> > @@ -347,3 +347,69 @@
>> >  #if GCC_VERSION >= 50100
>> >  #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
>> >  #endif
>> > +
>> > +/*
>> > + * turn individual warnings and errors on and off locally, depending
>> > + * on version.
>> > + */
>> > +#define __diag_GCC(version, s) __diag_GCC_ ## version(s)
>> > +
>> > +#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
>>
>>
>> Hmm, we would have to add this for every release.
>
> Well, strictly speaking only ones that we need to modify diags for - ie.
> in this series we could get away with only adding the GCC 8 macro if we
> wanted.
>
>> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
>> > index 6b79a9bba9a7..313a2ad884e0 100644
>> > --- a/include/linux/compiler_types.h
>> > +++ b/include/linux/compiler_types.h
>> > @@ -271,4 +271,22 @@ 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
>> > +
>> > +#ifndef __diag_GCC
>> > +#define __diag_GCC(string)
>> > +#endif
>>
>> __diag_GCC() takes two arguments,
>> so it should be:
>>
>> #ifndef __diag_GCC
>> #define __diag_GCC(version, s)
>> #endif
>>
>>
>> Otherwise, this would cause warning like this:
>>
>>
>> arch/arm64/kernel/sys.c:40:1: error: macro "__diag_GCC" passed 2
>> arguments, but takes just 1
>>  SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
>>  ^~~~~~~~~~
>
> Yes, good catch.
>
>> > +#define __diag_push()  __diag(push)
>> > +#define __diag_pop()   __diag(pop)
>> > +
>> > +#define __diag_ignore(compiler, version, option, comment) \
>> > +       __diag_ ## compiler(version, ignored option)
>> > +#define __diag_warn(compiler, version, option, comment) \
>> > +       __diag_ ## compiler(version, warning option)
>> > +#define __diag_error(compiler, version, option, comment) \
>> > +       __diag_ ## compiler(version, error   option)
>> > +
>>
>> To me, it looks like this is putting GCC/Clang specific things
>> in the common file, <linux/compiler_types.h> .
>>
>> All compilers must use "ignored", "warning", "error",
>> not allowed to use "ignore".
>
> We could move that to linux/compiler-gcc.h pretty easily.
>
>> I also wonder if we could avoid proliferating __diag_GCC_*.
>
> My thought is that it's unlikely we'll ever support enough different
> compilers for it to become problematic to list the ones we modify
> warnings for in linux/compiler_types.h.
>
>> I attached a bit different implementation below.
>>
>> I used -Wno-pragmas to avoid unknown option warnings.
>
> That doesn't seem very clean to me because it will hide typos or other
> mistakes. One advantage of Arnd's patch is that by specifying the
> compiler & version we only attempt to use pragmas that are appropriate
> so we don't need to ignore unknown ones.
>
>> Usage is
>>
>>        __diag_push();
>>        __diag_ignore(-Wattribute-alias,
>>                      "Type aliasing is used to sanitize syscall arguments");
>>               ...
>>        __diag_pop();
>>
>> Comments, ideas are appreciated.
>
> By removing the compiler & version arguments you're enforcing that if we
> ignore a warning for one compiler we must ignore it for all, regardless
> of whether it's problematic. This essentially presumes that warnings
> with the same name will behave the same across compilers, which feels
> worse to me than having users of this explicitly state which compilers
> need the pragmas.



Fair enough.

V2 is good except one nit.
(I left a comment in it)


I can fix it up locally
if it is tedious to re-spin, though.





> Thanks,
>     Paul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Paul Burton June 21, 2018, 12:02 a.m. UTC | #4

Hi Masahiro,

On Thu, Jun 21, 2018 at 08:21:01AM +0900, Masahiro Yamada wrote:
> V2 is good except one nit.
> (I left a comment in it)

Thanks, and yes I agree with your comment that the GCC<4.6 __diag()
definition can be removed.

> I can fix it up locally if it is tedious to re-spin, though.

If you wouldn't mind that'd be great :)

Thanks,
    Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Masahiro Yamada June 23, 2018, 8:40 a.m. UTC | #5

2018-06-20 5:14 GMT+09:00 Paul Burton <[e-mail]>:
> This series introduces infrastructure allowing compiler diagnostics to
> be disabled or their severity modified for specific pieces of code, with
> suitable abstractions to prevent that code from becoming tied to a
> specific compiler.
>
> This infrastructure is then used to disable the -Wattribute-alias
> warning around syscall definitions, which rely on type mismatches to
> sanitize arguments.
>
> Finally PowerPC-specific #pragma's are removed now that the generic code
> is handling this.
>
> The series takes Arnd's RFC patches & addresses the review comments they
> received. The most notable effect of this series to to avoid warnings &
> build failures caused by -Wattribute-alias when compiling the kernel
> with GCC 8.
>
> Applies cleanly atop v4.18-rc1.


Series, applied to linux-kbuild/fixes.
(since we need to fix warnings from GCC 8.1)


Thanks!

Patch

diff --git a/Makefile b/Makefile
index ca2af1a..d610d81 100644
--- a/Makefile
+++ b/Makefile
@@ -817,6 +817,8 @@  KBUILD_CFLAGS   += $(call cc-option,-Werror=designated-init)
 # change __FILE__ to the relative path from the srctree
 KBUILD_CFLAGS  += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)

+KBUILD_CFLAGS   += $(call cc-option,-Wno-pragmas)
+
 # use the deterministic mode of AR if available
 KBUILD_ARFLAGS := $(call ar-option,D)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index f1a7492..3f9c1cc 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -3,6 +3,8 @@ 
 #error "Please don't include <linux/compiler-gcc.h> directly, include
<linux/compiler.h> instead."
 #endif

+#include <linux/stringify.h>
+
 /*
  * Common definitions for all gcc versions go here.
  */
@@ -259,6 +261,16 @@ 
  */
 #define __visible      __attribute__((externally_visible))

+/* turn individual warnings and errors on and off locally */
+#define __diag_gcc(s)  _Pragma(__stringify(GCC diagnostic s))
+
+#define __diag_push()  __diag_gcc(push)
+#define __diag_pop()   __diag_gcc(pop)
+
+#define __diag_ignore(option, comment) __diag_gcc(ignored __stringify(option))
+#define __diag_warn(option, comment)   __diag_gcc(warning __stringify(option))
+#define __diag_error(option, comment)  __diag_gcc(error __stringify(option))
+
 #endif /* GCC_VERSION >= 40600 */
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 6b79a9b..32e354f 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -271,4 +271,24 @@  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_push
+#define __diag_push()
+#endif
+
+#ifndef __diag_pop
+#define __diag_pop()
+#endif
+
+#ifndef __diag_ignore
+#define __diag_ignore(option, comment)
+#endif
+
+#ifndef __diag_warn
+#define __diag_warn(option, comment)
+#endif
+
+#ifndef __diag_error
+#define __diag_error(option, comment)
+#endif