I've yet to be convinced that this makes sense for every warning. It really depends on the warning IMO.
Edit to elaborate:
I was mostly referring to non-committed/non-release code not deserving -Werror [1]. However, even for release builds, the story kind of depends on whether your builds are hermetic or not. If your commit also includes a snapshot of all your dependencies including your compiler toolchain executables, then off the top of my head, I can't think of any cases where you want warnings without errors (though perhaps there might be some). But if you're ever going to compile the same commit with a different toolchain (like say your system toolchain that you updated), then I don't think you want -Werror, otherwise every time the warning catches a new case, you'll fail to build something that previously compiled fine.
Why, what’s the alternative - leave the warnings hanging around? I work on a multi million loc.C++ codebase that used -Werror. If we turned it off, the codebase would be full of warnings in hours.
Use -Werror for the build server and have a policy that you can't commit code that generates warnings. That lets you still build locally without having to immediately fix minor warnings that would be a distraction.
If the policy is followed no errors will be generated on the build server. You have to have a codebase that builds cleanly for this to be possible so you're forced to resolve warnings before they become a cancer.
There are no old warnings because the commits introducing them would have been rejected. Obviously a prerequisite of this is that you get your codebase to the point where it has no warnings. It is sort of like asking how the build server should differentiate between new test failures and existing test failures.
You update the compiler and suddenly the same code now generates warnings. Are these old warnings or new warnings? I have seen that with printing an integer in a loop 0..99. Old compilers didn't understand the limit so warned that my buffer was not big enough. Middle aged compilers were silent because I assume they understood the range. New compilers started warning again that the range was -2147483648..99 dumb shits.
Or are you one of those to never update a working system, which I completely agree with.
If you're working in a controlled environment, such as proprietary software with a fixed number of toolchains or a few OSS projects like Chromium that use their own fixed toolchain, then it makes sense to turn on -Werror. However, most OSS projects deal with tons of different compilers, so it is not possible to enforce -Werror, as different compilers and versions handle warnings differently.
it takes a bit of discipline, but i think it is entirely possible. Even when some OSS project support a lot of different build environment, the support is usually tiered with some of the target having daily or per commit builds. Those should be setup with -Werror + exceptions where it makes sense.
Once a project get to stable state (as in for a given target, all the warning are either turned off, or handled). Upgrading or adding new target get easier.
Even projects with a “stable” set of targets have too much churn for this to be practical. Just because you support “latest macOS” doesn’t mean my build should fail if I’m on a beta build ahead.
Yes, I typically run toolchains that are slightly ahead or different than the one that is actually used in CI. I don't think it is reasonable to think that projects will be using it, though they often do appreciate me testing it early for them.
Please let me know so I can fix issues. Or let the compiler vendor know, sometimes they add something new to get feedback on how useful it is in the real world.
The alternative is to fix warnings! You don't have to completely fail the build, to have the good engineering sense to fix warnings.
Unless you never build locally? And never look at a build log? You really never improve code if Werror doesn't force you to?
It's really obnoxious to also immediately and completely fail the build for someone who wants to use an updated compiler, or wants to compile for a different architecture, or wants to compare to an older compiler, or a different vendor's compiler, or test with and updated library with changed headers, or ...
Turning on -Werror means that once the warnings are eliminated, they stay eliminated, and a developer who adds code that produces a warning has their checkin rejected. If it isn't used the number of warnings will just grow and grow.
You're right, updated compilers that have more warnings are an issue, and that's why the document recommends that -Werror be used during development but not in the shipped code (for open source projects), so the recipient of the code isn't blocked by the problem you cite.
It definitely does for C/C++. I think there are two reasons:
1. C/C++ build systems tend to be super noisy printing every file that they compile, often long commands and usually in an ugly style, so it is very easy to just miss warnings or to give up even trying to look for them because the output is so verbose.
2. C/C++ tend to produce a lot of warnings that are very annoying to resolve properly and usually not worth the effort. Sign and size conversion warnings are probably the worst. They're low information warnings that usually don't have a clear solution, so people learn to just accept that some warnings should be ignored and they stop trying to fix warnings in general.
3. It's usually difficult to prevent warnings from third party code from being shown and you can't do much about those. There is `-isystem` but it's not well supported and it doesn't solve everything.
I think Go got rid of warnings entirely because of how bad the warning experience in C/C++. But my experience of Rust has shown that warnings can be done sensibly so I wouldn't use `-Werror` in Rust.
In C++ though, you should use `-Werror` in CI and then whitelist specific warnings. Just don't enable it for downstream users.
I wouldn't be so careless about sign conversion warnings. C can silently convert signed integers to unsigned and that can cause mayhem. I inherited a codebase that was exhibiting a classic signed conversion problem when a temperature reading dropped below 0°C. I simply turned on -Wextra and it flagged all the culprits with implicit conversion to unsigned. Throw in some casts and it was resolved.
What would be more useful would be some better UI for displaying the warnings.
e.g. -Woutput_warning_[html,json,text]=DIRECTORY_TO_OUTPUT_TO
or something, which would create a output file per compilation unit that displayed the warnings in a friendlier way.
It's possible something like this exists and I'm unaware...
Indeed, -Werror doesn’t make sense, but specifying specific errors (like -Werror=return-type to prevent forgetting to return a value from a function) is very, very necessary.
Most security/safety feature either on the language or the tooling side are are some what annoying since by definition they tend to add constrains to one workflow. The real question is whether they introduce more pain than they safe in average.
Right, I understand that. I'm overwhelmingly in support for most warnings. I just think that a handful don't have good tradeoffs or are not necessarily the most valuable for many projects.
The deprecated attribute is useless when -Werror and -Wall are both on. You can't land the attribute if any uses exist because any use is now a compiler error. You might as well just delete the entity and sort things out from there.
Normally you can pragma turn the error off for each use. I'm meaning to dp this for stuff where our ABI must support the function but I don't want anyone to use it otherwise.
The issue is that not every snapshot of a source file is intended to be committed as-is.
So it really depends if you're testing code or committing it. When you're debugging something locally, warnings like "unused parameter" or "dead code" become extremely annoying as errors. I need to be able to put "return 0;" in the middle of my function and run it as-is without having to battle the toolchain for five minutes after that every damn time.
Of course i don't have the specifics of your usual configuration. But usually this mostly a configuration problem. Dev builds usually don't have -Werror turned on ( or have a very easy way to turn Werror off). In most project i worked on, Werror is informance on the release, and continious test builds.
But i do agree that Werror on the code/build/test cycle sound like a pain.
I mean sure, you can work around any problem if you put enough effort into it. For this particular problem, in reality it introduces friction. e.g., one practical problem you face (if you've put sufficient work into your tooling to get to this point) is that your "dev" builds will now miss the cache far more often, because CI is running with different flags. Again, you can work around that... by (for example) spending more money and energy building with different flags. Nothing is impossible to do with enough investment, but these don't come free.
In the proposed context "developers should", yeah, it totally makes sense.
You've got different categories of warnings:
Things that are harmless but trivial to fix - so why not fix them.
Things that are valid in this specific context - they probably deserve a comment / pragma / explicit disabling.
Things that are real issues that should be fixed but won't stop the compilation - and if they're invisible because of the noise of the previous 3 categories, you're going to have issues.
Just today had to fix two things which had warnings available for a long time and with the recent clang they became an error. They shouldn't been fixed upstream ages ago, but they were ok with the warnings.
> Things that are harmless but trivial to fix - so why not fix them.
Because you're only trying out some experimental changes and are not ready to commit yet anyway. Making sure all corners are smoothened enough so that noone can cut himself makes no sense in that case.
It makes sense on my project where we have 20 million lines of C++, all covered by it. Only rarely does it stop my build on a I don't care (generally i'm adding temp debugging code that i'll rip out before commiting). If you keep clean code clean it isn't hard to leave werror on and even compiler upgrades are not that painful.
> and even compiler upgrades are not that painful.
…if you do them frequently enough.
A problem with OSS code is that it may lie dormant for years, then get picked up by somebody. Even if they try to compile for the same CPU architecture with the same but newer compiler, getting things to compile with -Werror again can be a challenge. Changing architecture (e.g. integers becoming 64 bit, triggering lots of ‘possible truncation’ errors) or compiler makes it harder.
I don’t think there’s a solution for that problem. Bit rot exists, and the longer you ignore it, the more work it is to get rid of it, and not trying to get rid of it is an invitation for having a serious warning getting drowned in harmless ones.
I maintain 20 million lines of C++ and I don't update my compiler very often. There is some pain on update, but it is small and generally easy to fix. Part of this is I already build with gcc and clang (including clang static analisys), and also run cppcheck.
Much of the complaints about upgrading compilers seem to be legacy from gcc 2.x days when they were adding a lot more warnings. Now gcc and clang both do a lot of testing against real world code to see if the warnings are noisy or not before adding them. (either that they are about visual studio or some other compiler I don't use)
It's been a long time since I've done anything in C, but I tended to use -Werror -Wall -Wextra and then disable individual warnings/errors if I was sure they were false positives. It can be tedious, but I figured better safe than sorry.
EDIT: For the last couple of years I've used Go almost exclusively, where every warning is an error. It was annoying and tedious at first, but I've come to appreciate it.
Every compiler flag indicates a place where somebody failed.
Those are either compiler implementators, language or standard designers, or all of them.
>When compiling C or C++ code on compilers such as GCC and clang, turn on these flags for detecting vulnerabilities at compile time and enable run-time protection mechanisms:
No, it is just respect for backwards compatibility.
Though, what might be nice is a shortcut or set of shortcuts for various recommendations e.g. -fopenssf-recommendation or something (and then you can override the parts you don't want afterwards). You could call it -fsafer but that might be too complicated to get everyone to agree on what it means...
This kind of disdain really should be put to rest. All it does is alienate people that were on the fence, or maintain projects with different goals in mind.
And besides, we'll be making similar comments about more your favorite recent languages once they're 38 years old and the field of computing changes. And if you don't believe so, you're in for a very rude awakening.
I can already see the replies that refuse to understand this point.
>And besides, we'll be making similar comments about more your favorite recent languages once they're 38 years old and the field of computing changes. And if you don't believe so, you're in for a very rude awakening.
I'm not going to have a problem with critique of my $fav_lang if its designers and implementators underperform.
So far they managed to do really good job after >2 decades.
>This kind of disdain really should be put to rest. All it does is alienate people that were on the fence, or maintain projects with different goals in mind.
I believe that languages and compilers should go way beyond just "enable things to be possible", but they also should be user friendly and be safe/sane by default.
That's why I'm critiquing this chaos. Don't you see this long ass list:
> I believe that languages and compilers should go way beyond just "enable things to be possible", but they also should be user friendly and be safe/sane by default.
Everyone one wants sane and safe default, and ease of use etc... etc... The questions here is more nuance is about should an existing ( and dare i say very successful) language navigate the need for backward compatibility with with security and UX concerns.
> That's why I'm critiquing this chaos.
Critiquing a design is fair. Critiquing without understanding the context and constraints around the said design is not particularly helpful and tend to rub people the wrong way. Adding platitudes like "user friendly and be safe/sane" gets old even faster.
> It is insane.
Not really... it's just the results of very very constrained design space. Reality is messy
You're replying to a blog post by the OpenSSF on how to harden C and C++ programs offering nothing of substance except childish remarks based entirely on your own subjective opinions.
It seems like you're replying to a field of strawmen you've constructed rather than what's being said.
It's a sign that the compiler devs value backward compatibility with earlier incarnations that lacked such programmer aids. These are features that come with an associated cost and you don't pay that price in C without explicitly choosing it.
Not mentioned in this doc, I like using `-fsanitize=safe-stack` in release builds. Accesses to stack allocated arrays which have non-safe offsets (compiler fails to prove all accesses are in bounds) cause the object to be allocated on the unsafe stack, elsewhere in memory where overruns can only touch other things on the unsafe stack. Provably in-bounds accessed variables, return addresses, compiler generated spill slots, etc., go on the safe stack. This has such a small performance impact, you'll measure improvement and slowdown in equal measure just because of the uncontrolled effects this has on memory layout. The main downside is that presently you can't enable it when building a shared object. Docs: https://clang.llvm.org/docs/SafeStack.html
Also! While the usual sanitizer runtime libraries aren't security hardened for use in production environments, but for UBSan there's -fsanitize-minimal-runtime which switches to a different runtime library that is intended for this purpose (or use -fsanitize-trap=... instead, which executes an illegal instruction on error). Note that if your program terminates with a UBSan error, an attacker who can check whether your program terminated or not could use that as a primitive to leak data, so consider the security impact on your use case carefully. UBSan has a quite small performance impact when building with optimization, so you could deploy to production with it enabled, or parts of it enabled.
We did include `-D_GLIBCXX_ASSERTIONS` but we intentionally did not include `-D_GLIBCXX_DEBUG`, because we've been focusing on production releases.
The current plan is to add more information distinguishing between production code and instrumented test code, and adding options specific to each. There are many options that might make sense for instrumented test code that don't make sense for production code (and vice versa).
The idea of using `-fsanitize-minimal-runtime` is interesting. I don't have any direct experience with that option. I've created an issue to investigate maybe adding that to the guide. Thanks for the tip!
https://github.com/ossf/wg-best-practices-os-developers/issu...
I'm not really convinced by the arguments that rpath should be discouraged wholesale. That seems like pessimistic/paranoid advice. It's very useful for people distributing executables to vendor their libraries and for that to take precedence over the default loader paths.
I see the argument for setgid/setuid binaries. That's a better argument against setgid/setuid permissions (doubly so if they're not statically linked) than against rpath.
GCC/LLVM want projects to use the latest version of their compilers and if they added all this be default it would break millions of projects compiles.
Those project maintainers won't just add a million flags to make "legacy project 224" compile, they will just use the last compiler it worked with for eternity.
See that article that's current trending on HN if your response is "they just just fix the errors" or something along those lines, many times there isn't any business case to spend man-hours on something that works fine and has worked fine for decades.
71 comments
[ 3.0 ms ] story [ 125 ms ] threadI've yet to be convinced that this makes sense for every warning. It really depends on the warning IMO.
Edit to elaborate:
I was mostly referring to non-committed/non-release code not deserving -Werror [1]. However, even for release builds, the story kind of depends on whether your builds are hermetic or not. If your commit also includes a snapshot of all your dependencies including your compiler toolchain executables, then off the top of my head, I can't think of any cases where you want warnings without errors (though perhaps there might be some). But if you're ever going to compile the same commit with a different toolchain (like say your system toolchain that you updated), then I don't think you want -Werror, otherwise every time the warning catches a new case, you'll fail to build something that previously compiled fine.
[1] https://news.ycombinator.com/item?id=38481255
Or are you one of those to never update a working system, which I completely agree with.
Once a project get to stable state (as in for a given target, all the warning are either turned off, or handled). Upgrading or adding new target get easier.
Are you speaking here from experience ? Because mine has been quite different.
Unless you never build locally? And never look at a build log? You really never improve code if Werror doesn't force you to?
It's really obnoxious to also immediately and completely fail the build for someone who wants to use an updated compiler, or wants to compile for a different architecture, or wants to compare to an older compiler, or a different vendor's compiler, or test with and updated library with changed headers, or ...
You're right, updated compilers that have more warnings are an issue, and that's why the document recommends that -Werror be used during development but not in the shipped code (for open source projects), so the recipient of the code isn't blocked by the problem you cite.
Only in shitty teams without discipline and only if warnings are not tracked in some other way.
Like -Werror, and additional "lint"-style checks to make sure that coding guidelnes are followed.
Non-shitty teams automate as much of the flow as possible, to catch mistakes early and help the human reviewers catch everything.
1. C/C++ build systems tend to be super noisy printing every file that they compile, often long commands and usually in an ugly style, so it is very easy to just miss warnings or to give up even trying to look for them because the output is so verbose.
2. C/C++ tend to produce a lot of warnings that are very annoying to resolve properly and usually not worth the effort. Sign and size conversion warnings are probably the worst. They're low information warnings that usually don't have a clear solution, so people learn to just accept that some warnings should be ignored and they stop trying to fix warnings in general.
3. It's usually difficult to prevent warnings from third party code from being shown and you can't do much about those. There is `-isystem` but it's not well supported and it doesn't solve everything.
I think Go got rid of warnings entirely because of how bad the warning experience in C/C++. But my experience of Rust has shown that warnings can be done sensibly so I wouldn't use `-Werror` in Rust.
In C++ though, you should use `-Werror` in CI and then whitelist specific warnings. Just don't enable it for downstream users.
e.g. -Woutput_warning_[html,json,text]=DIRECTORY_TO_OUTPUT_TO or something, which would create a output file per compilation unit that displayed the warnings in a friendlier way.
It's possible something like this exists and I'm unaware...
So it really depends if you're testing code or committing it. When you're debugging something locally, warnings like "unused parameter" or "dead code" become extremely annoying as errors. I need to be able to put "return 0;" in the middle of my function and run it as-is without having to battle the toolchain for five minutes after that every damn time.
But i do agree that Werror on the code/build/test cycle sound like a pain.
You've got different categories of warnings:
Things that are harmless but trivial to fix - so why not fix them.
Things that are valid in this specific context - they probably deserve a comment / pragma / explicit disabling.
Things that are real issues that should be fixed but won't stop the compilation - and if they're invisible because of the noise of the previous 3 categories, you're going to have issues.
Just today had to fix two things which had warnings available for a long time and with the recent clang they became an error. They shouldn't been fixed upstream ages ago, but they were ok with the warnings.
Because you're only trying out some experimental changes and are not ready to commit yet anyway. Making sure all corners are smoothened enough so that noone can cut himself makes no sense in that case.
…if you do them frequently enough.
A problem with OSS code is that it may lie dormant for years, then get picked up by somebody. Even if they try to compile for the same CPU architecture with the same but newer compiler, getting things to compile with -Werror again can be a challenge. Changing architecture (e.g. integers becoming 64 bit, triggering lots of ‘possible truncation’ errors) or compiler makes it harder.
I don’t think there’s a solution for that problem. Bit rot exists, and the longer you ignore it, the more work it is to get rid of it, and not trying to get rid of it is an invitation for having a serious warning getting drowned in harmless ones.
Much of the complaints about upgrading compilers seem to be legacy from gcc 2.x days when they were adding a lot more warnings. Now gcc and clang both do a lot of testing against real world code to see if the warnings are noisy or not before adding them. (either that they are about visual studio or some other compiler I don't use)
EDIT: For the last couple of years I've used Go almost exclusively, where every warning is an error. It was annoying and tedious at first, but I've come to appreciate it.
Those are either compiler implementators, language or standard designers, or all of them.
>When compiling C or C++ code on compilers such as GCC and clang, turn on these flags for detecting vulnerabilities at compile time and enable run-time protection mechanisms:
>-O2 -Wall -Wformat=2 -Wconversion -Wtrampolines -Wimplicit-fallthrough \ >-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 \ >-D_GLIBCXX_ASSERTIONS \ >-fstrict-flex-arrays=3 \ >-fstack-clash-protection -fstack-protector-strong \ >-Wl,-z,nodlopen -Wl,-z,noexecstack \ >-Wl,-z,relro -Wl,-z,now
Christ.
Though, what might be nice is a shortcut or set of shortcuts for various recommendations e.g. -fopenssf-recommendation or something (and then you can override the parts you don't want afterwards). You could call it -fsafer but that might be too complicated to get everyone to agree on what it means...
Works for language standard versions ¯\_(ツ)_/¯
And besides, we'll be making similar comments about more your favorite recent languages once they're 38 years old and the field of computing changes. And if you don't believe so, you're in for a very rude awakening.
I can already see the replies that refuse to understand this point.
I'm not going to have a problem with critique of my $fav_lang if its designers and implementators underperform.
So far they managed to do really good job after >2 decades.
>This kind of disdain really should be put to rest. All it does is alienate people that were on the fence, or maintain projects with different goals in mind.
I believe that languages and compilers should go way beyond just "enable things to be possible", but they also should be user friendly and be safe/sane by default.
That's why I'm critiquing this chaos. Don't you see this long ass list:
">-O2 -Wall -Wformat=2 -Wconversion -Wtrampolines -Wimplicit-fallthrough \ >-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 \ >-D_GLIBCXX_ASSERTIONS \ >-fstrict-flex-arrays=3 \ >-fstack-clash-protection -fstack-protector-strong \ >-Wl,-z,nodlopen -Wl,-z,noexecstack \ >-Wl,-z,relro -Wl,-z,now"
It is insane. Just because someone values different things then it doesnt make it flawless.
Everyone one wants sane and safe default, and ease of use etc... etc... The questions here is more nuance is about should an existing ( and dare i say very successful) language navigate the need for backward compatibility with with security and UX concerns.
> That's why I'm critiquing this chaos.
Critiquing a design is fair. Critiquing without understanding the context and constraints around the said design is not particularly helpful and tend to rub people the wrong way. Adding platitudes like "user friendly and be safe/sane" gets old even faster.
> It is insane.
Not really... it's just the results of very very constrained design space. Reality is messy
>
It seems like you're replying to a field of strawmen you've constructed rather than what's being said.
Admittedly, it took longer than expected.
Also! While the usual sanitizer runtime libraries aren't security hardened for use in production environments, but for UBSan there's -fsanitize-minimal-runtime which switches to a different runtime library that is intended for this purpose (or use -fsanitize-trap=... instead, which executes an illegal instruction on error). Note that if your program terminates with a UBSan error, an attacker who can check whether your program terminated or not could use that as a primitive to leak data, so consider the security impact on your use case carefully. UBSan has a quite small performance impact when building with optimization, so you could deploy to production with it enabled, or parts of it enabled.
The current plan is to add more information distinguishing between production code and instrumented test code, and adding options specific to each. There are many options that might make sense for instrumented test code that don't make sense for production code (and vice versa).
The idea of using `-fsanitize-minimal-runtime` is interesting. I don't have any direct experience with that option. I've created an issue to investigate maybe adding that to the guide. Thanks for the tip! https://github.com/ossf/wg-best-practices-os-developers/issu...
I see the argument for setgid/setuid binaries. That's a better argument against setgid/setuid permissions (doubly so if they're not statically linked) than against rpath.
I think HN's current link is the right one. The blog post announces its release. HN is linking to the actual guide.
Those project maintainers won't just add a million flags to make "legacy project 224" compile, they will just use the last compiler it worked with for eternity.
https://olano.dev/2023-11-30-code-is-run-more-than-read/
See that article that's current trending on HN if your response is "they just just fix the errors" or something along those lines, many times there isn't any business case to spend man-hours on something that works fine and has worked fine for decades.
I've never understood why you would want to delete null pointer checks. The redhat blog also mentions this: https://www.redhat.com/en/blog/security-flaws-caused-compile...