Warn about any use of stack-allocated variable length arrays. There are a couple of reasons why these are bad: without care they can cause exploits where a caller specifies a very large array which crashes the program. And they are a problem where stack space is limited (kernel code, threads on 32 bit machines).
-Wframe-larger-than=5000 -Wstack-usage=10000
Prevent large stack frames. Same reasons as above.
-Wshadow
Prevent shadowed variables, eg. a local variable with the same name as a global.
-Werror
Turn warnings into errors. Only use this for developer builds though, because newer compilers introduce new warnings and that will cause problems for downstream packagers.
When building a library, make sure all symbols between compilation units but within the library are not exported. Basically reduces the load-time overhead and prevents random ELF symbol interposition. If you use this, then to export a symbol you have to use '__attribute__((__visibility__("default")))'.
-fno-strict-overflow -Wno-strict-overflow
(Controversially) unbreaks the C compiler by allowing signed overflows to do the obvious thing.
Frame pointers are old-school technology for most of the platforms, even in embedded usage. The libunwind library should be used instead, with the -funwind-tables or -fasynchronous-unwind-tables compiler options and additionally -rdynamic, -fno-dwarf2-cfi-asm, -fvar-tracking-assignments.
-rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use.
var-tracking-assignments in on by default when you use any variant of -g (which you should). You can also configure the targeted dwarf version there (which by default is 5 these days) and the level of debugging info, which you should set to the max. This makes your dwarf2-related options redundant.
-funwind-tables is also there by default in C++, not sure for C.
> -rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use.
-rdynamic exports global symbols from the main binary so that they're available to satisfy dependencies of dynamically linked libraries and modules. For example, a Perl or Lua interpreter, or an Apache daemon, will use -rdynamic so that its symbols can satisfy the dependencies of binary modules, rather than requiring the engine implementation to be linked via a shared library itself, which is more brittle. It's sort of midway between static and dynamic linking in terms of ensuring everybody is using the same implementation of a shared framework.
Maybe I'm missing something but isn't it redundant to have both stack and frame pointers? Just use negative offsets from the frame pointer to address local variables.
agreed on 64bit. Also this advise is old it hasn't popped up out of nowhere. I still recall it being crucial back in the early 2000's to improve performance. And today it remains very valid because of 32 bit IoT systems.
Right, on 32 bit you do want to omit frame pointers so %ebp can be used as a general register. (Fedora is on the verge of dropping 32 bit platforms entirely.)
Yes. Most calculations won't do the right thing with wrapping overflow and you'd be better of by them being undefined and tested via the relevant tooling (e.g. ubsan). Those that do should be written in a way to make that explicit.
Compilers can make commutative optimizations with signed integer overflow that is impossible with wrapping semantics. This is why Zig and Carbon both make unsigned integer overflow undefined as well. There is absolutely no advantage in most cases to wrapping semantics unless it is actually required for the algorithm (like some hashing or PRNG functions, for example).
In my opinion the improved code quality and bounds checking you can get with VLAs are worth it. Yes, the an attacker should not control the size and one should use -fstack-clash-protection and when the stack size is limited you should think twice (but vla can also reduce stack size if the alternative is a fixed size array on stack with larger fixed upper bound).
Looking at it from an attacker’s point of view, a VLA is a primitive for adding an arbitrary offset to the stack pointer: extremely powerful and dangerous.
You can’t use VLAs for large allocations because the stack has a limited amount of space and you don’t know what that limit is. So you must check the size is reasonable before declaring a VLA. So you might as well declare a fixed-size array that is large enough for all reasonable uses.
VLAs can enable precise bounds checking with the undefined behavior sanitizer. This is useful and you do not have precise bounds checking anymore when you replace it with a larger fixed size array. Also you increase stack size when you replace with a larger fixed size array.
Yes, one should not let an attacker control the size of the VLA. But in any case, one should use -fstack-clash-protection .
With stack clash protection if an attacker can control the size of VLA and it becomes to large, you can a trap with is likely DoS. With a fixed size array which you overflow instead, it is more likely a RCE. If you check the size, it does not matter.
That you can't use the stack for large allocations has nothing to with VLAs and also depends on what you call large and how large your stack is.
They were made optional in C11, while Google paid for the development effort to clean the Linux kernel of them, because they are only yet another source of possible CVEs.
Maybe you did not realize this, but I somewhat helped with this effort. In the kernel it may make (some) sense. The overall idea that VLA are always bad is incorrect.
> The overall idea that VLA are always bad is incorrect.
I mean, I'm yet to see a definitive example where automatic* VLA was actually right tool for the job.
I know they were supposedly introduced for the numerical analysis, but I fail to see what problem it actually did solve. Yes, the syntax way neater than piecemeal allocation, but it could have been solved with functions and macros anyway. Did VLA just hit some sweet spot in performance between fixed size arrays and heap-allocated ones, which made number crunching better optimized?
* as opposed to VM types used in function parameters and for allocating multi-dimensional arrays on heap
Stack allocation is much faster than heap allocation, and compared to fixed-size arrays VLAs save stack space. For example, a recursive divide-by-conquer algorithm that allocates arrays on the stack may use log(n) stack space compared to n². Using VLAs instead of the heap simplifies the logic because you get automatic deallocation with proper scoping. VLAs and VM were introduced as one feature, so I am not sure there was ever a the question of adding only one of them.
In terms of security, there are some issues but it is largely overblown and misunderstood. A dynamic buffer is always a sensitive piece of code when dealing with data from the network. VLA were the language tool of choice and then got a bad reputation because they were involved in many CVEs. But correlation is not causation. The main real issue is that - if the attacker can control the size of the VLA -, it is possible to overflow the stack into the heap. This can be avoided by using stack clash protection which compilers support only since a couple of years. With stack protection I believe VLAs can be safer than fixed size arrays due to improved bounds checking.
> The main real issue is that - if the attacker can control the size of the VLA -, it is possible to overflow the stack into the heap.
I think the main gripe most programmers have with VLA is lack of control over them. Fixed size array is, nomen omen, fixed and predictable - can even be tested beforehand. malloc() at least returns NULL on fail (although memory overcommitment muddies the situation), so program can take some action. But what happens when VLA fails? Segfault? Stack crash protection if compiled with it or worse if without? None of those options is graceful from the perspective of end user.
> This can be avoided by using stack clash protection which compilers support only since a couple of years.
As you yourself say, it's been only few years since this protection made its way into compilers. But that's not the issue. The issue is that `-fstack-clash-protection` isn't part of C language, it's part of compiler. What's the incentive for the developer to use less certain feature when there are easier alternatives?
Yes this is something we need to work on: Some way to detect stack overflow or prevent it from happening in C. And obviously you need to understand the resource consumption of your algorithm, but this also applied to other stack allocations and recursion etc. And with memory over-commitment you also do not have control over malloc anymore. You simply get a fault once you access the memory...
Your second point I do not understand: How is it the fault of the C language, if it is poorly implemented? And yes, if you use a poor implementation, you may need to avoid VLA. I am not blaming programmers who do this. My point is that it not inherently a bad language feature and with a good implementation it may also be better than the alternatives depending on the situation.
VLA are poorly defined feature for which some implementations offer additional protection.
And I agree, automatic VLA aren't inherently a bad feature; it's just a poor feature of C99, C11, C17 and sadly still C23*.
I have faith we will be able to finally tackle it in C2y, but until then, I'm with Eskil in opinion ISO C would be better off without them all those years.
* Thank you for your work on N3121, hopefully we will vote it in during next meeting :)
I do not think VLA are anymore poorly defined then most other stuff in the C standard. It generally allows a wide range of implementations. As a language features, it is has excellent properties. (automatic life time and dynamic bounds).
Similar to UB and many other related issues, the main issues are primarily about what compilers do or not do. I have countless examples were C compiler could easily warn about issues or be more safe, but don't. Very similar to the arguments against VLAs there are people who want to throw the complete C language away. The story is always the same: Compilers do a poor job, but the language is blamed. Then programmers blame WG14 instead of complaining to their vendor. I do not see how VLAs are any different in this regard compared to other parts of C.
And the answer can never be to go back on fundamentally good language feature (a bounded buffer - damit!) but always push towards better implementations.
And I am part of WG14 (so a hapless goldbrick). The reason VLA were made optional to make implementing C easier, as C99 was not adopted quickly. Although the reason for the slow adoption were others. MSVC did not really support C at all at C99 time because they thought everyone should transition to C++. Only very recently C support catched up in MSVC.
It basically shows that if you use automatic VLA you have can do some checks on VLA.
But if I prohibit automatic VLA altogether and use fixed size array, I don't need to worry about that at all and the example falls back to what I presented with UBSan doing just its regular thing.
If you have larger fixed size array you can get an error when you overflow the larger array. But this then does not necessarily prevent invalid accesses to the parts of the array which go beyond the actual size of what is stored in it or might not prevent bound overflows when copying from the array to some other place.
As I understand it the Clang static analyzer is not intended to be used directly. It's best to use it through CodeChecker, which provide high level commands and do the low level calls to the Clang SA (and also Clang tidy). In particular if you want whole project analysis (CTU, cross translation units analysis) CC will do the all the needed calls in a single "analyze" command.
I currently use `--analyze -Xanalyzer -analyzer-opt-analyze-headers` for checking my header-only libs in clang, but AFAIK the recommended workflow is to use clang-tidy which is better configurable than calling clang directly.
If you're on Mac with Xcode, the static analyzer and the sanitizers ASAN, UBSAN and TSAN are simple UI checkboxes.
That codechecker thing is sooo tedios to install. Why do I need to 70 MB of environment for some python script which just wraps clang tools anyway? :-(
I like scan-build better than codechecker, but codechecker is the only thing that does CTU, which is much more thorough.
That being said, codechecker is not that great in terms of it's interface. Over all, it really needs to be implemented in a GUI application as there are many many fine tuning flags you can set with the clang analyzer (as well as potential for parallelization) that either cant be taken advantage of, or is too cumbersome to use codechecker to do.
How do I use the sanitizers with freestanding targets? They seem to require a lot of library support which is unavailable on freestanding programs.
The article mentions the address sanitizer keeps track of allocations and their sizes. I wrote my own allocator, how do I integrate it with the sanitizer? I added malloc attributes to all allocation functions but they don't seem to do anything.
You need to provide implementations of the sanitizer callbacks yourself. For example, to use the address sanitizer you should use `-fsanitize=kernel-address`, and then the compiler will generate calls to `__asan_load8_noabort`, `__asan_load4_noabort`, `__asan_store8_noabort`... when loads/stores happen.
In those functions, you need to implement the sanitizer yourself, and have it panic if it detects an invalid access (address sanitizers are usually implemented with shadow memory). You'll have to give your sanitizer enough shadow memory during initialization, and also add your own alloc/free sanitizer callbacks to your allocator (so the sanitizer knows when things are allocated/freed).
I have an example [1] that implements a basic ASAN and UBSAN in a kernel (it's written in D, but could be easily adapted to C or C++). Hopefully it's helpful!
The flag applies the transitive property[1] when propagating constraints. This gives more accurate results, but at the cost of analyzer speed.
if (4 < x) {
if (x < y) {
if (y == 0) {
// Unreachable since the above three conditions cannot
// all be true at the same time (without multithreading).
}
}
}
For a more detailed explanation, see [2]. (Also the inspiration for the above example.)
-Wdouble-promotion can be a big help on microcontrollers with 32-bit FPUs. An accidental double promotion can be the difference between a single CPU instruction and an implicit function call to a built-in software computation. Not a nice thing to have in one of your inner loops!
Setting the compiler to treat floating-point literals as floats instead of doubles helps too, but I still add the trailing ‘f’ just to be on the safe side.
Another great UBSan feature is -fsanitize=integer which detects truncation and sign changes in implicit casts. But if you start using it, you'll realize that explicit casts enforced by -Wconversion only hide errors. You really have to remove as many explicit casts as possible to detect unwanted truncation.
After testing and fuzzing with -fsanitize=integer for a while, I came to the conclusion that implicit casts are a feature and C even has an advantage there compared to other languages which require explicit casts. To test for unwanted truncation, you really need two different casting operations. One which silently truncates in the rare cases where you really need it and another one which warns about truncation in test builds.
So my recommendation is: Remove all explicit casts and warnings like -Wconversion. Test with -fsanitize=integer. If UBSan detects truncation, you can often adjust some types so that no truncation happens. Only use explicit casts if you must convert between signed and unsigned chars for example.
Also the problem goes both way. I often see people trying to learn React with little grasp of JS, or Django like it's a language, and not something based on Python.
This problem of skill shortcuts is not confined to programming.
I work on networking. People always want to learn "packet capture analysis" because Wireshark has a cool name and looks impressive.
imo there is no such skill as packet capture analysis.
Once one learns the OSI model and a bunch of Ethernet and TCP/IP implementation behaviour, then what Wireshark presents becomes obvious and supported by that contextual knowledge.
That means building the code twice. In practice the only thing valgrind does that asan doesn't is checking for non-initialized variables. There's another sanitizer for this called msan.
Unfortunately not all of the sanitizers are working in clang for Apple Silicon machines. For my own CI pipeline I run both Darwin and Linux builds, leaks and valgrind respectively, along with the kitchen sink of compiler sanitizers:
Here’s the GitHub actions for those platforms and a link to the Makefile:
I worked with a programmer who, rather than fixing his code to address the problems these warnings are intended to catch, would sprinkle his code with #pragma or whatever incantation necessary to cause the compiler to ignore the problem code in that specific case. Thus when the automated builds ran, even with warnings on, it would never flag his bad code.
I get that sometimes there are circumstances where the warning is a false positive, as the article mentions, so these escape hatches are useful. It's not the case with my former co-worker who seemed to reflexively use these escape hatches whenever he didn't understand why the compiler didn't like his code. It didn't help that he was dependent on his IDE to hold his hand. I suspect today he'd just let one of the LLMs write his code for him.
I would expect forcibly overriding warnings to be a massive red flag in code review? Like, what company cares enough to care about warnings in CI but not catch people bypassing that check?
That's used in dev mode for various projects like GNU coreutils etc.
When distros etc. are building though these warnings are not used. It's worth stating as I've seen this issue many times, that -Werror at least should not be used by default for third parties building your code
I also find setting -std=c11 and -fpedantic very important, as otherwise you are writing against the gnu89 and don't get warnings about extension usage.
I used to have an environment variable defined $W_REALLY_ALL that turned on, really, all the warnings. It always drove me nuts that -Wall didn’t turn them all on and even -Wextra was insufficient. There are really very few false positives in the environments I’ve worked.
A lot of them are excessively pedantic. It is extremely for callbacks to have unused arguments. There is no point in writing extra code just to nullify ineffectual warnings. These are tools that serve us not the other way around.
Unused parameter warnings are pretty useful to remind you when a parameter is no longer needed after you change things or to catch some mispellings / missing code. You can always cast to (void) to "use" an otherwise uneeded parameter or remove the parameter name (or comment it out to keep tools checking declaration and definition name matches happy) keeping only the type.
That's true. There are some that are pedantic (-Wpedantic, anyone?) and excessive, but I find it's easier to evaluate those on a case-by-case basis, or opt-out of the ones that are generally useless, than it is to ignore an entire class of warnings.
For instance, a lot of projects I've worked on prefer to be ready for the day if/when the code has to be compiled with a different compiler, so -Wpedantic is great.
Clang doesn't turn on all warnings with -Wall because a lot of projects turn on -Wall -Werror by default, and then get mad when compiler updates break their builds. So -Wall -Werror has to remain reasonably stable release-over-release.
I think -Wno-sign-conversion is a bad idea if you like -Wconversion. It's either with or without conversion warnings, in my opinion, because while you do not lose bits when assigning an unsigned to an int (or vice versa), you do get a misinterpretation of the sign bit and thus a change of the conceptional value the bits represent. It's just highly annoying in C that no-one cares, because C integer typing is so lax by default.
Unlike recommended in the article, I started to enable -Wsign-conversion on my own code. At first it's very annoying because it's noisy and full of false positives (where the implicit sign conversion definitely can't cause any trouble), but it's definitely a good thing for becoming aware of places where integer promotion kicks in and might potentially cause unexpected results.
zig cc has great defaults, which includes UB sanitizers. Quite a few projects had bugs found and fixed just by compiling them with zig cc instead of gcc or clang.
People do not write makefiles anymore. At least not by hand.
Typical AAA gamedev project has at least 4 targets defined in the build system of choice. Each target gets a set of flags for each platform. Typically there are ways to define common sets of flags and use them in the specific target.
Half jokingly, choosing build system for a project is a very important step and one of things that kills desire to do C or C++ projects on the side. The article is a very friendly description of at least a decade of experience.
Debugging build systems that do a lot of things ( unity files, caching, multiplatform and distributed builds ) has become an art in itself. Personally I prefer to capture commandlines that are passed to compiler as result to make sure that options are correct.
…? Plenty of people write makefiles by hand. ./configure && make && make install is still a common method of doing build systems, even though there are a lot of new ways to also do builds now.
And some people hand-write Makefiles for some simple projects. But those are the output of configure scripts (or CMake files), so hand-writing a configure script (or CMakeLists.txt) isn't hand-writing the resulting Makefile. Writing Makefiles by hand for a single platform and target is easy, I do it for task automation, but it's unusably complex for multi-platform software with multiple build configurations.
Writing software is a team effort. You need to be on same page with your team. Which means one needs to learn 'one of a lot of new ways'.
In continuation of my AAA project example. For a standalone utility that targets only one Linux platform and will never need to consume libraries from AAA project a makefile is still acceptable.
Its quite simple. Games is what I do for living for quite some time. They are a decent specific example of a software project that employs lots of people who need to build software every day on a job. This way I can say what industry does instead of what I think it should do.
I just want to remind, typical GameDev experience can differ substantially from typical SoftDev experience, so I'd be cautious from extrapolating on whole industry.
I would go further and say that nowadays "people should not use C++ anymore". You need a very good reason to start a big new C or C++ project. Half of such projects should "just be Java" (or C#, maybe even Python, Julia or JS with Electron etc.). The ones where C/C++ would have originally really made sense should now be Rust or Zig.
Meta build systems like cmake allow defining different build types with their own compile settings. I wouldn't recommend using 'raw' Makefiles directly for C/C++ development.
Really wish gcc had a -std flag for "POSIX but no GNU extensions". Currently you're forced to choose between -std=cXX for the pure C standard or -std=gnuXX for C + POSIX + GNU extensions.
Exactly, these are libray extensions so they are controlled via defines and not (directly) via compiler flags. Which ones there are (and what they do) depend on the standard C library and not the compiler. For glibc: https://ftp.gnu.org/old-gnu/Manuals/glibc-2.2.3/html_node/li...
It's good practice to turn on all warnings but then what? If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to treat all warnings as errors.
That might sound drastic but that in fact enforces discipline and the removal of warnings and is IMHO very good practice especially if you start doing that from day one.
For legacy projects this can be a goal after an comprehensive phase of warnings removal.
I've worked on projects with and without "all warnings as errors" and ultimately it is well worth it.
I personally like turning on `-Werror=specific-warning` `-Werror=another-specific-warning`, for a long list of warnings. Then, if the compiler adds a new warning, you have a chance to review it and eliminate all instances of it before making it an error.
That doesn't prevent the compiler from expanding an existing warning, but that seems to break code less often than a blanket `-Werror`.
Yes, but don't turn it on except in developer builds. For downstream packagers it's a pain if a newer compiler adds a warning which then breaks the build.
it depends on the promises the maintainer makes to themselves and to their users. I'd rather break the build and get immediately alerted by (hopefully moderately) upset downstream packagers than having this warning become visible in my users builds.
It isn't just about getting rid of a warning, but the new compiler has changed some important logic so this warning might highlight potentially undefined behavior. Am I, the maintainer OK with that - that is the main question I believe.
Might be useful to do regression with any new compiler being released especially when a product is used in safety or security critical systems.
The trouble is a downstream packager might be compiling the whole distro -- 1000s of packages -- and they're not in a position to fix a new warning somewhere deep inside a program they don't know and probably don't use. They are more likely to hack the build system to add -Wno-error while cursing the upstream project.
This may be a controversial statement, but I don't think projects should distribute any flags aside from the bare minimum to get everything working. E.g.:
-std=c17 -lm
And let downstream add their own flags (-Owhatever, -fsomething,...) on top of that.
There are many more flags such as -ffast-math where the project will be better positioned to decide if they make sense than some random packager juggling hundreds of packages. Even for optimzation flags that don't change behavior there is no clear best set of flags that you should blindly apply to all projects.
The only way to ensure discipline and rid a large code base of warnings is to use -Werror
It may sound drastic until you're the poor intern that needs to refactor an active codebase with thousands of warning. Other than the technical debt it creates you're setting yourself up for having serious issue slipping through in all the noise.
"But some warnings might be false positives" ?
If one really must override the behavior it should be done explicitly and with a strong justification. This is possible either with a pragma[1]
#pragma GCC diagnostic ignored "-Wfoo"
or by globally[2] removing the entire warning category.
> The only way to ensure discipline and rid a large code base of warnings is to use -Werror
And this works as long as everybody has the same setup as you. This is unfortunately never the case. Kernel changes, libc changes, the compiler changes and sooner or later you will break the build.
> If the aim is no warnings, and that can even be a hard rule, then you also want to add -Werror to treat all warnings as errors
Hard disagree. No warnings means CI should fail (or otherwise scream loudly), not every build of your in-progress working tree. You can add a commit hook if you want but if it needs to be enforced then it isn't really discipline.
Even for CI you probably don't want -Werror but instead run the build completely so that you can track all warnings.
And for source released to others -Werror is downright rude as you have no idea what warnings their possible future compiler will have.
Exactly, turning on -Werror is good practice during development and when running CI, but not when building the source distribution of your code (because the user might have a slightly different version of the compiler which has different warnings).
Many replies seem to imply open source by default. Of course projects may have specific requirements, and open source is indeed a specific use case, but the majority of software is commercial, closed source.
Regarding compiler versions, many projects I worked on actually check that during the build because the code may only have been designed for or tested against a specific compiler/compiler version. In general upgrading the version of the compiler is an activity in itself because, indeed, this may require solving new warnings/compilation issues and a full test campaign.
> but the majority of software is commercial, closed source
That may be true for 'end user applications', but definitely not for libraries (thankfully). I use exactly zero closed source (precompiled) dependencies in my hobby and professional work because they are such pain to work with. Thankfully these days there's an open source alternative for pretty much everything, 15 years ago the situation was much more dire.
Oh you use plenty of closed source software in your daily life. The majority of software out there is closed source.
Edit: the applications you use on your desktop represent a very small portion of the software running in the world, including software that crosses your path on a daily basis and so 'software devlopment' is not synonymous with open source.
But when I look at the tools and applications I use daily, there's only two important applications that are closed source: VSCode and Chrome (and even this is debatable because both of those tools are just thin wrappers around Chromium: https://chromium.googlesource.com/chromium/src.git).
Changing defaults is hard and I can understand why compiler writers would be reluctant:
Many of the suggested warnings will reject valid-but-marginal code that you might not have time to rewrite (or might not be able to, eg. it's in an external library that you don't understand well).
Sanitisers and static analysers very often complain about quite valid code that doesn't need to be fixed at all.
Some hardening flags cause significant performance regressions.
Who is to say that -g3 is the best option? It causes huge binaries to be generated which might not be useful if you have other means to debug the code or can't/don't use gdb.
"Having strong opinions on which compiler flags to use" seems like it'd be a bit miserable. Flags should be for either unusual situations where you want to do something differently, or for situations where you're required to make a choice. If these flags are useful >90% of the time, why aren't they default?
