Useful GCC warning options not enabled by -Wall -Wextra
kristerw.blogspot.com
kristerw.blogspot.com
Why are those warnings not automatically included in "-Wall -Wextra"? Is there any issue with enabling them? Are these somewhat experimental?
How do these GCC warning compare to Clang warnings? Do these warnings exist there, too?
https://github.com/llvm-mirror/clang/blob/master/include/cla...
http://fuckingclangwarnings.com/ has descriptions of many of the warnings, but more recent warnings aren't covered because the website hasn't been updated since 2014.
I just found a GitHub repo that lists which warning flags are supported by different versions of clang and gcc:
Seriously?
Unfortunately there's no corresponding -Wuseless-parentheses, which if present would truly make -Weverything a "damned if you do, damned if you don't".
As you say, some warnings are contradictory and it's up to the project to decide which one of them is more important.
Because tons of people keep complaining "muh my build breaks if they enable new warnings". Dammit, idiots, if your build break there's a good chance your software is already broken too; that's the point of breaking the build: making bugs visible so that you have to fix them.
That's what this says at the bottom:
Maybe, but where's the problem ? Just add -Wno-effc++ afterwards.
As the other reply points out, you can just use -Wno-effc++ on top of -Weverything. You could also avoid that particular problem by not using C++ ;-).
At least, I would like an additional flag that really does enable all warnings, say -Wabsolutelyall or something like that.
It should also disable Werror, otherwise we will be claiming for some new flags in a few years.
When you compile with -Werror, you surrender control over your build's success to the whims of the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior. The personal opinions of future compiler authors shouldn't affect whether your program builds successfully. Sometimes warnings are bogus.
Sure, use -Werror in development. But don't make it the default.
I generally only leave them out in short simple to follow 'normal math' order of operations.
1. Dead/Duplicated Code.
2. Probably Broken Code.
Anything else is stylistic and the realm of opinion. Even if it's informed opinion enforcing it belongs in a linter.
There’s some practical and political difficulty with adding new warnings to a widely used compiler, partly because you don’t want to generate too many new warnings for existing code, partly because people compile with -Werror and upgrading the compiler will break (well, sprain) their build. Those difficulties would be easy to skirt by just tossing new warnings into lint mode and promoting them to compiler warnings if they turn out to be very useful in practice.
So, let's say that you find some 2003 software on some deep corner of SourceForge that you want to use.
Would you rather :
* Have the build fail because new warnings were introduced ? * Have the build succeed and then get crashes at runtime because the behaviour of the compiler changed anyways and the program was rendered invalid ?
I'll personally always want the first one.
No. The only thing that matters is that the language rules are respected. Or maybe you'd want programs that were correct with the first C compilers to still work today ? Because for instance they didn't do type checks at all. `void f(float, float); int x = f;` you say ? no problem! `char c; c[-15];` ? no problem! Everything did compile, sure. Is it the world we want ? Most certainly not.
* It's been some time since I last partook in language lawyering, and can't remember if standard compliant C compilers are allowed to produce diagnostics, the term used, for well-defined code as long as they also succeed.
The majority of the warning options showed in the article are intended to find unintended code (which can however perfectly defined if not intended behaviour as opposed to invalid, undefined behaviour). The very basic examples showed for those options also themselves do not contain anything that by itself would make the code not standard-compliant.
int err = 0;
if (err = f()) {
}
The whole point of undefined behaviour in C and C++ is to let the compiler cheat: ie a Java or Haskell compiler would have to take into account that (i < i + 1) can sometimes be wrong for native ints, and would have to prove that overflow can't happen in order to optimize this comparison away to True. Undefined behaviour in the standard frees C and C++ compilers from these obligations, and they can just assume overflow for signed ints won't happen.
These shortcuts (plus a lot of smarts) make it feasible to write a fast optimizing compiler with the 1970s state of the art in static analysis.
'Warning, you have undefined behavior', let the programmer decide what the intent of the section is and fix it.
I've been playing around with some code from 1993---the Viola web browser (one of the first graphical web browsers). Ooh boy was it fun when I ran it with "-pedantic -Wall -Wextra". Hundreds, if not thousands of warnings. A ton of unused argumetns, implicit declarations (no prototypes), unused variables, return type not specified, a few "control reaches end of non-void function," some format string mismatches. Pretty much what you would expect from K&R C (the code paid some lip service to ANSI C, but not a whole lot).
Also, the code runs (not well, but it does) on 32-bit systems, but immediately crashes on a 64-bit system. That's because of the whole "int==long==ptr==32 bits" that permeates the code.
And don’t worry, if the new idea I’m trying is worth it, I will fix it up so it doesn’t have any warnings. And I saved a lot of time by not fixing all these warnings on the previous 2 ideas I tried.
It’s just annoying to have to remove every unused variable/parameter when trying changes out.
Warnings in that case are still useful. If the project is otherwise warning-free then it's easy enough to spot the new warnings and judge if they represent a real mistake.
My preference today is to set -Werror on CI builds. Any code that's final should be warning-free, but it's unnecessarily painful for work-in-progress.
int foo = doSomething();
return 0; // Debug XXX DONOTSUBMIT
int bar = doSomethingElse();
return foo + bar;
int foo = doSomething();
if (1 > 0) return 0; // Debug XXX DONOTSUBMIT
int bar = doSomethingElse();
return foo + bar;
I am curious about this particular example. How does that ever make sense, where do you have that?
I'd accept that it would be nice to have memcpy(p1, p2, n) with valid pointers p1 and p2 to still be defined when n=0, because maybe n is variable and might work out to be 0 in some cases, which you then don't need to treat specially. I don't know whether that case is defined or not.
But in the case where either p1 or p2 are NULL, the memcpy will be illegal no matter what n is, so if p1 or p2 are variable you always have to check them anyway, and to run into the memcpy(0, 0, 0) case you would actually have to special case n=0 when p1 or p2 were NULL, which makes no sense.
Unless 0 is actually a valid address (kernel code with identity mapping at 0, simple platforms), but then NULL becomes a whole different beast.
I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?
I think that in some cases, the claim that undefined behavior allows for better optimization is bullshit.
That's not enough. For the case that resolves to memcpy(0, 0, 0), other_buffer is also 0. That means you need to guard against memcpy (0, buf, buffsz) with buff/buffsz != 0 somewhere anyway. Unless you are saying that the standard says that the more general memcpy(p, 0, 0) is undefined, which is a slightly different discussion.
> I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?
In any world where you need to e.g. lock the destination memory before accessing it, or do any other preparation/teardown. Not all platforms out there have one linear address space. In small scale applications, as just one example, it is not uncommon to have different pointer types for different types of memory, e.g. internal vs. external memory, I/O address spaces (lots of side effects there), maybe even a non-volatile memory address space.
In my opinion, it is easy and sensible to declare that memcpy(0,0,0) shall be undefined, especially because, again, no sensible program should ever reach that case without error (which, again, is different from the memcpy(p1, p2, 0) with p1/p2 != 0 case which can be sensible reached).
In fact, I'd argue that your proposal is the one that introduces the special case that all implementations would now have to take care of, adding complexity at best and a performance impact at worst, for something that a program won't ever do.
As for the distinction between a==0&&b==0 and a==0||b==0: first of all, the standard does make memcpy(a,b,0) undefined when a==0||b==0. Second, it's perfectly reasonable to run into a situation where both and b can be zero: consider the same demand-allocated buffer case. Why wouldn't you also want to demand-allocate the destination buffer?
It's common to see valid but semantically-meaningless constructs like that in autogenerated C code. Same with quite a few other cases mentioned in the article.
It's just not a good idea for a compiler author to arbitrarily start throwing warnings for memcpy(a,b,0) at this late date, regardless of a and b. That ship has sailed, leaks and all.
This means the code takes advantage of CPU pipelines and branch predictions, which means the source/destination pointer might be de-referenced and loaded into a register before the number of bytes is being inspected, or the assembly is executed with a 0 value, which still makes the CPU do a load or store on the src or dest pointer. This is being done to gain performance, and doing things in another order or explicitly validating the input decreases performance.
If then the src/dest pointer is NULL, the above scenario plays out badly, but in order to not sacrifice performance, the burden is placed on the caller to correctly use memcpy() instead of memcpy() detecting invalid use.
You might not agree with this as a design decision but I'm not arguing that this is the right trade-off, just answering:
> I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?
By my common sense, the answer is no.
In mathematics, this is called a discontinuity. Google the value of 0^0 for comparable discussion (it ‘is’ 1 because that makes more formulas look nice)
Otherwise you surrender control over the correctness of your program "to the whims of the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior".
More debatable decisions will be made in compiler implementations and I'd rather force the user who compiles the program to understand the implications of warnings by turning them into errors than letting them ignore them (because "it compiled, after all!").
If I was to make the call, I'd make "-Werror" default in all compilers, remove the option and allow to disable it by "--I-really-want-to-ignore-warnings-for-fscks-sake" which when used would sleep(30) after an all-caps nag screen that tells people "don't ever use that option."
There's practically no chance for a corrupted gzip-compressed tarball to decompress cleanly (even if by chance it inflates without error, there's a CRC32 of the uncompressed payload at the end of the gzip stream).
I don't want to keep posting this[1] in every C discussion, but when it's relevant, it's relevant. TL;DR A particular version of a compiler interpreted undefined behavior in such a way as to elide code that specifically tested for an error case to exit early because the error implied undefined behavior, so the test could never be reached without undefined behavior. If that sounds like circular logic, that's because it is.
The error happened to exist in a fairly specific version of clang shipped by apple, but not in any of the major release versions:
I was not able to reproduce this bug on other platforms, perhaps because the compiler optimization is not applied by other compilers. On Linux, I tested GCC 4.9, 5.4, and 6.3, as well as Clang 3.6, 3.8, and 4.0. None were affected. Nevertheless, the compiler behavior is technically allowed, thus it should be assumed that it can happen on other platforms as well.
The specific compiler version which was observed to be affected is:
$ clang++ --version
Apple LLVM version 8.1.0 (clang-802.0.41)
Target: x86_64-apple-darwin16.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
(Note: Despite being Clang-based, Apple's compiler version numbers have no apparent relationship to Clang version numbers.)
Anyway, I think even more likely than running into new compiler warnings/errors is running into external dependency issues, which require you to touch the software anyway, and, I'd argue, are often much harder to fix than fixing code that triggers warnings.
More seriously, software certainly wears, not relative to itself but relative to the environment it is run in. It wears in the sense that active maintenance operations are needed to have old software perform as it should: installing old libraries, running it in emulators, etc.
warning: ISO C forbids assignment between function pointer and ‘void *’ [-pedantic]
If not... different developers have different errors.
I try to build with all of the warnings turned on, multiple static analysis tools, and with multiple OS / compilers. Yet every time a compiler or OS is upgraded, there's a whole suite of new build warnings.
Static analysis tools are a different problem: I find that most of them have so many false positives they are not worth looking at.
I'm wondering what it is about HN that lets people be condescending know-it-alls.
If you look, there's only one of me online. I write between 10K and 20K lines of C a year.
So yes, I have tried this in the last 10 years.
People that decide what memcpy does, have vastly different problems in mind (and hardware realities) from people complaining about it when using it on <random platform>.
You mean the people who write the specs of the language? Well you surrender control to these folks when you decided to pick this language!
Otherwise the problem about memcpy of zero-size and nullptr is unfortunate from compiler engineers (at least clang engineers) point of view. The story is that the compiler is optimizing based on programmer annotations: after all if a programmer takes the effort to annotate their APIs, let's benefit from this. However the GNU libc annotated `memcpy` (and others) with this non_null attribute, causing this fragility on invalid (according to the spec) but common code.
Opinion of clang authors is somehow "Deleting a null pointer check on p after a memcpy(p, q, 0) call seems extremely user-hostile, and very unlikely to result in a valuable improvement to the program".
Clang dev thread: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066....
Note in this thread that clang devs are trying to get the language changed to remove this UB.
Why would upgrading a compiler be any different?
People who do production builds with -Wall and -Werror ruin it for the rest of us.
In OCaml there is a very simple rule: Development builds run with "-warn-error", which is OCaml-speak for "-Werror". Production builds run without that option - simply because future version of ocamlc might introduce new warnings that would make the build fail.
This makes a lot of sense to me.
Why did the C/C++ ecosystem adopt a different aprroach to production build compiler options?
(I'm not saying that these warnings are pointless, but you can have a bug-free code base that triggers a lot of these)
Not all of them are actually errors, maybe depending on your style. For example, shadowing a variable is a well-defined feature of the C language, and so some code patterns take advantage of that fact. Other people avoid shadowing like the plague.
I think a lot of C++ devs will disagree with this. It's also very uncommon from my experience.
One could argue that this should be solved by semantic highlighting or naming conventions instead.
First of all, it's not even a substitute. m_foo won't work if it's declared in a class based on a template parameter; you'll still need a qualifier. And suddenly that makes the code inconsistent, which in itself is kind of bad. Inconsistency makes it harder to read code and spot mistakes. And why should how you refer to a variable change based on the mere fact that the class containing it takes a template parameter?
Second, there is really no good reason why you should have to change
class C { C(int width) : value(width) { } int width; };
class C { C(int width) : m_width(width) { } int m_width; };
Furthermore, there is no reason to encode the member-ness of a variable into its name. What's so special about that information? Should we start encoding the access specifiers too (private/public/etc.)? What about whether it's a class member? s_m_priv_width instead of just width? Why can't we just call the variable whatever darn thing it represents?
And my tongue is halfway in my cheek here, but this feels akin to naming your American kid AmeriAlex instead of just Alex. What sense does that make?
Finally, let's get semantic highlighting out of the way -- it fundamentally cannot with C++ syntax or with the C preprocessor, and it's twice as impossible with the two. It's never clear what m_foo refers to. Is it a global or is it in the base class? If it's in a header file included by two other .c files, and the preceding header files declare different base classes, what color should it have? It would refer to different things (maybe in one case in a namespace, and in another case in a class). Coloring is just an unsolvable problem in this language; it's not reliable in telling you where a variable referred to is declared.
> class C { C(int width) : value(width) { } int width; };
You mean to write
class C { C(int width) : width(width) { } int width; };
Here's an example I can think of:
b = (a == 0) ? 42 : 42;
But let's be creative ... What if each of those 42s are expansions of different macros that vary based on an ifdef? You could have one platform or configuration where it expands to ? 43 : 42, and another where it expands to ? 42 : 42. Since the preprocessor does those expansions early, your compiler doesn't know the difference at the time of generating the warning. I just confirmed this in a small program, replacing the first 42 with FOO, the 2nd with BAR, and #defining them both to 42:
warn.c:9:23: warning: this condition has identical branches [-Wduplicated-branches]
b = (a == 0) ? FOO : BAR;
b = 42
-Werror=switch
Using this, you can enforce that a switch statement over an enum value is complete (covers all cases) simply by omitting the default case.
This is not about the coding police coming to get you but preventing you from shooting yourself into your feet when you extend the enumeration. Quite often you want to cover all cases and write code that signals an error if the default branch is called. CL even has a special version of the CASE macro (CL's "switch") called ECASE[1] with the only difference being that ECASE signals an error on unhandled cases.
[1] http://www.lispworks.com/documentation/HyperSpec/Body/m_case...
This.
And not necessarily yourself, but whomever maintains the software after you.
Of course this doesn't prevent someone from adding a "default" label in some future iteration.
There are exceptions to this "some cases are no-ops" thing. But I think it's the minority of switches I've seen.
From that perspective it does seem like the coding police angle. It's reminiscent of certain things in Java, like making a bunch of implicit conversions into errors, or C#, which forbids implicit fall-through on a switch. Those too are for safety and maintainability. They also make a lot of constructs more ugly. My question is: is it worth it? In this case (unintended pun) I don't think so.
Then you use:
default: break;
People generally "fix" warnings by writing equally useless statements like "x = x" (common for -Wunused). I've seen codebases which aim for zero warnings, but with -Wextra/-Weverything it's just silly. Warnings are supposed to be just that: warnings.
I'd rather want a clean way to suppress the warning and mark it as such instead, so that's hidden by default when the programmer takes note of it.
We have push/pop pragmas, but they are even worse readability-wise. I'd rather have a neighboring comment with dedicated syntax instead.
I like your idea of marking warnings. Warnings should be treated like the messages from any static analysis tool. A lot of times they are worth at least looking at but you should be able to mark them as "not a problem" if you think you know better. That way you will always look at new warnings and not end up ignoring all warnings because there are too many that are irrelevant.
I recall a bug report with quite the flame war in it, but couldn't find it.
-Wimplicit-function-declaration: warn when a function is used without being declared (not needed in -std=c99 mode, but I write in c90). Catches functions not being declared in header files, which would result in broken type checking.
-Wmissing-prototypes: external function is defined without a previous prototype declaration. This catches functions that should be static: you fix it by making the function static, or else by prototyping it (in a header) so that it is properly public.
-Wstrict-prototypes: catches accidental old-style definitions or declarations, like void foo(), which isn't a prototype.
Actually it could be much more interesting than that -- the implicit function decl will be `int foo(int)` so you risk your compiler pushing the wrong args on the stack.
I find clang's behavior particularly dangerous for warnings referring to removal of null pointer checks, which are undefined behavior (if (this != nullptr)...)
There should perhaps be an option to control this.
If you code runs fine with two or three different compilers you can also be sure that you don't rely on undefined or compiler specific behaviour.
It's the same reason some multiplatform libraries are extremely battle tested and comparatively bug free when compared to single platform code.
This probes for:
checking whether C compiler accepts -Werror=unknown-warning-option... yes
checking whether C compiler accepts -Wno-suggest-attribute=format... yes
checking whether C compiler accepts -fno-strict-aliasing... yes
checking whether C compiler accepts -Wall... yes
checking whether C compiler accepts -Wextra... yes
checking whether C compiler accepts -Wundef... yes
checking whether C compiler accepts -Wnested-externs... yes
checking whether C compiler accepts -Wwrite-strings... yes
checking whether C compiler accepts -Wpointer-arith... yes
checking whether C compiler accepts -Wmissing-declarations... yes
checking whether C compiler accepts -Wmissing-prototypes... yes
checking whether C compiler accepts -Wstrict-prototypes... yes
checking whether C compiler accepts -Wredundant-decls... yes
checking whether C compiler accepts -Wno-unused-parameter... yes
checking whether C compiler accepts -Wno-missing-field-initializers... yes
checking whether C compiler accepts -Wdeclaration-after-statement... yes
checking whether C compiler accepts -Wformat=2... yes
checking whether C compiler accepts -Wold-style-definition... yes
checking whether C compiler accepts -Wcast-align... yes
checking whether C compiler accepts -Wformat-nonliteral... yes
checking whether C compiler accepts -Wformat-security... yes
checking whether C compiler accepts -Wsign-compare... yes
checking whether C compiler accepts -Wstrict-aliasing... yes
checking whether C compiler accepts -Wshadow... yes
checking whether C compiler accepts -Winline... yes
checking whether C compiler accepts -Wpacked... yes
checking whether C compiler accepts -Wmissing-format-attribute... yes
checking whether C compiler accepts -Wmissing-noreturn... yes
checking whether C compiler accepts -Winit-self... yes
checking whether C compiler accepts -Wredundant-decls... (cached) yes
checking whether C compiler accepts -Wmissing-include-dirs... yes
checking whether C compiler accepts -Wunused-but-set-variable... no
checking whether C compiler accepts -Warray-bounds... yes
checking whether C compiler accepts -Wimplicit-function-declaration... yes
checking whether C compiler accepts -Wreturn-type... yes
checking whether C compiler accepts -Wswitch-enum... yes
checking whether C compiler accepts -Wswitch-default... yes
checking whether C compiler accepts -Wno-suggest-attribute=format... no
checking whether C compiler accepts -Wno-error=unused-parameter... yes
checking whether C compiler accepts -Wno-error=missing-field-initializers... yes
checking whether C compiler accepts -Werror=unknown-warning-option... (cached) yes
checking whether the linker accepts -Wl,--no-as-needed... no
checking whether the linker accepts -Wl,--fatal-warnings... no
checking whether the linker accepts -Wl,-fatal_warnings... yes
checking whether the linker accepts -Wl,-fatal_warnings... yes