[PATCH] Revert "Enable '-Werror' by default for all kernel builds"
ndesaulniers at google.com
Tue Sep 7 18:38:40 UTC 2021
This reverts commit 3fe617ccafd6f5bb33c2391d6f4eeb41c1fd0151.
The above commit seems as though it was merged in response to
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'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
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
Cc: Arnd Bergmann <arnd at arndb.de>
Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
Cc: Guenter Roeck <linux at roeck-us.net>
Cc: Kees Cook <keescook at chromium.org>
Cc: Mark Brown <broonie at kernel.org>
Cc: Masahiro Yamada <masahiroy at kernel.org>
Cc: Nathan Chancellor <nathan at kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers at google.com>
Makefile | 3 ---
init/Kconfig | 14 --------------
2 files changed, 17 deletions(-)
diff --git a/Makefile b/Makefile
index d45fc2edf186..6bc1c5b17a62 100644
@@ -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)
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
@@ -137,20 +137,6 @@ config COMPILE_TEST
here. If you are a user/distributor, say N here to exclude useless
drivers to be distributed.
- bool "Compile the kernel with warnings as errors"
- default y
- 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.
bool "Compile test UAPI headers"
depends on HEADERS_INSTALL && CC_CAN_LINK
More information about the Linux-security-module-archive