On Tue, Sep 7, 2021 at 11:39 AM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote: > > This reverts commit 3fe617ccafd6f5bb33c2391d6f4eeb41c1fd0151. > > The above commit seems as though it was merged in response to > https://lore.kernel.org/linux-hardening/CAHk-=wj4EG=kCOaqyPEq5VXa97kyUHsBpBn3DWwE91qcnDytOQ@xxxxxxxxxxxxxx/. > > While I can appreciate the intent of enabling -Werror, I don't think it > is the right tool to address the root cause of developers not testing > certain toolchains or configurations, or taking existing reports they're > getting serious enough. > > Having more appropriate CI or processes in place to prevent untested > patches from being merged into mainline may also be worth discussing. I agree that -Werror by default needs more discussion. Default WERROR makes building old kernels with new compilers more painful. CI systems could do a better job surfacing compiler warnings if they don't do it currently. > I'd also like to see such a patch sent formally to the list for > discussion and have time to soak in next rather than be merged directly > into mainline without either. > > -Werror is great for preventing new errors from creeping in when a > codebase is free of warnings for all configs and all targets and the > toolchain is never updated. Unfortunately, none of the above is the case > for the Linux kernel at this time. > > The addition of new compiler diagnostic flags in the -W group to -Wall > make toolchain updates excessively more painful. This can lead to > commits that disable warnings rather than work towards addressing them. > Some diagnostics are useful but take incredible work or churn to > completely free a codebase from them. > > Warning can be upgraded to errors with -Werror=foo or downgraded from > errors back to warnings via -Wno-error=foo. -Wno-error=foo is a double > edged sword; it doesn't help you spot the introduction of additional > instances of that warning easily. > > This change has caused nearly all of our CI to go red, and requires us > to now disable CONFIG_WERROR until every last target and every last > config is addressed. Rather than require everyone to disable the above > config to keep builds going, perhaps certain CI systems should instead > set CFLAGS_KERNEL=-Werror. > > Why don't we just fix every warning? We have been, for years, and we're > still not done yet. See our issue tracker below, contributors wanted. > > With more time/active discussion, we can probably land something more > appropriate. It should involve the Kbuild maintainer and list. > > For instance, I have questions around how should such a config interact > with randconfigs and allconfigs. This config also seems to duplicate the > existing CONFIG_PPC_DISABLE_WERROR without merging the two. > > I do recognize the irony of someone who's spent a lot of time cleaning > up warnings to be advocating for disabling -Werror...it's not lost on > me. Our Pixel (née Nexus) team has been effectively carrying an out of > tree patch enabling -Werror since before I ever contributed to the > kernel. > > Cc: Arnd Bergmann <arnd@xxxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > Cc: Mark Brown <broonie@xxxxxxxxxx> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > Link: https://github.com/ClangBuiltLinux/linux/issues/1449 > Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > --- > Makefile | 3 --- > init/Kconfig | 14 -------------- > 2 files changed, 17 deletions(-) > > diff --git a/Makefile b/Makefile > index d45fc2edf186..6bc1c5b17a62 100644 > --- a/Makefile > +++ b/Makefile > @@ -785,9 +785,6 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong > > KBUILD_CFLAGS += $(stackp-flags-y) > > -KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror > -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) > - > ifdef CONFIG_CC_IS_CLANG > KBUILD_CPPFLAGS += -Qunused-arguments > # The kernel builds with '-std=gnu89' so use of GNU extensions is acceptable. > diff --git a/init/Kconfig b/init/Kconfig > index 8cb97f141b70..e708180e9a59 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -137,20 +137,6 @@ config COMPILE_TEST > here. If you are a user/distributor, say N here to exclude useless > drivers to be distributed. > > -config WERROR > - bool "Compile the kernel with warnings as errors" > - default y > - help > - A kernel build should not cause any compiler warnings, and this > - enables the '-Werror' flag to enforce that rule by default. > - > - However, if you have a new (or very old) compiler with odd and > - unusual warnings, or you have some architecture with problems, > - you may need to disable this config option in order to > - successfully build the kernel. > - > - If in doubt, say Y. > - > config UAPI_HEADER_TEST > bool "Compile test UAPI headers" > depends on HEADERS_INSTALL && CC_CAN_LINK > > base-commit: 4b93c544e90e2b28326182d31ee008eb80e02074 > -- > 2.33.0.153.gba50c8fa24-goog > -- 宋方睿
