[GIT PULL] Enable -Wimplicit-fallthrough for Clang for 5.16-rc1

[GIT PULL] Enable -Wimplicit-fallthrough for Clang for 5.16-rc1

[Gustavo A. R. Silva]

The following changes since commit 3906fe9bb7f1a2c8667ae54e967dc8690824f4ea:

  Linux 5.15-rc7 (2021-10-25 11:30:31 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git tags/enable-clang-fallthrough-5.16-rc1

for you to fetch changes up to 428a8bf629ecc118d1eadbb629312c25fde2103f:

  Makefile: Enable -Wimplicit-fallthrough for Clang (2021-11-13 15:24:33 -0600)

----------------------------------------------------------------
Enable -Wimplicit-fallthrough for Clang for 5.16-rc1

Hi Linus,

Please, pull the following patch that enables -Wimplicit-fallthrough
for Clang 14+, globally.

We had almost 40,000[1] of these issues for Clang in the beginning,
and now I think we are in good shape and it is now possible to enable
-Wimplicit-fallthrough for Clang, with this finally getting rid of
the unintentional fallthrough bug-class in the kernel, entirely. :)

I have to say that I'm sending this pull-request this late in the
merge window, intentionally. I first wated to make sure that no other
warning shows up before sending it.

This patch has been baking in linux-next for a couple of developement
cycles, now. So, I think we are pretty much ready to merge it into
mainline.

[1] https://github.com/KSPP/linux/issues/115

Thanks!

----------------------------------------------------------------
Gustavo A. R. Silva (1):
      Makefile: Enable -Wimplicit-fallthrough for Clang

 Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

Re: [GIT PULL] Enable -Wimplicit-fallthrough for Clang for 5.16-rc1

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Sat, Nov 13, 2021 at 1:36 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> This patch has been baking in linux-next for a couple of developement
> cycles, now. So, I think we are pretty much ready to merge it into
> mainline.

Ugh. It's also very ugly.

Wouldn't something like this (TOTALLY UNTESTED!) work and do the right thing?

This seems like a natural for a Kconfig decision.

Also, does -Wimplicit-fallthrough=5 work with clang too? That would
simplify things a bit, and then we could just use a regular boolean
and do

  KBUILD_CFLAGS-$(CC_IMPLICIT_FALLTHROUGH) += -Wimplicit-fallthrough=5

in the Makefile, which is more in like with what we do for other
config-time cflags..

           Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1537 bytes --]

 Makefile     | 6 +-----
 init/Kconfig | 5 +++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index d83d72c26aaa..9cf9d5c1ed8f 100644
--- a/Makefile
+++ b/Makefile
@@ -789,7 +789,7 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 KBUILD_CFLAGS += $(stackp-flags-y)
 
 KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
-KBUILD_CFLAGS += $(KBUILD_CFLAGS-y)
+KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CPPFLAGS += -Qunused-arguments
@@ -801,10 +801,6 @@ KBUILD_CFLAGS += -Wno-gnu
 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,)
 # gcc inanely warns about local variables called 'main'
 KBUILD_CFLAGS += -Wno-main
 endif
diff --git a/init/Kconfig b/init/Kconfig
index 45bcaa8e7481..036b750e8d8a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -885,6 +885,11 @@ config ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
 config CC_HAS_INT128
 	def_bool !$(cc-option,$(m64-flag) -D__SIZEOF_INT128__=0) && 64BIT
 
+config CC_IMPLICIT_FALLTHROUGH
+	string
+	default "-Wimplicit-fallthrough=5" if CC_IS_GCC
+	default "-Wimplicit-fallthrough" if CC_IS_CLANG && $(cc-option,-Wunreachable-code-fallthrough)
+
 #
 # For architectures that know their GCC __int128 support is sound
 #

Re: [GIT PULL] Enable -Wimplicit-fallthrough for Clang for 5.16-rc1

[Gustavo A. R. Silva]

On Sat, Nov 13, 2021 at 03:20:19PM -0800, Linus Torvalds wrote:
> On Sat, Nov 13, 2021 at 1:36 PM Gustavo A. R. Silva
> <gustavoars@kernel.org> wrote:
[..]
> Wouldn't something like this (TOTALLY UNTESTED!) work and do the right thing?
> 
> This seems like a natural for a Kconfig decision.
> 
> Also, does -Wimplicit-fallthrough=5 work with clang too? That would
> simplify things a bit, and then we could just use a regular boolean
> and do
> 
>   KBUILD_CFLAGS-$(CC_IMPLICIT_FALLTHROUGH) += -Wimplicit-fallthrough=5

It doesn't work:

error: unknown warning option '-Wimplicit-fallthrough=5'; did you mean '-Wimplicit-fallthrough'? [-Werror,-Wunknown-warning-option]

However, your patch does work. :)

I'll send it as a proper patch, shortly.

Thanks
--
Gustavo