I work at Red Hat on GCC, the GNU Compiler Collection. The next major release of GCC, GCC 6, is just around the corner, so I thought I'd post about a new compiler warning I've contributed to it:
-Wmisleading-indentation
.
One of the more common "gotchas" in C and C++ programming relates to missing braces. For example, in the following:
if (some_condition ()) do_foo (); do_bar ();
the "do_bar ();" looks like it's guarded by the conditional, but it's actually outside of it.
Similarly, in this code fragment:
for (i = 0; i < n; i++) sum[i] = a[i] + b[i]; prod[i] = a[i] * b[i];
the computation of prod[i] is actually outside of the loop, despite what the indentation might suggest.
Perhaps the most famous example of this is the so-called "goto fail" vulnerability that affected OS X's SSL implementation, aka CVE-2014-1266:
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; goto fail; /* more checking here. */ fail: /* cleanups */ return err;
where the 2nd goto fail;
was always executed, leading to signature-checking code being skipped, with "err" set to 0, effectively leading to invalid certificates being accepted as valid.
This felt to me like something the compiler ought to warn about, so for gcc 6 I've written a new warning: -Wmisleading-indentation
.
The new warning is issued when the indentation of the code might mislead a human reader about the code's true block structure.
For example, given CVE-2014-1266 above, gcc 6 will issue this:
sslKeyExchange.c: In function 'SSLVerifySignedServerKeyExchange': sslKeyExchange.c:631:8: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] goto fail; ^~~~ sslKeyExchange.c:629:4: note: ...this 'if' clause, but it is not if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) ^~
Similarly, for the second example above:
test.c: In function ‘example_2’: test.c:57:5: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] prod[i] = a[i] * b[i]; ^~~~ test.c:55:3: note: ...this ‘for’ clause, but it is not for (i = 0; i < n; i++) ^~~
At a high level, the underlying implementation looks at control statements (if/else, while, for), and if it sees them guard a single statement without braces, it looks at the followup statement. It complains if both have the same indentation.
That's a simplified description - we spent a fair amount of time working on heuristics in the warning, to try to ensure that it warns for all cases that are reasonable to warn for, whilst not complaining unduly for indentation that's merely bad (rather than being actively misleading). We've also tested it with a variety of coding styles: GNU, K&R, Linux kernel, etc.
For example, it will warn about poorly-written one-liners:
if (flag) x++; y++;
like this:
test.c: In function ‘example_3’: test.c:25:10: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] x++; y++; ^ test.c:24:3: note: ...this ‘if’ clause, but it is not if (flag) ^~
but it doesn't warn for things like:
void all_on_one_line (void) { foo (0); if (flagA) foo (1); foo (2); foo (3); }
I added the new -Wmisleading-indentation warning to "-Wall" in gcc 6's development tree in December, and it's been finding real world bugs, for example one in elfutils: https://gnu.wildebeest.org/blog/mjw/2016/01/09/looking-forward-to-gcc6-nice-new-warnings/
and a couple in the Linux kernel's "perf" tool:
* https://lkml.org/lkml/2015/12/14/460
* https://lkml.org/lkml/2015/12/14/461
GCC 6 will be available in Fedora 24:
https://fedoraproject.org/wiki/Changes/GCC6
and in other fast-moving distributions.
Alternatively, if you're feeling adventurous, you can download a development snapshot of gcc 6 from https://gcc.gnu.org/snapshots.html
Enjoy - I hope you don't find any such warnings in your code!
Last updated: April 5, 2018