Hacker News new | ask | show | jobs
by mau 4502 days ago
Don't C compilers warn about code that is never executed?

I'm actually pretty impressed how this bug was not caught before. The compiler warning is just one thing that came to my mind but test coverage should have been the strongest hint. A linter could have also make the thing easier for a code review. Probably these parts of the code should have stricter coding guideline.

3 comments

> Don't C compilers warn about code that is never executed?

Not by default. On both GCC and Clang this kind of error isn't caught using -Wall or even -Wextra - you have to explicitly opt-in using -Wunreachable-code. It'd really be nice if that changed and Clang basically had close to -Weverything on by default and required you to opt out, preferably with something like `#pragma ALLOW_SHODDY_<category>` per file to put some pressure on C programmers not to just continue ignoring errors rather than fixing them.

In the latest GCC releases, -Wunreachable-code has been removed (the option still exists but is silently ignored for compat with existing Makefiles), as its output varied so much between releases (depending on what got optimized away).
I'd hope after this someone brings it back in some form – it's an incredibly useful tool and a diversity of implementations would be great for security-critical code.
You are 100% right. LLVM should fire a dead code warning here. However later code is used as a jump target for goto so it's quite hard for the compiler to infer this (or is it because its a different scope?).

Either way, definitely avoidable.

Looks typical of a merge cock up to me as the indentation is preserved suggesting it's a duplicate line or there was an if statement on the previous line that was removed.

The basic blocks from just after the unconditional goto up to the "fail:" label are all unreachable, so a dead code warning could be issued.

> However later code is used as a jump target for goto so it's quite hard for the compiler to infer this

Nope, because a jump target always starts a new basic block.

Looks to me like Apple aren't running Code-Coverage tests on their OS. That's kind of very scary.
I agree that they should. This is one of the types of errors that lint was traditionally used to find.