Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752182AbdLETZT (ORCPT ); Tue, 5 Dec 2017 14:25:19 -0500 Received: from mail-vk0-f65.google.com ([209.85.213.65]:47031 "EHLO mail-vk0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751463AbdLETZP (ORCPT ); Tue, 5 Dec 2017 14:25:15 -0500 X-Google-Smtp-Source: AGs4zMaorxAyJ3VAObQtuRcXSaTBLEd2hSNXf99TxVrGtiVKY4idUuMvHbpYVFHw4z4MTennJDTDvmi80D6kI61XQJk= MIME-Version: 1.0 In-Reply-To: <20171205153259.2522368-1-arnd@arndb.de> References: <20171205153259.2522368-1-arnd@arndb.de> From: Kees Cook Date: Tue, 5 Dec 2017 11:25:13 -0800 X-Google-Sender-Auth: a6LVwLY7VeeU4QBBcctwkHi-DMw Message-ID: Subject: Re: [PATCH 1/2] [RFC] kbuild: add macro for controlling warnings to linux/compiler.h To: Arnd Bergmann Cc: linux-kbuild , Michal Marek , Masahiro Yamada , Douglas Anderson , Al Viro , Heiko Carstens , Mauro Carvalho Chehab , Matthew Wilcox , Matthias Kaehlcke , Ingo Molnar , Josh Poimboeuf , Andrew Morton , Thomas Gleixner , Gideon Israel Dsouza , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5991 Lines: 175 On Tue, Dec 5, 2017 at 7:32 AM, Arnd Bergmann 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 > --- > 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