* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:14 ` Marco Elver
@ 2021-09-07 22:18 ` Linus Torvalds
2021-09-07 22:33 ` Randy Dunlap
2021-09-07 22:28 ` Guenter Roeck
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Linus Torvalds @ 2021-09-07 22:18 UTC (permalink / raw)
To: Marco Elver
Cc: Nick Desaulniers, llvm, LSM List, linux-toolchains,
Arnd Bergmann, Greg Kroah-Hartman, Guenter Roeck, Kees Cook,
Mark Brown, Masahiro Yamada, Nathan Chancellor, Michal Marek,
Andrew Morton, Vipin Sharma, Chris Down, Rasmus Villemoes,
Daniel Borkmann, Vlastimil Babka, Linux Kbuild mailing list,
Linux Kernel Mailing List
On Tue, Sep 7, 2021 at 3:14 PM Marco Elver <elver@google.com> wrote:
>
>
> config WERROR
> bool "Compile the kernel with warnings as errors"
> - default y
> + default COMPILE_TEST
That seems reasonable. It very much is about build-testing.
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:18 ` Linus Torvalds
@ 2021-09-07 22:33 ` Randy Dunlap
2021-09-13 9:32 ` Pavel Machek
0 siblings, 1 reply; 21+ messages in thread
From: Randy Dunlap @ 2021-09-07 22:33 UTC (permalink / raw)
To: Linus Torvalds, Marco Elver
Cc: Nick Desaulniers, llvm, LSM List, linux-toolchains,
Arnd Bergmann, Greg Kroah-Hartman, Guenter Roeck, Kees Cook,
Mark Brown, Masahiro Yamada, Nathan Chancellor, Michal Marek,
Andrew Morton, Vipin Sharma, Chris Down, Rasmus Villemoes,
Daniel Borkmann, Vlastimil Babka, Linux Kbuild mailing list,
Linux Kernel Mailing List
On 9/7/21 3:18 PM, Linus Torvalds wrote:
> On Tue, Sep 7, 2021 at 3:14 PM Marco Elver <elver@google.com> wrote:
>>
>>
>> config WERROR
>> bool "Compile the kernel with warnings as errors"
>> - default y
>> + default COMPILE_TEST
>
> That seems reasonable. It very much is about build-testing.
That and 2 more things IMO:
a. having developers be responsible for build warnings, not just
build errors
b. having maintainers merge them more like they are build errors
and not just some warnings that can be overlooked.
I don't see enough of a. or b. :(
--
~Randy
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:33 ` Randy Dunlap
@ 2021-09-13 9:32 ` Pavel Machek
2021-09-13 9:46 ` Greg Kroah-Hartman
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Pavel Machek @ 2021-09-13 9:32 UTC (permalink / raw)
To: Randy Dunlap
Cc: Linus Torvalds, Marco Elver, Nick Desaulniers, llvm, LSM List,
linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman,
Guenter Roeck, Kees Cook, Mark Brown, Masahiro Yamada,
Nathan Chancellor, Michal Marek, Andrew Morton, Vipin Sharma,
Chris Down, Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
Hi!
> >> config WERROR
> >> bool "Compile the kernel with warnings as errors"
> >>- default y
> >>+ default COMPILE_TEST
> >
> >That seems reasonable. It very much is about build-testing.
>
> That and 2 more things IMO:
>
> a. having developers be responsible for build warnings, not just
> build errors
>
> b. having maintainers merge them more like they are build errors
> and not just some warnings that can be overlooked.
>
> I don't see enough of a. or b. :(
Do we really want developers treat warnings as errors? When the code
is okay but some random version of gcc dislikes it...
Plus, there's question of stable. We already get ton of churn there
("this fixes random warning"). WERROR will only encourage that...
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 9:32 ` Pavel Machek
@ 2021-09-13 9:46 ` Greg Kroah-Hartman
2021-09-13 10:02 ` Pavel Machek
2021-09-13 9:50 ` Florian Weimer
2021-09-13 14:33 ` Guenter Roeck
2 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-13 9:46 UTC (permalink / raw)
To: Pavel Machek
Cc: Randy Dunlap, Linus Torvalds, Marco Elver, Nick Desaulniers,
llvm, LSM List, linux-toolchains, Arnd Bergmann, Guenter Roeck,
Kees Cook, Mark Brown, Masahiro Yamada, Nathan Chancellor,
Michal Marek, Andrew Morton, Vipin Sharma, Chris Down,
Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On Mon, Sep 13, 2021 at 11:32:56AM +0200, Pavel Machek wrote:
> Hi!
>
> > >> config WERROR
> > >> bool "Compile the kernel with warnings as errors"
> > >>- default y
> > >>+ default COMPILE_TEST
> > >
> > >That seems reasonable. It very much is about build-testing.
> >
> > That and 2 more things IMO:
> >
> > a. having developers be responsible for build warnings, not just
> > build errors
> >
> > b. having maintainers merge them more like they are build errors
> > and not just some warnings that can be overlooked.
> >
> > I don't see enough of a. or b. :(
>
> Do we really want developers treat warnings as errors? When the code
> is okay but some random version of gcc dislikes it...
>
> Plus, there's question of stable. We already get ton of churn there
> ("this fixes random warning"). WERROR will only encourage that...
I will not be backporting this patch to older stable kernels, but I
_want_ to see stable builds build with no warnings. When we add
warnings, they are almost always things we need to fix up properly.
Over time, I have worked to reduce the number of build warnings in older
stable kernels. For newer versions of gcc, sometimes that is
impossible, but we are close...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 9:46 ` Greg Kroah-Hartman
@ 2021-09-13 10:02 ` Pavel Machek
2021-09-13 10:51 ` Greg Kroah-Hartman
2021-09-20 16:26 ` Geert Uytterhoeven
0 siblings, 2 replies; 21+ messages in thread
From: Pavel Machek @ 2021-09-13 10:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Randy Dunlap, Linus Torvalds, Marco Elver, Nick Desaulniers,
llvm, LSM List, linux-toolchains, Arnd Bergmann, Guenter Roeck,
Kees Cook, Mark Brown, Masahiro Yamada, Nathan Chancellor,
Michal Marek, Andrew Morton, Vipin Sharma, Chris Down,
Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]
Hi!
> > Do we really want developers treat warnings as errors? When the code
> > is okay but some random version of gcc dislikes it...
> >
> > Plus, there's question of stable. We already get ton of churn there
> > ("this fixes random warning"). WERROR will only encourage that...
>
> I will not be backporting this patch to older stable kernels, but I
> _want_ to see stable builds build with no warnings. When we add
> warnings, they are almost always things we need to fix up properly.
Well, everyone _wants_ to see clean builds... unless the price is too
high.
> Over time, I have worked to reduce the number of build warnings in older
> stable kernels. For newer versions of gcc, sometimes that is
> impossible, but we are close...
You clearly can't backport this patch, but for 5.16-stable, you'll
have it in, and now warnings are same as errors... and I don't believe
that's good idea for stable.
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 10:02 ` Pavel Machek
@ 2021-09-13 10:51 ` Greg Kroah-Hartman
2021-09-20 16:26 ` Geert Uytterhoeven
1 sibling, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-13 10:51 UTC (permalink / raw)
To: Pavel Machek
Cc: Randy Dunlap, Linus Torvalds, Marco Elver, Nick Desaulniers,
llvm, LSM List, linux-toolchains, Arnd Bergmann, Guenter Roeck,
Kees Cook, Mark Brown, Masahiro Yamada, Nathan Chancellor,
Michal Marek, Andrew Morton, Vipin Sharma, Chris Down,
Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On Mon, Sep 13, 2021 at 12:02:30PM +0200, Pavel Machek wrote:
> Hi!
>
> > > Do we really want developers treat warnings as errors? When the code
> > > is okay but some random version of gcc dislikes it...
> > >
> > > Plus, there's question of stable. We already get ton of churn there
> > > ("this fixes random warning"). WERROR will only encourage that...
> >
> > I will not be backporting this patch to older stable kernels, but I
> > _want_ to see stable builds build with no warnings. When we add
> > warnings, they are almost always things we need to fix up properly.
>
> Well, everyone _wants_ to see clean builds... unless the price is too
> high.
>
> > Over time, I have worked to reduce the number of build warnings in older
> > stable kernels. For newer versions of gcc, sometimes that is
> > impossible, but we are close...
>
> You clearly can't backport this patch, but for 5.16-stable, you'll
> have it in, and now warnings are same as errors... and I don't believe
> that's good idea for stable.
I do, it will force us to keep these trees clean over time.
And it will be in 5.15, not 5.16 :)
Worst case, we disable it in 4 years when gcc 15 or so generates so
many errors we can't resolve them in this old kernel.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 10:02 ` Pavel Machek
2021-09-13 10:51 ` Greg Kroah-Hartman
@ 2021-09-20 16:26 ` Geert Uytterhoeven
1 sibling, 0 replies; 21+ messages in thread
From: Geert Uytterhoeven @ 2021-09-20 16:26 UTC (permalink / raw)
To: Pavel Machek
Cc: Greg Kroah-Hartman, Randy Dunlap, Linus Torvalds, Marco Elver,
Nick Desaulniers, llvm, LSM List, linux-toolchains,
Arnd Bergmann, Guenter Roeck, Kees Cook, Mark Brown,
Masahiro Yamada, Nathan Chancellor, Michal Marek, Andrew Morton,
Vipin Sharma, Chris Down, Rasmus Villemoes, Daniel Borkmann,
Vlastimil Babka, Linux Kbuild mailing list,
Linux Kernel Mailing List
On Mon, Sep 13, 2021 at 12:50 PM Pavel Machek <pavel@ucw.cz> wrote:
> > > Do we really want developers treat warnings as errors? When the code
> > > is okay but some random version of gcc dislikes it...
> > >
> > > Plus, there's question of stable. We already get ton of churn there
> > > ("this fixes random warning"). WERROR will only encourage that...
> >
> > I will not be backporting this patch to older stable kernels, but I
> > _want_ to see stable builds build with no warnings. When we add
> > warnings, they are almost always things we need to fix up properly.
>
> Well, everyone _wants_ to see clean builds... unless the price is too
> high.
>
> > Over time, I have worked to reduce the number of build warnings in older
> > stable kernels. For newer versions of gcc, sometimes that is
> > impossible, but we are close...
>
> You clearly can't backport this patch, but for 5.16-stable, you'll
> have it in, and now warnings are same as errors... and I don't believe
> that's good idea for stable.
The good thing about the config option is that there's now a single point
to enable or disable -Werror. In the past, maintainers sprinkled -Werror
all over the various Makefiles in the tree, which means you have to
edit multiple files to disable it again.
Background: I've been investigating an issue that involved building old
v2.6.x kernels. Apart from having to use very old compilers, it still caused
compiler warnings that obviously weren't seen with the slightly different
compiler versions used by maintainers who added -Werror.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 9:32 ` Pavel Machek
2021-09-13 9:46 ` Greg Kroah-Hartman
@ 2021-09-13 9:50 ` Florian Weimer
2021-09-13 17:42 ` Linus Torvalds
2021-09-13 14:33 ` Guenter Roeck
2 siblings, 1 reply; 21+ messages in thread
From: Florian Weimer @ 2021-09-13 9:50 UTC (permalink / raw)
To: Pavel Machek
Cc: Randy Dunlap, Linus Torvalds, Marco Elver, Nick Desaulniers,
llvm, LSM List, linux-toolchains, Arnd Bergmann,
Greg Kroah-Hartman, Guenter Roeck, Kees Cook, Mark Brown,
Masahiro Yamada, Nathan Chancellor, Michal Marek, Andrew Morton,
Vipin Sharma, Chris Down, Rasmus Villemoes, Daniel Borkmann,
Vlastimil Babka, Linux Kbuild mailing list,
Linux Kernel Mailing List
* Pavel Machek:
> Do we really want developers treat warnings as errors? When the code
> is okay but some random version of gcc dislikes it...
There are some warnings-as-errors which are quite reasonable, like
-Werror=implicit-function-declaration (which we can't make the compiler
default without cleaning up userspace first) and perhaps
-Werror=implicit-int. Some other warnings can be used to enforce coding
style, and there -Werror could make sense as well (-Werror=vla and
others).
But there are also warnings which are emitted by the GCC middle-end (the
optimizers), and turning on -Werror for those is very problematic.
These warnings are very target-specific and also depend on compiler
version and optimization parameters. Unfortunately that includes the
buffer size warnings based on function attributes (which would otherwise
be a good fit for the kernel because it uses few external headers).
GCC also lacks a facility to suppress warnings if they concern code that
was introduced during optimization and removed again later
(e.g. inlining, constant propagation, dead code removal).
Thanks,
Florian
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 9:50 ` Florian Weimer
@ 2021-09-13 17:42 ` Linus Torvalds
0 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2021-09-13 17:42 UTC (permalink / raw)
To: Florian Weimer
Cc: Pavel Machek, Randy Dunlap, Marco Elver, Nick Desaulniers, llvm,
LSM List, linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman,
Guenter Roeck, Kees Cook, Mark Brown, Masahiro Yamada,
Nathan Chancellor, Michal Marek, Andrew Morton, Vipin Sharma,
Chris Down, Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On Mon, Sep 13, 2021 at 2:50 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> But there are also warnings which are emitted by the GCC middle-end (the
> optimizers), and turning on -Werror for those is very problematic.
People say that, but let's face it, that's simply not true.
There are real problematic warnings, and we just turn those warnings
off. People who want the self-flagellation can enable them with W=1
(or bigger values), and spend their life fighting stupid random
compiler warnings that have tons of false positives.
But the fact is, I've required a warning-free build on x86-64 for
anything I notice for the last several years by now, and it really
hasn't been a problem.
What _has_ been a problem is that (a) build bots don't care about and
(b) the configs I don't personally test (other non-x86-64
architectures stand out, but there's certainly been other
configuration issues too).
But "bogus compiler warnings" is very much *not* in that list of problems.
I've looked at a lot of the warnings that are now errors, and while a
number of them have made me go "So why didn't we see that on x86-64?"
not one of them has actually made me go "-Werror was wrong".
Because EVERY single one I've seen has been for something that should
have been fixed. Presumably long long ago, but the warning it
generated had been ignored.
So stop with the "some warnings just happen" crap. Outside of actual
compiler bugs, and truly stupid warnings (that we turn off), that's
simply not true.
And yes, those compiler bugs happen. The new warning already found one
issue with current gcc trunk (non-released). So right now the count is
"lots of valid warnings, and one compiler bug that was found _thanks_
to me enabling -Werror".
Yes, we have issues with having to work around older compiler bugs.
Those aren't going away, and yes, -Werror may well mean that
non-x86-64 people now have to deal with them.
And yes, this is painful. I'm very much aware of that. But we just
need to do it. Because the warnings don't go away on their own, and
not making them fatal clearly just means that they'll stay around
forever.
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-13 9:32 ` Pavel Machek
2021-09-13 9:46 ` Greg Kroah-Hartman
2021-09-13 9:50 ` Florian Weimer
@ 2021-09-13 14:33 ` Guenter Roeck
2 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2021-09-13 14:33 UTC (permalink / raw)
To: Pavel Machek, Randy Dunlap
Cc: Linus Torvalds, Marco Elver, Nick Desaulniers, llvm, LSM List,
linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman, Kees Cook,
Mark Brown, Masahiro Yamada, Nathan Chancellor, Michal Marek,
Andrew Morton, Vipin Sharma, Chris Down, Rasmus Villemoes,
Daniel Borkmann, Vlastimil Babka, Linux Kbuild mailing list,
Linux Kernel Mailing List
On 9/13/21 2:32 AM, Pavel Machek wrote:
> Hi!
>
>>>> config WERROR
>>>> bool "Compile the kernel with warnings as errors"
>>>> - default y
>>>> + default COMPILE_TEST
>>>
>>> That seems reasonable. It very much is about build-testing.
>>
>> That and 2 more things IMO:
>>
>> a. having developers be responsible for build warnings, not just
>> build errors
>>
>> b. having maintainers merge them more like they are build errors
>> and not just some warnings that can be overlooked.
>>
>> I don't see enough of a. or b. :(
>
> Do we really want developers treat warnings as errors? When the code
> is okay but some random version of gcc dislikes it...
>
> Plus, there's question of stable. We already get ton of churn there
> ("this fixes random warning"). WERROR will only encourage that...
>
All Chrome OS builds are already done with -Werror enabled. Having it
enabled in the incoming stable releases will reduce our workload when
backporting stable releases. I am actually working on making at
least chromeos-5.10 "clean" for allmodconfig builds on arm, arm64,
and x86 (everything else is hopeless, and even arm may be futile,
but arm64 and x86 seem to be doable).
I'd rather have warnings fixed in incoming stable releases than having
to pull additional patches into our kernels.
Guenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:14 ` Marco Elver
2021-09-07 22:18 ` Linus Torvalds
@ 2021-09-07 22:28 ` Guenter Roeck
2021-09-07 22:42 ` Segher Boessenkool
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2021-09-07 22:28 UTC (permalink / raw)
To: Marco Elver, Nick Desaulniers, Linus Torvalds
Cc: llvm, LSM List, linux-toolchains, Arnd Bergmann,
Greg Kroah-Hartman, Kees Cook, Mark Brown, Masahiro Yamada,
Nathan Chancellor, Michal Marek, Andrew Morton, Vipin Sharma,
Chris Down, Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On 9/7/21 3:14 PM, Marco Elver wrote:
> On Tue, Sep 07, 2021 at 01:30PM -0700, Nick Desaulniers wrote:
>> On Tue, Sep 7, 2021 at 12:16 PM Linus Torvalds <torvalds@linux-foundation.org> wrote:
> [...]
>>> I'm not going to revert that change. I probably will have to limit it
>>> (by making that WERROR option depend on certain expectations), but
>>> basically any maintainer who has code that causes warnings should
>>> expect that they will have to fix those warnings.
>>
>> I'm not 100% against it; I think it could land in a more useful
>> variation. But it would be good to discuss that on-list, and give it
>> time to soak. This is a conversation we should be having with CI
>> maintainers IMO, and folks that maintain the build infra, at least.
>> I'm happy to kick off that discussion with this RFC.
>
> Here's a datapoint: I had to disable CONFIG_WERROR on a bunch of syzbot
> instances which started failing because of -Werror [1], because syzbot's
> time is better spent on fuzzing, and having the odd warning in some
> subsystem penalize fuzzing of the entire kernel is not appropriate.
>
> [1] https://github.com/google/syzkaller/commit/e096c0a2a414e487412c9669426780ce5acdde9d
>
> Getting the kernel built is a hard requirement for any sort of runtime
> testing. Once the kernel is built, runtime testing of various subsystems
> can proceed in parallel. A single warning in some odd subsystem
> penalizing the _entire_ kernel's testing progress is inappropriate. The
> severity of a use-after-free bug found by runtime testing is orders of
> magnitude more severe than some "unused variable" warning. Now such a
> warning would delay finding bugs at runtime on many CI systems that
> haven't yet disabled the warning.
>
> I'm predicting most distributions and runtime-focused CIs will disable
> the warning.
>
> I have formulated this in the form of a patch below, that might move
> this new Kconfig option towards its appropriate usecases by default.
>
> The intent is not to dispute the usefulness of -Werror, but select the
> appropriate usecases by default, limiting friction for all those who can
> do little more than say CONFIG_WERROR=n.
>
> Thanks,
> -- Marco
>
> ------ >8 ------
>
> From: Marco Elver <elver@google.com>
> Date: Tue, 7 Sep 2021 23:12:08 +0200
> Subject: [PATCH] kbuild: Only default to -Werror if COMPILE_TEST
>
> The cross-product of the kernel's supported toolchains, architectures,
> and configuration options is large. So large, that it's generally
> accepted to be infeasible to enumerate and build+test them all
> (many compile-testers rely on randomly generated configs).
>
> Without the possibility to enumerate all possible combinations of
> toolchains, architectures, and configuration options, it is inevitable
> that compiler warnings in this space exist.
>
> With -Werror, this means that an innumerable set of kernels are now
> broken, yet had been perfectly usable before (confused compilers, code
> with warnings unused, or luck).
>
> Distributors will necessarily pick a point in the toolchain X arch X
> config space, and if unlucky, will have a broken build. Granted, those
> will likely disable CONFIG_WERROR and move on.
>
> The kernel's default configuration is unlikely to be suitable for all
> users, but it's inappropriate to force many users to set CONFIG_WERROR=n.
>
> This also holds for CI systems which are focused on runtime testing,
> where the odd warning in some subsystem will disrupt testing of the rest
> of the kernel. Many of those runtime-focused CI systems run tests or
> fuzz the kernel using runtime debugging tools. Runtime testing of
> different subsystems can proceed in parallel, and potentially uncover
> serious bugs; halting runtime testing of the entire kernel because of
> the odd warning (now error) in a subsystem or driver is simply
> inappropriate.
>
> Therefore, runtime-focused CI systems will likely choose CONFIG_WERROR=n
> as well.
>
> The appropriate usecase for -Werror is therefore compile-test focused
> builds (often done by developers or CI systems).
>
> Reflect this in the Kconfig option by making the default value of WERROR
> match COMPILE_TEST.
>
> Signed-off-by: Marco Elver <elver@google.com>
I like it.
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> init/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 8cb97f141b70..11f8a845f259 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -139,7 +139,7 @@ config COMPILE_TEST
>
> config WERROR
> bool "Compile the kernel with warnings as errors"
> - default y
> + default COMPILE_TEST
> help
> A kernel build should not cause any compiler warnings, and this
> enables the '-Werror' flag to enforce that rule by default.
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:14 ` Marco Elver
2021-09-07 22:18 ` Linus Torvalds
2021-09-07 22:28 ` Guenter Roeck
@ 2021-09-07 22:42 ` Segher Boessenkool
2021-09-07 22:55 ` Mark Brown
2021-09-07 23:00 ` Nathan Chancellor
4 siblings, 0 replies; 21+ messages in thread
From: Segher Boessenkool @ 2021-09-07 22:42 UTC (permalink / raw)
To: Marco Elver
Cc: Nick Desaulniers, Linus Torvalds, llvm, LSM List,
linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman,
Guenter Roeck, Kees Cook, Mark Brown, Masahiro Yamada,
Nathan Chancellor, Michal Marek, Andrew Morton, Vipin Sharma,
Chris Down, Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On Wed, Sep 08, 2021 at 12:14:19AM +0200, Marco Elver wrote:
> Here's a datapoint: I had to disable CONFIG_WERROR on a bunch of syzbot
> instances which started failing because of -Werror [1], because syzbot's
> time is better spent on fuzzing, and having the odd warning in some
> subsystem penalize fuzzing of the entire kernel is not appropriate.
Similarly, I have to disable -Werror (which various archs and subsystems
already use) whenever I test building the kernel with new toolchains.
It is the biggest set of kernel patches I keep, already, since many
years.
I actually have good hopes that a centralised -Werror thing will make
this easier :-)
Maybe there can be an E=[01] kernel build flag to disable / enable
CONFIG_WERROR? Something that will override it for just that command.
This would make life easier for many use cases, while at the same time
not being something that people can "forget" they did.
Segher
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:14 ` Marco Elver
` (2 preceding siblings ...)
2021-09-07 22:42 ` Segher Boessenkool
@ 2021-09-07 22:55 ` Mark Brown
2021-09-07 23:00 ` Nathan Chancellor
4 siblings, 0 replies; 21+ messages in thread
From: Mark Brown @ 2021-09-07 22:55 UTC (permalink / raw)
To: Marco Elver
Cc: Nick Desaulniers, Linus Torvalds, llvm, LSM List,
linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman,
Guenter Roeck, Kees Cook, Masahiro Yamada, Nathan Chancellor,
Michal Marek, Andrew Morton, Vipin Sharma, Chris Down,
Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]
On Wed, Sep 08, 2021 at 12:14:19AM +0200, Marco Elver wrote:
> I'm predicting most distributions and runtime-focused CIs will disable
> the warning.
Yes, indeed.
> Date: Tue, 7 Sep 2021 23:12:08 +0200
> Subject: [PATCH] kbuild: Only default to -Werror if COMPILE_TEST
>
> The cross-product of the kernel's supported toolchains, architectures,
> and configuration options is large. So large, that it's generally
> accepted to be infeasible to enumerate and build+test them all
> (many compile-testers rely on randomly generated configs).
Reviwed-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] Revert "Enable '-Werror' by default for all kernel builds"
2021-09-07 22:14 ` Marco Elver
` (3 preceding siblings ...)
2021-09-07 22:55 ` Mark Brown
@ 2021-09-07 23:00 ` Nathan Chancellor
4 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2021-09-07 23:00 UTC (permalink / raw)
To: Marco Elver
Cc: Nick Desaulniers, Linus Torvalds, llvm, LSM List,
linux-toolchains, Arnd Bergmann, Greg Kroah-Hartman,
Guenter Roeck, Kees Cook, Mark Brown, Masahiro Yamada,
Michal Marek, Andrew Morton, Vipin Sharma, Chris Down,
Rasmus Villemoes, Daniel Borkmann, Vlastimil Babka,
Linux Kbuild mailing list, Linux Kernel Mailing List
On Wed, Sep 08, 2021 at 12:14:19AM +0200, Marco Elver wrote:
> On Tue, Sep 07, 2021 at 01:30PM -0700, Nick Desaulniers wrote:
> > On Tue, Sep 7, 2021 at 12:16 PM Linus Torvalds <torvalds@linux-foundation.org> wrote:
> [...]
> > > I'm not going to revert that change. I probably will have to limit it
> > > (by making that WERROR option depend on certain expectations), but
> > > basically any maintainer who has code that causes warnings should
> > > expect that they will have to fix those warnings.
> >
> > I'm not 100% against it; I think it could land in a more useful
> > variation. But it would be good to discuss that on-list, and give it
> > time to soak. This is a conversation we should be having with CI
> > maintainers IMO, and folks that maintain the build infra, at least.
> > I'm happy to kick off that discussion with this RFC.
>
> Here's a datapoint: I had to disable CONFIG_WERROR on a bunch of syzbot
> instances which started failing because of -Werror [1], because syzbot's
> time is better spent on fuzzing, and having the odd warning in some
> subsystem penalize fuzzing of the entire kernel is not appropriate.
>
> [1] https://github.com/google/syzkaller/commit/e096c0a2a414e487412c9669426780ce5acdde9d
>
> Getting the kernel built is a hard requirement for any sort of runtime
> testing. Once the kernel is built, runtime testing of various subsystems
> can proceed in parallel. A single warning in some odd subsystem
> penalizing the _entire_ kernel's testing progress is inappropriate. The
> severity of a use-after-free bug found by runtime testing is orders of
> magnitude more severe than some "unused variable" warning. Now such a
> warning would delay finding bugs at runtime on many CI systems that
> haven't yet disabled the warning.
>
> I'm predicting most distributions and runtime-focused CIs will disable
> the warning.
>
> I have formulated this in the form of a patch below, that might move
> this new Kconfig option towards its appropriate usecases by default.
>
> The intent is not to dispute the usefulness of -Werror, but select the
> appropriate usecases by default, limiting friction for all those who can
> do little more than say CONFIG_WERROR=n.
>
> Thanks,
> -- Marco
>
> ------ >8 ------
>
> From: Marco Elver <elver@google.com>
> Date: Tue, 7 Sep 2021 23:12:08 +0200
> Subject: [PATCH] kbuild: Only default to -Werror if COMPILE_TEST
>
> The cross-product of the kernel's supported toolchains, architectures,
> and configuration options is large. So large, that it's generally
> accepted to be infeasible to enumerate and build+test them all
> (many compile-testers rely on randomly generated configs).
>
> Without the possibility to enumerate all possible combinations of
> toolchains, architectures, and configuration options, it is inevitable
> that compiler warnings in this space exist.
>
> With -Werror, this means that an innumerable set of kernels are now
> broken, yet had been perfectly usable before (confused compilers, code
> with warnings unused, or luck).
>
> Distributors will necessarily pick a point in the toolchain X arch X
> config space, and if unlucky, will have a broken build. Granted, those
> will likely disable CONFIG_WERROR and move on.
>
> The kernel's default configuration is unlikely to be suitable for all
> users, but it's inappropriate to force many users to set CONFIG_WERROR=n.
>
> This also holds for CI systems which are focused on runtime testing,
> where the odd warning in some subsystem will disrupt testing of the rest
> of the kernel. Many of those runtime-focused CI systems run tests or
> fuzz the kernel using runtime debugging tools. Runtime testing of
> different subsystems can proceed in parallel, and potentially uncover
> serious bugs; halting runtime testing of the entire kernel because of
> the odd warning (now error) in a subsystem or driver is simply
> inappropriate.
>
> Therefore, runtime-focused CI systems will likely choose CONFIG_WERROR=n
> as well.
>
> The appropriate usecase for -Werror is therefore compile-test focused
> builds (often done by developers or CI systems).
>
> Reflect this in the Kconfig option by making the default value of WERROR
> match COMPILE_TEST.
>
> Signed-off-by: Marco Elver <elver@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> init/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 8cb97f141b70..11f8a845f259 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -139,7 +139,7 @@ config COMPILE_TEST
>
> config WERROR
> bool "Compile the kernel with warnings as errors"
> - default y
> + default COMPILE_TEST
> help
> A kernel build should not cause any compiler warnings, and this
> enables the '-Werror' flag to enforce that rule by default.
> --
> 2.33.0.153.gba50c8fa24-goog
^ permalink raw reply [flat|nested] 21+ messages in thread