Compromise reached as Linux kernel community protests about treating compiler warnings as errors
- Reference: 1631100772
- News link: https://www.theregister.co.uk/2021/09/08/compromise_linux_kernel_compiler_warnings/
- Source link:
Linux creator and maintainer Linus Torvalds [1]amended the Makefile used to compile the kernel so that -Werror was the default, saying: "We really should always have a clean build." The code was merged into what will be version 5.15 of the kernel.
"I guess the good news is that some builds still pass," [2]said another developer, noting 64 failures out of 153 builds for different platforms. "That sadly proves the point of that patch," Torvalds responded.
[3]
"Our Clang builds got bit pretty hard by this," [4]said another , to which Torvalds said: "Clang is clearly doing a *HORRIBLE* job with stack usage."
[5]
[6]
Google software engineer Nick Desaulniers, who works on compiling the Linux Kernel with Clang and LLVM went further, submitting a [7]patch to revert the change. "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… -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," he said. "This change has caused nearly all of our CI to go red."
Desaulniers added that he strongly favoured fixing warnings. "I do recognize the irony of someone who's spent a lot of time cleaning up warnings to be advocating for disabling -Werror."
[8]GitHub merges 'useless garbage' says Linus Torvalds as new NTFS support added to Linux kernel 5.15
[9]Tachyum's Prodigy emulator achieves first boot, runs Linux and says 'hello, world'
[10]When you finish celebrating Linux turning 30, try new Linux 5.14, says Linus Torvalds
[11]30 years of Linux: OS was successful because of how it was licensed, says Red Hat
"No," said Torvalds. "It was merged in response [to] years of pain, with the last one just being the final drop… 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… and I'm most definitely not convinced when the 'let's finally enable -Werror after years of talking about it' people end up going 'but but but I have thousands of warnings'. That's the POINT of that commit."
The arguments continued, with Google software engineer Marco Elver [12]stating : "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."
[13]
Elver proposed a compromise. "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."
COMPILE_TEST is used to compile the kernel including drivers that will not be loaded. "Developers still, opposing to distributors, might want to build such drivers to compile-test them… If you are a developer and want to build everything available, say Y here," says the [14]comment explaining COMPILE_TEST.
"That seems reasonable. It very much is about build-testing," said Torvalds. He [15]committed Elver's patch yesterday.
[16]
Hat tip to Michael Larabel for [17]spotting the discussion, which gives insight into how Torvalds manages the kernel code and his ability to find compromise despite strong opinions. ®
Get our [18]Tech Resources
[1] https://www.theregister.com/2021/09/06/github_merges_useless_garbage_says/
[2] https://lore.kernel.org/lkml/20210906142615.GA1917503@roeck-us.net/#t
[3] https://pubads.g.doubleclick.net/gampad/jump?co=1&iu=/6978/reg_software/front&sz=300x50%7C300x100%7C300x250%7C300x251%7C300x252%7C300x600%7C300x601&tile=2&c=2YTjeO9aWjC2TH3joErfr1wAAAQM&t=ct%3Dns%26unitnum%3D2%26raptor%3Dcondor%26pos%3Dtop%26test%3D0
[4] https://lore.kernel.org/lkml/CAHk-=wgZkQ+eZ02TaCpAWo_ffiLMwA2tYNHyL+B1dQ4YB0qfmA@mail.gmail.com/
[5] https://pubads.g.doubleclick.net/gampad/jump?co=1&iu=/6978/reg_software/front&sz=300x50%7C300x100%7C300x250%7C300x251%7C300x252%7C300x600%7C300x601&tile=4&c=44YTjeO9aWjC2TH3joErfr1wAAAQM&t=ct%3Dns%26unitnum%3D4%26raptor%3Dfalcon%26pos%3Dmid%26test%3D0
[6] https://pubads.g.doubleclick.net/gampad/jump?co=1&iu=/6978/reg_software/front&sz=300x50%7C300x100%7C300x250%7C300x251%7C300x252%7C300x600%7C300x601&tile=3&c=33YTjeO9aWjC2TH3joErfr1wAAAQM&t=ct%3Dns%26unitnum%3D3%26raptor%3Deagle%26pos%3Dmid%26test%3D0
[7] https://lore.kernel.org/lkml/20210907183843.33028-1-ndesaulniers@google.com/
[8] https://www.theregister.com/2021/09/06/github_merges_useless_garbage_says/
[9] https://www.theregister.com/2021/09/01/tachyums_prodigy_emulator_boots/
[10] https://www.theregister.com/2021/08/30/linux_5_14/
[11] https://www.theregister.com/2021/08/25/30_years_of_linux_red_hat/
[12] https://lore.kernel.org/lkml/YTfkO2PdnBXQXvsm@elver.google.com/
[13] https://pubads.g.doubleclick.net/gampad/jump?co=1&iu=/6978/reg_software/front&sz=300x50%7C300x100%7C300x250%7C300x251%7C300x252%7C300x600%7C300x601&tile=4&c=44YTjeO9aWjC2TH3joErfr1wAAAQM&t=ct%3Dns%26unitnum%3D4%26raptor%3Dfalcon%26pos%3Dmid%26test%3D0
[14] https://github.com/torvalds/linux/blob/master/init/Kconfig
[15] https://github.com/torvalds/linux/commit/b339ec9c229aaf399296a120d7be0e34fbc355ca
[16] https://pubads.g.doubleclick.net/gampad/jump?co=1&iu=/6978/reg_software/front&sz=300x50%7C300x100%7C300x250%7C300x251%7C300x252%7C300x600%7C300x601&tile=3&c=33YTjeO9aWjC2TH3joErfr1wAAAQM&t=ct%3Dns%26unitnum%3D3%26raptor%3Deagle%26pos%3Dmid%26test%3D0
[17] https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.15-Werror-Pain
[18] https://whitepapers.theregister.com/
Re: Seems like a good idea
You should try being part of the support team for a product whose Administrators believe that until a server crashes there is not a problem. All those those messages in the log saying that things are not well can be ignored as clearly it is still working...until it isn't
Pro-active Admin/Support seems to be an antiquated philosophy treated in the same way as bean-counters deciding redundancy is too expensive if you already have resilience. If you cannot prove the time you spend fixing things causing warning messages will save money then clearly you have too much time on your hands and maybe they can reduce the team size.
Bitter? Moi? Heaven forfend
Re: Seems like a good idea
On a slightly different point. just go into the windows event log a few days after installing it and see how many warnings and errors there are .......
Re: Seems like a good idea
It is generally a good idea to run like that (I do), but there are some compilers out there that emit false-positives. Not easy to manage if there is no way to suppress the warning and there is no problem to fix.
Warnings are usually there for a reason.
Something like an unused variable warning should not be trivialized. I have come across a case where a variable was declared for a specific purpose but the code accidentally used a similarly named variable that was being used for something else. Turns out that two different values don't fit into one storage location.
The only reason this is such a pain now is that -Werror wasn't the default from the start and things have accumulated over the years. I hope that a slow cleanup can take place with a view to eventually allowing -Werror to be the default for ALL builds. If it turns out that there are some truly trivial warnings, this is a good opportunity to either remove them or downgrade them to observations.
I'd say there should be a way to determine at what level a warning should change into an error. An unused variable from a copy book is a lot less serious than opening a file for output/update without any writing or deleting in that file.
Libraries
You also need some way to manage the use of libraries that are provided as source code; it is possible that a particular application will only use part of the API, rendering large numbers of objects and functions "unused".
Not a problem if you only consider "use" within a single translation unit, but more complex if you have system wide checks in place (e.g., when working to something like MISRA).
"An unused variable"
Should not exist.
If you write your code properly, you know what variables you use and why. This is not a crapshoot, developers do not generally declare variables without a reason.
If you have an unused variable, you need to check why it is unused because there is a chance that your code might be using some other variable instead, and that's when mayhem happens.
If your variable is truly unused for good reason, then remove the declaration and recompile.
Good code is clean code.
Back in the day when I was learning how to program with IBMs BASICA compiler, I learned that there is not a single compiler message that does not warrant attention. If you have more than one definition of a variable in the Common area, you're in trouble.
These days, I code business applications. An error is when an indispensable resource is not available. A warning is when a document is missing a given parameter. If the code encounters an error, it logs the problem and bails out. If it encounters a warning, it logs the problem and soldiers on to the next item.
But I code for high-level applications, ie not kernel-level code. I cannot imagine a kernel module that has a "warning" like "HDD not available" that should not be looked into.
Re: "An unused variable"
An example: I maintain PDCursesMod, based on a library whose specification goes back to the late 1980s. In a perfect world, if I had a function such as
int foo( int variable);
which no longer makes use of variable and doesn't return a value, I'd change that to
void foo( void);
There is, however, a specification of what parameters are passed to foo and what it returns, and 40+ years of code written to that spec. I can't just toss my toys out of my pram and say "not gonna do that anymore". (If 'foo' has an unused local variable, then you're right; that should be fixed. In fact, an unused local variable usually means I did something wrong; unused variables passed to a function are usually -- though not always -- less worrisome.)
I only get warnings when a variable is unused, not that the return value is unused. (I could imagine a savvy compiler figuring out the latter, but haven't seen it done.) To suppress such warnings, I define this handy macro:
[1]#define INTENTIONALLY_UNUSED_PARAMETER( param) (void)(param)
and then, within foo(),
[2] INTENTIONALLY_UNUSED_PARAMETER( variable);
which both says to the compiler "don't bug me about this" and to the human reader "yeah, I know this isn't being used; I planned it that way". (I've become a big fan of -Werror.)
[1] https://github.com/Bill-Gray/PDCursesMod/blob/master/curspriv.h#L127
[2] https://github.com/Bill-Gray/PDCursesMod/blob/master/pdcurses/attr.c#L243
Re: "An unused variable"
You can get unused return warnings by adding an annotation to the function.
(void)variable;
Just do it! Although I think I read about a future GCC not warning about that in the future . . .
A billion years ago...
In internet time, we used lint (which could be somewhat frustrating as there are some warnings it just doesn't shut up about). Apparently the GCC folks decided that a separate program was Wrong [tm] and that warnings would be built into the compiler but they are not turned on by default.
As frustrating as lint could be, it was an excellent tool to trap silly errors (and some not so silly).
Treating development builds to the -Wall statement makes sense; if a particular warning is not an issue then there are ways to work around it (suppressing a specific instance of a warning is quite simple). Doing -Werr makes us consider the warning(s) more seriously.
Source that may be compiled on a new compiler that may have new and interesting warnings of no interest at the current time should not default to -Werr after passing the 'no warnings' test at development time; after all, you want that build to succeed.
Now I see the devs point of view here. To quote Henry Spencer, "De-linting a program which has never been linted before is often a cleaning of the stables such as thou wouldst not wish on thy worst enemies"
Substitute clear all warnings for de-linting in the above and you have what is now being done.
So -Werr is probably the right thing to do to prove valid construction (but not necessarily what the code will actually do...).
No warnings = healthy code
... any maintainer who has code that causes warnings should expect that they will have to fix those warnings…
Once again, albeit after years of pain , Torvalds tells it as it is.
In my opinion as an end user, a very necessary statement.
We have already read about Torvalds [1]complaining about incorrectly commented code and I see this as a follow up to that.
But as I have mentioned before, it is not only incorrectly commented code: how much unfixed code has been piling up for years ie: won't fix code because it produced a harmless warning , did not affect enough people or because the kernel version was soon to be EOL'd?
Of course, some may get snipped out by the next kernel version/s but most will probably not.
And that's just dirt/grime being piled up and festering in some dark corner, much of it originated in the type of code that the strict implementation of -Werror will eventually eliminate.
In time, kernel code without warnings will be a matter of course. ie: clean, healthy code with no useless crud left behind.
Today, these warnings (the pain LT refers to) compose part of that dirt/grime which may eventually raise it's ugly head and end up breaking something further down the line and probably take a lot of time and manpower to fix.
So whatever the cost the implementation of -Werror, it is better to assume it as early as possible.
I see it as an unavoidable, essential part of healthy professional coding.
----
The only way forward is lean and clean code.
---
It is a concept that would seem to have been lost and Linux Torvalds is only reminding us all that he has not lost sight of it.
Kudos to him.
O.
[1] http://www.theregister.com/2021/06/26/linux_kernel_contributor_from_huawei/
Re: professional coding
Indeed.
A true professional programmer is not just a guy who knows how to code, it is a person who knows what to do in a given environment with respect to data security and operational procedures.
And now GDPR.
One day, professional programmers will be held to the same standards as engineers.
And that will be a Good Thing (TM).
Re: No warnings = healthy code
No warnings == healthy code
at least in my language.
I have a free software C project that supports three different compilers on four different chip architectures. I took the time to get rid of all warnings as I didn't want someone else to have to dig into whether a "known" warning is a new problem.
The big issue comes in when you have to use compiler extensions, especially to access things like SIMD features. Since they are non-standard, there is no common standard. Things like LLVM/Clang simply don't have the same extent of support that GCC has. MS VC does things differently. 64 bit ARM will do some things differently from 32 bit ARM and may need extra variables for CPU specific features in order to do the same thing. X86 is such a dog's breakfast of optional features on different chip models that it's pretty much hopeless to try to cover them all.
The end result is that you need lots of #ifdef and macro statements to sort out the differences between compilers and chip architectures. You don't find out about these things until you actually test them however. And you can't test them if you don't have the hardware and compiler tool chain.
So when anybody can rock up with a different chip, or worse, a different compiler, it's pretty much hopeless to expect anything other than fairly vanilla C code to pass through without some warnings. And if you get third parties who have these oddballs sending you patches full if $ifdefs that you can't test that's not really an answer either.
I think a more realistic answer would be to set one compiler and a core set of chip architectures which must pass without warnings and leave dealing with the rest optional, depending on how realistic that is.
Lack of warnings...
...means you can now concentrate on finding the really subtle problems.
The fact that a compiler is 'happy' with your code by no means means it is healthy code.
Many, many years ago, I was working on some programs that did quantum mechanical calculations to simulate molecules. I was debugging a set of programs as a result of trying to run them on a different architecture of CPU to the original. One of the subroutines did multiplication of two matrices together.
Now, [1]matrix multiplication is not generally commutative . The ordering of the arguments mattered . So if you have matrix 'A', and multiply by matrix 'B', you will, under certain circumstances get a different answer if you multiply matrix 'B' by matrix 'A'. In other words AxB does not always equal BxA.
So, if you have a subroutine that accepts two matrix arguments, multiplies them and gives you the result, it is rather important that you specify them consistently in the correct order.
Of course, in the thousands of calls to this particular subroutine from all over the program, the order had been mixed up in one or two, resulting in code that compiled fine, but gave entirely borked results.
Lack of compiler errors means the code is free to have deeper problems that ruin your day.
NN
[1] https://www.engageny.org/sites/default/files/downloadable-resources/2015/Nov/precalculus-m2-topic-b-lesson-10-teacher.pdf
Re: Lack of warnings...
Nobody is expecting a compiler to pick up problem domain failures like that. But you can spend more time understanding the problem domain when you are not constantly having to decide which warnings you think matter.
The way I like to look at this is:
A warning is an error that doesn't prevent the compiler from outputting something.
How valid that something is depends on the warning, but at the end of the day any code that produces warnings says to me "this code was written by someone who doesn't really care".
I don't blame him
Years ago I made this exact commit to one of my projects along with a set of annotations for all of the functions that used format strings. The other programmer threw a hissyfit and turned it all off again complaining about the fact that he didn't have time for all of that. A couple weeks later we try it on some nice new 64 bit servers and his code couldn't even last 30 seconds without a crash.
Years later, he quit/got fired (depending on which side you ask) and I got stuck maintaining his code. I ignored the large bug list while enabling a set of warnings/ fixing them, in a loop for a few weeks. When the code was retested, 95% of the bugs were gone. His logic was fine and he could have saved himself years of effort had he fixed the warnings in the first place.
The warnings are there for a reason. Fix and clean up your code.
Seems like a good idea
Given that my compiler warns me when I screw up certain things like "use of = in condition context" and guff about pointer to non-equal pointer ... things that technically result in a valid executable, but not one that does what it is supposed to .
Warnings are there for a reason - to warn you that it thinks something is wrong. If you're getting spurious unnecessary warnings (function blah not defined in header (because you forgot static)), there's usually some sort of option or pragma to turn that off. Or, maybe, fix the problem?
It freaks me out that some people seem to think that thousands of warnings is not a big deal.