Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 16 Jul 2021 12:22:31 -0700 | Subject | Re: [GIT PULL] fallthrough fixes for Clang for 5.14-rc2 |
| |
On Fri, Jul 16, 2021 at 11:47 AM Nathan Chancellor <nathan@kernel.org> wrote: > > I am not really sure how to resolve that within checkFallThroughIntoBlock() or > fillReachableBlocks() but given that this is something specific to the kernel,
It's not at all specific to the kernel. Yes, the particular example was from the kernel, but the issue is very much generic.
Yes, that particular example was from the kernel and used a CONFIG option.
But I can actually point to user-space code that looks very much like it:
https://sources.debian.org/src/libreoffice/1:7.0.4-4/stoc/source/simpleregistry/simpleregistry.cxx/?hl=223#L223
look at that code, and tell me it makes sense.
You want to have the fallthrough for the case where abort() isn't marked as noreturn, but you don't want to get a warning for the case where a compile environment *does* have that noreturn thing.
See the issue? EXACT SAME THING.
This is in no way kernel-specific. The fact is, code can be unreachable without it being a bug.
A common example of unreachable code is things like this:
https://sources.debian.org/src/apparmor/2.13.6-10/parser/libapparmor_re/chfa.cc/?hl=338#L338
Look, it's a "switch (sizeof())", which means that only one of the cases is ever going to be reachable.
That code doesn't actually use "[[fallthrough]]" right now, and just uses the implicit fallthrough. But imagine if it was converted to use that fallthrough annotation. If the "sizeof()" isn't the largest size, those fallthrough's will be fundamentally unreachable, because the whole case is unreachable.
Warning about unreachable code is simply WRONG. It happens very naturally in C, exactly becuse people do conditionals based on compile-time constants. Those compile-time constants may be about things like "sizeof", they may be about things like that "abort() may be no-return or not".
But it can also easily be about patterns where you always check error returns, and some functions are inline and never (or always) return errors, so that your code ends up having stuff that is just statically always true (or always false), and then the implication is that there is unreachable code that the compiler will just compile away.
And no, this is in no way kernel-specific at all.
That warning needs
(a) a different flag - because "warn about unreachable" is completely different from "warn about implicit fallthrough"
(b) point to where the warning is
but honestly, it would be better to just remove the warning entirely, because it is just fundamentally wrong for all the reasons outlined above.
Linus
| |