Revert "Makefile: Enable -Wimplicit-fallthrough for Clang"

This reverts commit b7eb335e26a9c7f258c96b3962c283c379d3ede0.

It turns out that the problem with the clang -Wimplicit-fallthrough
warning is not about the kernel source code, but about clang itself, and
that the warning is unusable until clang fixes its broken ways.

In particular, when you enable this warning for clang, you not only get
warnings about implicit fallthroughs.  You also get this:

   warning: fallthrough annotation in unreachable code [-Wimplicit-fallthrough]

which is completely broken becasue it

 (a) doesn't even tell you where the problem is (seriously: no line
     numbers, no filename, no nothing).

 (b) is fundamentally broken anyway, because there are perfectly valid
     reasons to have a fallthrough statement even if it turns out that
     it can perhaps not be reached.

In the kernel, an example of that second case is code in the scheduler:

                switch (state) {
                case cpuset:
                        if (IS_ENABLED(CONFIG_CPUSETS)) {
                                cpuset_cpus_allowed_fallback(p);
                                state = possible;
                                break;
                        }
                        fallthrough;
                case possible:

where if CONFIG_CPUSETS is enabled you actually never hit the
fallthrough case at all.  But that in no way makes the fallthrough
wrong.

So the warning is completely broken, and enabling it for clang is a very
bad idea.

In the meantime, we can keep the gcc option enabled, and make the gcc
build use

    -Wimplicit-fallthrough=5

which means that we will at least continue to require a proper
fallthrough statement, and that gcc won't silently accept the magic
comment versions. Because gcc does this all correctly, and while the odd
"=5" part is kind of obscure, it's documented in [1]:

  "-Wimplicit-fallthrough=5 doesn’t recognize any comments as
   fallthrough comments, only attributes disable the warning"

so if clang ever fixes its bad behavior we can try enabling it there again.

Link: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
Cc: Kees Cook <keescook@chromium.org>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/Makefile b/Makefile
index eaa6929..e97e754 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -797,6 +797,12 @@ KBUILD_CFLAGS += -Wno-gnu
 # source of a reference will be _MergedGlobals and not on of the whitelisted names.
 # See modpost pattern 2
 KBUILD_CFLAGS += -mno-global-merge
+else
+
+# Warn about unmarked fall-throughs in switch statement.
+# Disabled for clang while comment to attribute conversion happens and
+# https://github.com/ClangBuiltLinux/linux/issues/636 is discussed.
+KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=5,)
 endif
 
 # These warnings generated too much noise in a regular build.
@@ -977,9 +983,6 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 # warn about C99 declaration after statement
 KBUILD_CFLAGS += -Wdeclaration-after-statement
 
-# Warn about unmarked fall-throughs in switch statement.
-KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=5,$(call cc-option,-Wimplicit-fallthrough,))
-
 # Variable Length Arrays (VLAs) should not be used anywhere in the kernel
 KBUILD_CFLAGS += -Wvla