Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] kbuild: Change fallthrough comments to attributes
@ 2019-08-12 21:47 Nathan Huckleberry
  2019-08-12 22:14 ` [PATCH v2] " Nathan Huckleberry
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Nathan Huckleberry @ 2019-08-12 21:47 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml
  Cc: linux-kbuild, linux-kernel, linux-mm, clang-built-linux,
	Nathan Huckleberry

Clang does not support the use of comments to label
intentional fallthrough. This patch replaces some uses
of comments to attributesto cut down a significant number
of warnings on clang (from ~50000 to ~200). Only comments
in commonly used header files have been replaced.

Since there is still quite a bit of noise, this
patch moves -Wimplicit-fallthrough to
Makefile.extrawarn if you are compiling with
clang.

Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
 Makefile                   |  4 +++
 include/linux/jhash.h      | 60 ++++++++++++++++++++++++++++----------
 include/linux/mm.h         |  9 ++++--
 include/linux/signal.h     | 14 +++++----
 include/linux/skbuff.h     | 12 ++++----
 lib/zstd/bitstream.h       | 10 +++----
 scripts/Makefile.extrawarn |  3 ++
 7 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/Makefile b/Makefile
index c391fbb07195..875930c19619 100644
--- a/Makefile
+++ b/Makefile
@@ -847,7 +847,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 KBUILD_CFLAGS += -Wdeclaration-after-statement
 
 # Warn about unmarked fall-throughs in switch statement.
+# If the compiler is clang, this warning is only enabled if W=1 in
+# Makefile.extrawarn
+ifndef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
+endif
 
 # Variable Length Arrays (VLAs) should not be used anywhere in the kernel
 KBUILD_CFLAGS += -Wvla
diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index ba2f6a9776b6..6cb2381501d1 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
 	}
 	/* Last block: affect all 32 bits of (c) */
 	switch (length) {
-	case 12: c += (u32)k[11]<<24;	/* fall through */
-	case 11: c += (u32)k[10]<<16;	/* fall through */
-	case 10: c += (u32)k[9]<<8;	/* fall through */
-	case 9:  c += k[8];		/* fall through */
-	case 8:  b += (u32)k[7]<<24;	/* fall through */
-	case 7:  b += (u32)k[6]<<16;	/* fall through */
-	case 6:  b += (u32)k[5]<<8;	/* fall through */
-	case 5:  b += k[4];		/* fall through */
-	case 4:  a += (u32)k[3]<<24;	/* fall through */
-	case 3:  a += (u32)k[2]<<16;	/* fall through */
-	case 2:  a += (u32)k[1]<<8;	/* fall through */
-	case 1:  a += k[0];
+	case 12:
+		c += (u32)k[11]<<24;
+		__attribute__((fallthrough));
+	case 11:
+		c += (u32)k[10]<<16;
+		__attribute__((fallthrough));
+	case 10:
+		c += (u32)k[9]<<8;
+		__attribute__((fallthrough));
+	case 9:
+		c += k[8];
+		__attribute__((fallthrough));
+	case 8:
+		b += (u32)k[7]<<24;
+		__attribute__((fallthrough));
+	case 7:
+		b += (u32)k[6]<<16;
+		__attribute__((fallthrough));
+	case 6:
+		b += (u32)k[5]<<8;
+		__attribute__((fallthrough));
+	case 5:
+		b += k[4];
+		__attribute__((fallthrough));
+	case 4:
+		a += (u32)k[3]<<24;
+		__attribute__((fallthrough));
+	case 3:
+		a += (u32)k[2]<<16;
+		__attribute__((fallthrough));
+	case 2:
+		a += (u32)k[1]<<8;
+		__attribute__((fallthrough));
+	case 1:
+		a += k[0];
 		 __jhash_final(a, b, c);
+		break;
 	case 0: /* Nothing left to add */
 		break;
 	}
@@ -132,10 +156,16 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 
 	/* Handle the last 3 u32's */
 	switch (length) {
-	case 3: c += k[2];	/* fall through */
-	case 2: b += k[1];	/* fall through */
-	case 1: a += k[0];
+	case 3:
+		c += k[2];
+		__attribute__((fallthrough));
+	case 2:
+		b += k[1];
+		__attribute__((fallthrough));
+	case 1:
+		a += k[0];
 		__jhash_final(a, b, c);
+		break;
 	case 0:	/* Nothing left to add */
 		break;
 	}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0334ca97c584..52d99f263ca3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -158,11 +158,14 @@ static inline void __mm_zero_struct_page(struct page *page)
 
 	switch (sizeof(struct page)) {
 	case 80:
-		_pp[9] = 0;	/* fallthrough */
+		_pp[9] = 0;
+		__attribute__((fallthrough));
 	case 72:
-		_pp[8] = 0;	/* fallthrough */
+		_pp[8] = 0;
+		__attribute__((fallthrough));
 	case 64:
-		_pp[7] = 0;	/* fallthrough */
+		_pp[7] = 0;
+		__attribute__((fallthrough));
 	case 56:
 		_pp[6] = 0;
 		_pp[5] = 0;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index b5d99482d3fe..4fb0a0041a37 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -129,11 +129,11 @@ static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
 		b3 = b->sig[3]; b2 = b->sig[2];				\
 		r->sig[3] = op(a3, b3);					\
 		r->sig[2] = op(a2, b2);					\
-		/* fall through */					\
+		__attribute__((fallthrough));				\
 	case 2:								\
 		a1 = a->sig[1]; b1 = b->sig[1];				\
 		r->sig[1] = op(a1, b1);					\
-		/* fall through */					\
+		__attribute__((fallthrough));				\
 	case 1:								\
 		a0 = a->sig[0]; b0 = b->sig[0];				\
 		r->sig[0] = op(a0, b0);					\
@@ -163,9 +163,9 @@ static inline void name(sigset_t *set)					\
 	switch (_NSIG_WORDS) {						\
 	case 4:	set->sig[3] = op(set->sig[3]);				\
 		set->sig[2] = op(set->sig[2]);				\
-		/* fall through */					\
+		__attribute__((fallthrough));				\
 	case 2:	set->sig[1] = op(set->sig[1]);				\
-		/* fall through */					\
+		__attribute__((fallthrough));				\
 	case 1:	set->sig[0] = op(set->sig[0]);				\
 		    break;						\
 	default:							\
@@ -186,7 +186,7 @@ static inline void sigemptyset(sigset_t *set)
 		memset(set, 0, sizeof(sigset_t));
 		break;
 	case 2: set->sig[1] = 0;
-		/* fall through */
+		__attribute__((fallthrough));
 	case 1:	set->sig[0] = 0;
 		break;
 	}
@@ -199,7 +199,7 @@ static inline void sigfillset(sigset_t *set)
 		memset(set, -1, sizeof(sigset_t));
 		break;
 	case 2: set->sig[1] = -1;
-		/* fall through */
+		__attribute__((fallthrough));
 	case 1:	set->sig[0] = -1;
 		break;
 	}
@@ -230,6 +230,7 @@ static inline void siginitset(sigset_t *set, unsigned long mask)
 		memset(&set->sig[1], 0, sizeof(long)*(_NSIG_WORDS-1));
 		break;
 	case 2: set->sig[1] = 0;
+		__attribute__((fallthrough));
 	case 1: ;
 	}
 }
@@ -242,6 +243,7 @@ static inline void siginitsetinv(sigset_t *set, unsigned long mask)
 		memset(&set->sig[1], -1, sizeof(long)*(_NSIG_WORDS-1));
 		break;
 	case 2: set->sig[1] = -1;
+		__attribute__((fallthrough));
 	case 1: ;
 	}
 }
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d8af86d995d6..4a1df6714dfe 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3639,19 +3639,19 @@ static inline bool __skb_metadata_differs(const struct sk_buff *skb_a,
 #define __it(x, op) (x -= sizeof(u##op))
 #define __it_diff(a, b, op) (*(u##op *)__it(a, op)) ^ (*(u##op *)__it(b, op))
 	case 32: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case 24: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case 16: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case  8: diffs |= __it_diff(a, b, 64);
 		break;
 	case 28: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case 20: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case 12: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__attribute__((fallthrough));
 	case  4: diffs |= __it_diff(a, b, 32);
 		break;
 	}
diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
index 3a49784d5c61..cc311bae44da 100644
--- a/lib/zstd/bitstream.h
+++ b/lib/zstd/bitstream.h
@@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s
 		bitD->bitContainer = *(const BYTE *)(bitD->start);
 		switch (srcSize) {
 		case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
-			/* fall through */
+			__attribute__((fallthrough));
 		case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
-			/* fall through */
+			__attribute__((fallthrough));
 		case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
-			/* fall through */
+			__attribute__((fallthrough));
 		case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24;
-			/* fall through */
+			__attribute__((fallthrough));
 		case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16;
-			/* fall through */
+			__attribute__((fallthrough));
 		case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8;
 		default:;
 		}
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index a74ce2e3c33e..e12359d69bb7 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -30,6 +30,9 @@ warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-option, -Wunused-const-variable)
 warning-1 += $(call cc-option, -Wpacked-not-aligned)
 warning-1 += $(call cc-option, -Wstringop-truncation)
+ifdef CONFIG_CC_IS_CLANG
+KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
+endif
 # The following turn off the warnings enabled by -Wextra
 warning-1 += -Wno-missing-field-initializers
 warning-1 += -Wno-sign-compare
-- 
2.23.0.rc1.153.gdeed80330f-goog



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-12 21:47 [PATCH] kbuild: Change fallthrough comments to attributes Nathan Huckleberry
@ 2019-08-12 22:14 ` " Nathan Huckleberry
  2019-08-12 22:40   ` Joe Perches
  2019-08-12 23:06 ` [PATCH] " Nick Desaulniers
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Nathan Huckleberry @ 2019-08-12 22:14 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml
  Cc: linux-kbuild, linux-kernel, linux-mm, clang-built-linux,
	Nathan Huckleberry

Clang does not support the use of comments to label
intentional fallthrough. This patch replaces some uses
of comments to attributesto cut down a significant number
of warnings on clang (from ~50000 to ~200). Only comments
in commonly used header files have been replaced.

Since there is still quite a bit of noise, this
patch moves -Wimplicit-fallthrough to
Makefile.extrawarn if you are compiling with
clang.

Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
 Makefile                            |  4 ++
 include/linux/compiler_attributes.h |  4 ++
 include/linux/jhash.h               | 60 +++++++++++++++++++++--------
 include/linux/mm.h                  |  9 +++--
 include/linux/signal.h              | 14 ++++---
 include/linux/skbuff.h              | 12 +++---
 lib/zstd/bitstream.h                | 10 ++---
 scripts/Makefile.extrawarn          |  3 ++
 8 files changed, 81 insertions(+), 35 deletions(-)

diff --git a/Makefile b/Makefile
index 1b23f95db176..93b9744e66a2 100644
--- a/Makefile
+++ b/Makefile
@@ -846,7 +846,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 KBUILD_CFLAGS += -Wdeclaration-after-statement
 
 # Warn about unmarked fall-throughs in switch statement.
+# If the compiler is clang, this warning is only enabled if W=1 in
+# Makefile.extrawarn
+ifndef CONFIG_CC_IS_CLANG
 KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
+endif
 
 # Variable Length Arrays (VLAs) should not be used anywhere in the kernel
 KBUILD_CFLAGS += -Wvla
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 6b318efd8a74..86c26bc0ace5 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -253,4 +253,8 @@
  */
 #define __weak                          __attribute__((__weak__))
 
+#if __has_attribute(fallthrough)
+#define __fallthrough                   __attribute__((fallthrough))
+#endif
+
 #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
diff --git a/include/linux/jhash.h b/include/linux/jhash.h
index ba2f6a9776b6..1d21e3f32823 100644
--- a/include/linux/jhash.h
+++ b/include/linux/jhash.h
@@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
 	}
 	/* Last block: affect all 32 bits of (c) */
 	switch (length) {
-	case 12: c += (u32)k[11]<<24;	/* fall through */
-	case 11: c += (u32)k[10]<<16;	/* fall through */
-	case 10: c += (u32)k[9]<<8;	/* fall through */
-	case 9:  c += k[8];		/* fall through */
-	case 8:  b += (u32)k[7]<<24;	/* fall through */
-	case 7:  b += (u32)k[6]<<16;	/* fall through */
-	case 6:  b += (u32)k[5]<<8;	/* fall through */
-	case 5:  b += k[4];		/* fall through */
-	case 4:  a += (u32)k[3]<<24;	/* fall through */
-	case 3:  a += (u32)k[2]<<16;	/* fall through */
-	case 2:  a += (u32)k[1]<<8;	/* fall through */
-	case 1:  a += k[0];
+	case 12:
+		c += (u32)k[11]<<24;
+		__fallthrough;
+	case 11:
+		c += (u32)k[10]<<16;
+		__fallthrough;
+	case 10:
+		c += (u32)k[9]<<8;
+		__fallthrough;
+	case 9:
+		c += k[8];
+		__fallthrough;
+	case 8:
+		b += (u32)k[7]<<24;
+		__fallthrough;
+	case 7:
+		b += (u32)k[6]<<16;
+		__fallthrough;
+	case 6:
+		b += (u32)k[5]<<8;
+		__fallthrough;
+	case 5:
+		b += k[4];
+		__fallthrough;
+	case 4:
+		a += (u32)k[3]<<24;
+		__fallthrough;
+	case 3:
+		a += (u32)k[2]<<16;
+		__fallthrough;
+	case 2:
+		a += (u32)k[1]<<8;
+		__fallthrough;
+	case 1:
+		a += k[0];
 		 __jhash_final(a, b, c);
+		break;
 	case 0: /* Nothing left to add */
 		break;
 	}
@@ -132,10 +156,16 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
 
 	/* Handle the last 3 u32's */
 	switch (length) {
-	case 3: c += k[2];	/* fall through */
-	case 2: b += k[1];	/* fall through */
-	case 1: a += k[0];
+	case 3:
+		c += k[2];
+		__fallthrough;
+	case 2:
+		b += k[1];
+		__fallthrough;
+	case 1:
+		a += k[0];
 		__jhash_final(a, b, c);
+		break;
 	case 0:	/* Nothing left to add */
 		break;
 	}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0334ca97c584..7acb131e287f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -158,11 +158,14 @@ static inline void __mm_zero_struct_page(struct page *page)
 
 	switch (sizeof(struct page)) {
 	case 80:
-		_pp[9] = 0;	/* fallthrough */
+		_pp[9] = 0;
+		__fallthrough;
 	case 72:
-		_pp[8] = 0;	/* fallthrough */
+		_pp[8] = 0;
+		__fallthrough;
 	case 64:
-		_pp[7] = 0;	/* fallthrough */
+		_pp[7] = 0;
+		__fallthrough;
 	case 56:
 		_pp[6] = 0;
 		_pp[5] = 0;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index b5d99482d3fe..fb750e87566f 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -129,11 +129,11 @@ static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
 		b3 = b->sig[3]; b2 = b->sig[2];				\
 		r->sig[3] = op(a3, b3);					\
 		r->sig[2] = op(a2, b2);					\
-		/* fall through */					\
+		__fallthrough;						\
 	case 2:								\
 		a1 = a->sig[1]; b1 = b->sig[1];				\
 		r->sig[1] = op(a1, b1);					\
-		/* fall through */					\
+		__fallthrough;						\
 	case 1:								\
 		a0 = a->sig[0]; b0 = b->sig[0];				\
 		r->sig[0] = op(a0, b0);					\
@@ -163,9 +163,9 @@ static inline void name(sigset_t *set)					\
 	switch (_NSIG_WORDS) {						\
 	case 4:	set->sig[3] = op(set->sig[3]);				\
 		set->sig[2] = op(set->sig[2]);				\
-		/* fall through */					\
+		__fallthrough;				\
 	case 2:	set->sig[1] = op(set->sig[1]);				\
-		/* fall through */					\
+		__fallthrough;				\
 	case 1:	set->sig[0] = op(set->sig[0]);				\
 		    break;						\
 	default:							\
@@ -186,7 +186,7 @@ static inline void sigemptyset(sigset_t *set)
 		memset(set, 0, sizeof(sigset_t));
 		break;
 	case 2: set->sig[1] = 0;
-		/* fall through */
+		__fallthrough;
 	case 1:	set->sig[0] = 0;
 		break;
 	}
@@ -199,7 +199,7 @@ static inline void sigfillset(sigset_t *set)
 		memset(set, -1, sizeof(sigset_t));
 		break;
 	case 2: set->sig[1] = -1;
-		/* fall through */
+		__fallthrough;
 	case 1:	set->sig[0] = -1;
 		break;
 	}
@@ -230,6 +230,7 @@ static inline void siginitset(sigset_t *set, unsigned long mask)
 		memset(&set->sig[1], 0, sizeof(long)*(_NSIG_WORDS-1));
 		break;
 	case 2: set->sig[1] = 0;
+		__fallthrough;
 	case 1: ;
 	}
 }
@@ -242,6 +243,7 @@ static inline void siginitsetinv(sigset_t *set, unsigned long mask)
 		memset(&set->sig[1], -1, sizeof(long)*(_NSIG_WORDS-1));
 		break;
 	case 2: set->sig[1] = -1;
+		__fallthrough;
 	case 1: ;
 	}
 }
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d8af86d995d6..1b7d3cf81dd8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3639,19 +3639,19 @@ static inline bool __skb_metadata_differs(const struct sk_buff *skb_a,
 #define __it(x, op) (x -= sizeof(u##op))
 #define __it_diff(a, b, op) (*(u##op *)__it(a, op)) ^ (*(u##op *)__it(b, op))
 	case 32: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case 24: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case 16: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case  8: diffs |= __it_diff(a, b, 64);
 		break;
 	case 28: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case 20: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case 12: diffs |= __it_diff(a, b, 64);
-		 /* fall through */
+		__fallthrough;
 	case  4: diffs |= __it_diff(a, b, 32);
 		break;
 	}
diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
index 3a49784d5c61..36c9aeafd801 100644
--- a/lib/zstd/bitstream.h
+++ b/lib/zstd/bitstream.h
@@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s
 		bitD->bitContainer = *(const BYTE *)(bitD->start);
 		switch (srcSize) {
 		case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
-			/* fall through */
+			__fallthrough;
 		case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
-			/* fall through */
+			__fallthrough;
 		case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
-			/* fall through */
+			__fallthrough;
 		case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24;
-			/* fall through */
+			__fallthrough;
 		case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16;
-			/* fall through */
+			__fallthrough;
 		case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8;
 		default:;
 		}
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index a74ce2e3c33e..e12359d69bb7 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -30,6 +30,9 @@ warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-option, -Wunused-const-variable)
 warning-1 += $(call cc-option, -Wpacked-not-aligned)
 warning-1 += $(call cc-option, -Wstringop-truncation)
+ifdef CONFIG_CC_IS_CLANG
+KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
+endif
 # The following turn off the warnings enabled by -Wextra
 warning-1 += -Wno-missing-field-initializers
 warning-1 += -Wno-sign-compare
-- 
2.23.0.rc1.153.gdeed80330f-goog



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-12 22:14 ` [PATCH v2] " Nathan Huckleberry
@ 2019-08-12 22:40   ` Joe Perches
  2019-08-12 23:11     ` Nick Desaulniers
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2019-08-12 22:40 UTC (permalink / raw)
  To: Nathan Huckleberry, yamada.masahiro, michal.lkml, Nathan Chancellor
  Cc: linux-kbuild, linux-kernel, linux-mm, clang-built-linux

On Mon, 2019-08-12 at 15:14 -0700, Nathan Huckleberry wrote:
> Clang does not support the use of comments to label
> intentional fallthrough. This patch replaces some uses
> of comments to attributesto cut down a significant number
> of warnings on clang (from ~50000 to ~200). Only comments
> in commonly used header files have been replaced.
> 
> Since there is still quite a bit of noise, this
> patch moves -Wimplicit-fallthrough to
> Makefile.extrawarn if you are compiling with
> clang.

Unmodified clang does not emit this warning without a patch.

> diff --git a/Makefile b/Makefile
[]
> @@ -846,7 +846,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>  KBUILD_CFLAGS += -Wdeclaration-after-statement
>  
>  # Warn about unmarked fall-throughs in switch statement.
> +# If the compiler is clang, this warning is only enabled if W=1 in
> +# Makefile.extrawarn
> +ifndef CONFIG_CC_IS_CLANG
>  KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
> +endif

It'd be better to remove CONFIG_CC_IS_CLANG everywhere
eventually as it adds complexity and makes .config files
not portable to multiple systems.

> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
[]
> @@ -253,4 +253,8 @@
>   */
>  #define __weak                          __attribute__((__weak__))
>  
> +#if __has_attribute(fallthrough)
> +#define __fallthrough                   __attribute__((fallthrough))

This should be __attribute__((__fallthrough__))

And there is still no agreement about whether this should
be #define fallthrough or #define __fallthrough

https://lore.kernel.org/patchwork/patch/1108577/

> diff --git a/include/linux/jhash.h b/include/linux/jhash.h
[]
> @@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
[]
> +	case 12:
> +		c += (u32)k[11]<<24;
> +		__fallthrough;

You might consider trying out the scripted conversion tool
attached to this email:

https://lore.kernel.org/lkml/61ddbb86d5e68a15e24ccb06d9b399bbf5ce2da7.camel@perches.com/




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] kbuild: Change fallthrough comments to attributes
  2019-08-12 21:47 [PATCH] kbuild: Change fallthrough comments to attributes Nathan Huckleberry
  2019-08-12 22:14 ` [PATCH v2] " Nathan Huckleberry
@ 2019-08-12 23:06 ` " Nick Desaulniers
  2019-08-13  7:15 ` Christoph Hellwig
  2019-08-15 21:07 ` kbuild test robot
  3 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-08-12 23:06 UTC (permalink / raw)
  To: Nathan Huckleberry
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, LKML,
	Linux Memory Management List, clang-built-linux

On Mon, Aug 12, 2019 at 2:48 PM 'Nathan Huckleberry' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> Clang does not support the use of comments to label
> intentional fallthrough. This patch replaces some uses
> of comments to attributesto cut down a significant number
> of warnings on clang (from ~50000 to ~200). Only comments
> in commonly used header files have been replaced.
>
> Since there is still quite a bit of noise, this
> patch moves -Wimplicit-fallthrough to
> Makefile.extrawarn if you are compiling with
> clang.
>
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> ---
>  Makefile                   |  4 +++
>  include/linux/jhash.h      | 60 ++++++++++++++++++++++++++++----------
>  include/linux/mm.h         |  9 ++++--
>  include/linux/signal.h     | 14 +++++----
>  include/linux/skbuff.h     | 12 ++++----
>  lib/zstd/bitstream.h       | 10 +++----
>  scripts/Makefile.extrawarn |  3 ++
>  7 files changed, 77 insertions(+), 35 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index c391fbb07195..875930c19619 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -847,7 +847,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>  KBUILD_CFLAGS += -Wdeclaration-after-statement
>
>  # Warn about unmarked fall-throughs in switch statement.
> +# If the compiler is clang, this warning is only enabled if W=1 in
> +# Makefile.extrawarn
> +ifndef CONFIG_CC_IS_CLANG
>  KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)

This should go in the block higher up (see line 739) that already
exists for CONFIG_CC_IS_CLANG.

> +endif
>
>  # Variable Length Arrays (VLAs) should not be used anywhere in the kernel
>  KBUILD_CFLAGS += -Wvla
> diff --git a/include/linux/jhash.h b/include/linux/jhash.h
> index ba2f6a9776b6..6cb2381501d1 100644
> --- a/include/linux/jhash.h
> +++ b/include/linux/jhash.h

Probably should split this patch into:
1. Makefile and scripts/Makefile.extrawarn hunks
2. hunks for each other file that can be applied individually (as
makes sense for the maintainers' trees).

> @@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
>         }
>         /* Last block: affect all 32 bits of (c) */
>         switch (length) {
> -       case 12: c += (u32)k[11]<<24;   /* fall through */
> -       case 11: c += (u32)k[10]<<16;   /* fall through */
> -       case 10: c += (u32)k[9]<<8;     /* fall through */
> -       case 9:  c += k[8];             /* fall through */
> -       case 8:  b += (u32)k[7]<<24;    /* fall through */
> -       case 7:  b += (u32)k[6]<<16;    /* fall through */
> -       case 6:  b += (u32)k[5]<<8;     /* fall through */
> -       case 5:  b += k[4];             /* fall through */
> -       case 4:  a += (u32)k[3]<<24;    /* fall through */
> -       case 3:  a += (u32)k[2]<<16;    /* fall through */
> -       case 2:  a += (u32)k[1]<<8;     /* fall through */
> -       case 1:  a += k[0];
> +       case 12:
> +               c += (u32)k[11]<<24;
> +               __attribute__((fallthrough));
> +       case 11:
> +               c += (u32)k[10]<<16;
> +               __attribute__((fallthrough));
> +       case 10:
> +               c += (u32)k[9]<<8;
> +               __attribute__((fallthrough));
> +       case 9:
> +               c += k[8];
> +               __attribute__((fallthrough));
> +       case 8:
> +               b += (u32)k[7]<<24;
> +               __attribute__((fallthrough));
> +       case 7:
> +               b += (u32)k[6]<<16;
> +               __attribute__((fallthrough));
> +       case 6:
> +               b += (u32)k[5]<<8;
> +               __attribute__((fallthrough));
> +       case 5:
> +               b += k[4];
> +               __attribute__((fallthrough));
> +       case 4:
> +               a += (u32)k[3]<<24;
> +               __attribute__((fallthrough));
> +       case 3:
> +               a += (u32)k[2]<<16;
> +               __attribute__((fallthrough));
> +       case 2:
> +               a += (u32)k[1]<<8;
> +               __attribute__((fallthrough));
> +       case 1:
> +               a += k[0];
>                  __jhash_final(a, b, c);
> +               break;
>         case 0: /* Nothing left to add */
>                 break;
>         }
> @@ -132,10 +156,16 @@ static inline u32 jhash2(const u32 *k, u32 length, u32 initval)
>
>         /* Handle the last 3 u32's */
>         switch (length) {
> -       case 3: c += k[2];      /* fall through */
> -       case 2: b += k[1];      /* fall through */
> -       case 1: a += k[0];
> +       case 3:
> +               c += k[2];
> +               __attribute__((fallthrough));
> +       case 2:
> +               b += k[1];
> +               __attribute__((fallthrough));
> +       case 1:
> +               a += k[0];
>                 __jhash_final(a, b, c);
> +               break;
>         case 0: /* Nothing left to add */
>                 break;
>         }
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0334ca97c584..52d99f263ca3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -158,11 +158,14 @@ static inline void __mm_zero_struct_page(struct page *page)
>
>         switch (sizeof(struct page)) {
>         case 80:
> -               _pp[9] = 0;     /* fallthrough */
> +               _pp[9] = 0;
> +               __attribute__((fallthrough));
>         case 72:
> -               _pp[8] = 0;     /* fallthrough */
> +               _pp[8] = 0;
> +               __attribute__((fallthrough));
>         case 64:
> -               _pp[7] = 0;     /* fallthrough */
> +               _pp[7] = 0;
> +               __attribute__((fallthrough));
>         case 56:
>                 _pp[6] = 0;
>                 _pp[5] = 0;
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index b5d99482d3fe..4fb0a0041a37 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -129,11 +129,11 @@ static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
>                 b3 = b->sig[3]; b2 = b->sig[2];                         \
>                 r->sig[3] = op(a3, b3);                                 \
>                 r->sig[2] = op(a2, b2);                                 \
> -               /* fall through */                                      \
> +               __attribute__((fallthrough));                           \
>         case 2:                                                         \
>                 a1 = a->sig[1]; b1 = b->sig[1];                         \
>                 r->sig[1] = op(a1, b1);                                 \
> -               /* fall through */                                      \
> +               __attribute__((fallthrough));                           \
>         case 1:                                                         \
>                 a0 = a->sig[0]; b0 = b->sig[0];                         \
>                 r->sig[0] = op(a0, b0);                                 \
> @@ -163,9 +163,9 @@ static inline void name(sigset_t *set)                                      \
>         switch (_NSIG_WORDS) {                                          \
>         case 4: set->sig[3] = op(set->sig[3]);                          \
>                 set->sig[2] = op(set->sig[2]);                          \
> -               /* fall through */                                      \
> +               __attribute__((fallthrough));                           \
>         case 2: set->sig[1] = op(set->sig[1]);                          \
> -               /* fall through */                                      \
> +               __attribute__((fallthrough));                           \
>         case 1: set->sig[0] = op(set->sig[0]);                          \
>                     break;                                              \
>         default:                                                        \
> @@ -186,7 +186,7 @@ static inline void sigemptyset(sigset_t *set)
>                 memset(set, 0, sizeof(sigset_t));
>                 break;
>         case 2: set->sig[1] = 0;
> -               /* fall through */
> +               __attribute__((fallthrough));
>         case 1: set->sig[0] = 0;
>                 break;
>         }
> @@ -199,7 +199,7 @@ static inline void sigfillset(sigset_t *set)
>                 memset(set, -1, sizeof(sigset_t));
>                 break;
>         case 2: set->sig[1] = -1;
> -               /* fall through */
> +               __attribute__((fallthrough));
>         case 1: set->sig[0] = -1;
>                 break;
>         }
> @@ -230,6 +230,7 @@ static inline void siginitset(sigset_t *set, unsigned long mask)
>                 memset(&set->sig[1], 0, sizeof(long)*(_NSIG_WORDS-1));
>                 break;
>         case 2: set->sig[1] = 0;
> +               __attribute__((fallthrough));
>         case 1: ;
>         }
>  }
> @@ -242,6 +243,7 @@ static inline void siginitsetinv(sigset_t *set, unsigned long mask)
>                 memset(&set->sig[1], -1, sizeof(long)*(_NSIG_WORDS-1));
>                 break;
>         case 2: set->sig[1] = -1;
> +               __attribute__((fallthrough));
>         case 1: ;
>         }
>  }
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index d8af86d995d6..4a1df6714dfe 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3639,19 +3639,19 @@ static inline bool __skb_metadata_differs(const struct sk_buff *skb_a,
>  #define __it(x, op) (x -= sizeof(u##op))
>  #define __it_diff(a, b, op) (*(u##op *)__it(a, op)) ^ (*(u##op *)__it(b, op))
>         case 32: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case 24: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case 16: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case  8: diffs |= __it_diff(a, b, 64);
>                 break;
>         case 28: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case 20: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case 12: diffs |= __it_diff(a, b, 64);
> -                /* fall through */
> +               __attribute__((fallthrough));
>         case  4: diffs |= __it_diff(a, b, 32);
>                 break;
>         }
> diff --git a/lib/zstd/bitstream.h b/lib/zstd/bitstream.h
> index 3a49784d5c61..cc311bae44da 100644
> --- a/lib/zstd/bitstream.h
> +++ b/lib/zstd/bitstream.h
> @@ -259,15 +259,15 @@ ZSTD_STATIC size_t BIT_initDStream(BIT_DStream_t *bitD, const void *srcBuffer, s
>                 bitD->bitContainer = *(const BYTE *)(bitD->start);
>                 switch (srcSize) {
>                 case 7: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[6]) << (sizeof(bitD->bitContainer) * 8 - 16);
> -                       /* fall through */
> +                       __attribute__((fallthrough));
>                 case 6: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[5]) << (sizeof(bitD->bitContainer) * 8 - 24);
> -                       /* fall through */
> +                       __attribute__((fallthrough));
>                 case 5: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[4]) << (sizeof(bitD->bitContainer) * 8 - 32);
> -                       /* fall through */
> +                       __attribute__((fallthrough));
>                 case 4: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[3]) << 24;
> -                       /* fall through */
> +                       __attribute__((fallthrough));
>                 case 3: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[2]) << 16;
> -                       /* fall through */
> +                       __attribute__((fallthrough));
>                 case 2: bitD->bitContainer += (size_t)(((const BYTE *)(srcBuffer))[1]) << 8;
>                 default:;
>                 }
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index a74ce2e3c33e..e12359d69bb7 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -30,6 +30,9 @@ warning-1 += $(call cc-option, -Wunused-but-set-variable)
>  warning-1 += $(call cc-option, -Wunused-const-variable)
>  warning-1 += $(call cc-option, -Wpacked-not-aligned)
>  warning-1 += $(call cc-option, -Wstringop-truncation)
> +ifdef CONFIG_CC_IS_CLANG
> +KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)

copy+pasta? Should this be:
warning-1 += ...

further, there's a block at the end of this file already for
CONFIG_CC_IS_CLANG that this should go in.  Unlike the rest of the
items there, you SHOULD keep the call to cc-option since older
versions of Clang won't support the warning.

Thanks for the patch!
-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-12 22:40   ` Joe Perches
@ 2019-08-12 23:11     ` Nick Desaulniers
  2019-08-12 23:23       ` Joe Perches
  2019-08-13  6:33       ` Nathan Chancellor
  0 siblings, 2 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-08-12 23:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Nathan Chancellor, Linux Kbuild mailing list, LKML,
	Linux Memory Management List, clang-built-linux,
	Gustavo A. R. Silva

On Mon, Aug 12, 2019 at 3:40 PM Joe Perches <joe@perches.com> wrote:
>
> On Mon, 2019-08-12 at 15:14 -0700, Nathan Huckleberry wrote:
> > Clang does not support the use of comments to label
> > intentional fallthrough. This patch replaces some uses
> > of comments to attributesto cut down a significant number
> > of warnings on clang (from ~50000 to ~200). Only comments
> > in commonly used header files have been replaced.
> >
> > Since there is still quite a bit of noise, this
> > patch moves -Wimplicit-fallthrough to
> > Makefile.extrawarn if you are compiling with
> > clang.
>
> Unmodified clang does not emit this warning without a patch.

Correct, Nathan is currently implementing support for attribute
fallthrough in Clang in:
https://reviews.llvm.org/D64838

I asked him in person to evaluate how many warnings we'd see in an
arm64 defconfig with his patch applied.  There were on the order of
50k warnings, mostly from these headers.  I asked him to send these
patches, then land support in the compiler, that way should our CI
catch fire overnight, we can carry out of tree fixes until they land.
With the changes here to Makefile.extrawarn, we should not need to
carry any out of tree patches.

>
> > diff --git a/Makefile b/Makefile
> []
> > @@ -846,7 +846,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
> >  KBUILD_CFLAGS += -Wdeclaration-after-statement
> >
> >  # Warn about unmarked fall-throughs in switch statement.
> > +# If the compiler is clang, this warning is only enabled if W=1 in
> > +# Makefile.extrawarn
> > +ifndef CONFIG_CC_IS_CLANG
> >  KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
> > +endif
>
> It'd be better to remove CONFIG_CC_IS_CLANG everywhere
> eventually as it adds complexity and makes .config files
> not portable to multiple systems.
>
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> []
> > @@ -253,4 +253,8 @@
> >   */
> >  #define __weak                          __attribute__((__weak__))
> >
> > +#if __has_attribute(fallthrough)
> > +#define __fallthrough                   __attribute__((fallthrough))
>
> This should be __attribute__((__fallthrough__))

Agreed.  I think the GCC documentation on attributes had a point about
why the __ prefix/suffix was important, which is why we went with that
in Miguel's original patchset.

>
> And there is still no agreement about whether this should
> be #define fallthrough or #define __fallthrough
>
> https://lore.kernel.org/patchwork/patch/1108577/
>
> > diff --git a/include/linux/jhash.h b/include/linux/jhash.h
> []
> > @@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
> []
> > +     case 12:
> > +             c += (u32)k[11]<<24;
> > +             __fallthrough;
>
> You might consider trying out the scripted conversion tool
> attached to this email:
>
> https://lore.kernel.org/lkml/61ddbb86d5e68a15e24ccb06d9b399bbf5ce2da7.camel@perches.com/

I guess the thing I'm curious about is why /* fall through */ is being
used vs __attribute__((__fallthrough__))?  Surely there's some
discussion someone can point me to?
-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-12 23:11     ` Nick Desaulniers
@ 2019-08-12 23:23       ` Joe Perches
  2019-08-13  6:33       ` Nathan Chancellor
  1 sibling, 0 replies; 12+ messages in thread
From: Joe Perches @ 2019-08-12 23:23 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Nathan Chancellor, Linux Kbuild mailing list, LKML,
	Linux Memory Management List, clang-built-linux,
	Gustavo A. R. Silva

On Mon, 2019-08-12 at 16:11 -0700, Nick Desaulniers wrote:
> On Mon, Aug 12, 2019 at 3:40 PM Joe Perches <joe@perches.com> wrote:
> > On Mon, 2019-08-12 at 15:14 -0700, Nathan Huckleberry wrote:
> > > Clang does not support the use of comments to label
> > > intentional fallthrough. This patch replaces some uses
> > > of comments to attributesto cut down a significant number
> > > of warnings on clang (from ~50000 to ~200). Only comments
> > > in commonly used header files have been replaced.
> > > 
> > > Since there is still quite a bit of noise, this
> > > patch moves -Wimplicit-fallthrough to
> > > Makefile.extrawarn if you are compiling with
> > > clang.
> > 
> > Unmodified clang does not emit this warning without a patch.
> 
> Correct, Nathan is currently implementing support for attribute
> fallthrough in Clang in:
> https://reviews.llvm.org/D64838
> 
> I asked him in person to evaluate how many warnings we'd see in an
> arm64 defconfig with his patch applied.  There were on the order of
> 50k warnings, mostly from these headers.  I asked him to send these
> patches, then land support in the compiler, that way should our CI
> catch fire overnight, we can carry out of tree fixes until they land.
> With the changes here to Makefile.extrawarn, we should not need to
> carry any out of tree patches.
> 
> > > diff --git a/Makefile b/Makefile
> > []
> > > @@ -846,7 +846,11 @@ NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
> > >  KBUILD_CFLAGS += -Wdeclaration-after-statement
> > > 
> > >  # Warn about unmarked fall-throughs in switch statement.
> > > +# If the compiler is clang, this warning is only enabled if W=1 in
> > > +# Makefile.extrawarn
> > > +ifndef CONFIG_CC_IS_CLANG
> > >  KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough,)
> > > +endif
> > 
> > It'd be better to remove CONFIG_CC_IS_CLANG everywhere
> > eventually as it adds complexity and makes .config files
> > not portable to multiple systems.
> > 
> > > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > []
> > > @@ -253,4 +253,8 @@
> > >   */
> > >  #define __weak                          __attribute__((__weak__))
> > > 
> > > +#if __has_attribute(fallthrough)
> > > +#define __fallthrough                   __attribute__((fallthrough))
> > 
> > This should be __attribute__((__fallthrough__))
> 
> Agreed.  I think the GCC documentation on attributes had a point about
> why the __ prefix/suffix was important, which is why we went with that
> in Miguel's original patchset.
> 
> > And there is still no agreement about whether this should
> > be #define fallthrough or #define __fallthrough
> > 
> > https://lore.kernel.org/patchwork/patch/1108577/
> > 
> > > diff --git a/include/linux/jhash.h b/include/linux/jhash.h
> > []
> > > @@ -86,19 +86,43 @@ static inline u32 jhash(const void *key, u32 length, u32 initval)
> > []
> > > +     case 12:
> > > +             c += (u32)k[11]<<24;
> > > +             __fallthrough;
> > 
> > You might consider trying out the scripted conversion tool
> > attached to this email:
> > 
> > https://lore.kernel.org/lkml/61ddbb86d5e68a15e24ccb06d9b399bbf5ce2da7.camel@perches.com/
> 
> I guess the thing I'm curious about is why /* fall through */ is being
> used vs __attribute__((__fallthrough__))?  Surely there's some
> discussion someone can point me to?

AFAIK:

It's historic.

https://lkml.org/lkml/2019/8/4/83

coverity and lint do not support __attribute__((__fallthrough__))
but do support /* fallthrough */ comments in their analysis output.

I prefer converting all the comments to a macro / pseudo keyword.

The cvt_style.pl script does a reasonable job of conversion.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-12 23:11     ` Nick Desaulniers
  2019-08-12 23:23       ` Joe Perches
@ 2019-08-13  6:33       ` Nathan Chancellor
  2019-08-13  7:04         ` Joe Perches
  1 sibling, 1 reply; 12+ messages in thread
From: Nathan Chancellor @ 2019-08-13  6:33 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Joe Perches, Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, LKML, Linux Memory Management List,
	clang-built-linux, Gustavo A. R. Silva

On Mon, Aug 12, 2019 at 04:11:26PM -0700, Nick Desaulniers wrote:
> Correct, Nathan is currently implementing support for attribute
> fallthrough in Clang in:
> https://reviews.llvm.org/D64838
> 
> I asked him in person to evaluate how many warnings we'd see in an
> arm64 defconfig with his patch applied.  There were on the order of
> 50k warnings, mostly from these headers.  I asked him to send these
> patches, then land support in the compiler, that way should our CI
> catch fire overnight, we can carry out of tree fixes until they land.
> With the changes here to Makefile.extrawarn, we should not need to
> carry any out of tree patches.

I think that if we are modifying this callsite to be favorable to clang,
we should consider a straight revert of commit bfd77145f35c ("Makefile:
Convert -Wimplicit-fallthrough=3 to just -Wimplicit-fallthrough for
clang"). It would save us a change in scripts/Makefile.extrawarn and
tying testing of this warning to W=1 will make the build noisy from
all of the other warnings that we don't care about plus we will need to
revert that change once we have finished the conversion process anyways.
I think it is cleaner to just pass KCFLAGS=-Wimplicit-fallthrough to
make when testing so that just that additional warning appears but
that is obviously subjective.

> > You might consider trying out the scripted conversion tool
> > attached to this email:
> >
> > https://lore.kernel.org/lkml/61ddbb86d5e68a15e24ccb06d9b399bbf5ce2da7.camel@perches.com/

I gave the script a go earlier today and it does a reasonable job at
convering the comments to the fallthrough keyword. Here is a list of
the warnings I still see in an x86 allyesconfig build with D64838 on
next-20190812:

https://gist.github.com/ffbd71b48ba197837e1bdd9bb863b85f

I have gone through about 20-30 of them and while there are a few missed
conversion spots (which is obviously fine for a treewide conversion),
the majority of them come from a disagreement between GCC and Clang on
emitting a warning when falling through to a case statement that is
either the last one and empty or simply breaks..

Example: https://godbolt.org/z/xgkvIh

I have more information on our issue tracker if anyone else wants to
take a look: https://github.com/ClangBuiltLinux/linux/issues/636

I personally think that GCC is right and Clang should adapt but I don't
know enough about the Clang codebase to know how feasible this is. I
just know there will be even more churn than necessary if we have to
annotate all of those places, taking the conversion process from maybe a
release cycle to several.

Cheers,
Nathan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-13  6:33       ` Nathan Chancellor
@ 2019-08-13  7:04         ` Joe Perches
  2019-08-13  7:43           ` Joe Perches
  2019-08-13  9:48           ` David Laight
  0 siblings, 2 replies; 12+ messages in thread
From: Joe Perches @ 2019-08-13  7:04 UTC (permalink / raw)
  To: Nathan Chancellor, Nick Desaulniers
  Cc: Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, LKML, Linux Memory Management List,
	clang-built-linux, Gustavo A. R. Silva

On Mon, 2019-08-12 at 23:33 -0700, Nathan Chancellor wrote:
> On Mon, Aug 12, 2019 at 04:11:26PM -0700, Nick Desaulniers wrote:
> > Correct, Nathan is currently implementing support for attribute
> > fallthrough in Clang in:
> > https://reviews.llvm.org/D64838
> > 
> > I asked him in person to evaluate how many warnings we'd see in an
> > arm64 defconfig with his patch applied.  There were on the order of
> > 50k warnings, mostly from these headers.  I asked him to send these
> > patches, then land support in the compiler, that way should our CI
> > catch fire overnight, we can carry out of tree fixes until they land.
> > With the changes here to Makefile.extrawarn, we should not need to
> > carry any out of tree patches.
> 
> I think that if we are modifying this callsite to be favorable to clang,
> we should consider a straight revert of commit bfd77145f35c ("Makefile:
> Convert -Wimplicit-fallthrough=3 to just -Wipmlicit-fallthrough for
> clang").

oh bother.

> It would save us a change in scripts/Makefile.extrawarn and
> tying testing of this warning to W=1 will make the build noisy from
> all of the other warnings that we don't care about plus we will need to
> revert that change once we have finished the conversion process anyways.
> I think it is cleaner to just pass KCFLAGS=-Wimplicit-fallthrough to
> make when testing so that just that additional warning appears but
> that is obviously subjective.
> 
> > > You might consider trying out the scripted conversion tool
> > > attached to this email:
> > > 
> > > https://lore.kernel.org/lkml/61ddbb86d5e68a15e24ccb06d9b399bbf5ce2da7.camel@perches.com/
> 
> I gave the script a go earlier today and it does a reasonable job at
> convering the comments to the fallthrough keyword. Here is a list of
> the warnings I still see in an x86 allyesconfig build with D64838 on
> next-20190812:
> 
> https://gist.github.com/ffbd71b48ba197837e1bdd9bb863b85f

> I have gone through about 20-30 of them and while there are a few missed
> conversion spots (which is obviously fine for a treewide conversion),

The _vast_ majority of case /* fallthrough */ style comments
in switch
blocks are immediately before another case or default

The afs ones seem to be because the last comment in the block
is not the fallthrough, but a description of the next case;

e.g.: from fs/afs/fsclient.c:

		/* extract the volume name */
	case 3:
		_debug("extract volname");
		ret = afs_extract_data(call, true);
		if (ret < 0)
			return ret;

		p = call->buffer;
		p[call->count] = 0;
		_debug("volname '%s'", p);
		afs_extract_to_tmp(call);
		call->unmarshall++;
		/* Fall through */

		/* extract the offline message length */
	case 4:

The script modifies a /* fallthrough */ style comment
only if the next non-blank line is 'case <foo>' or "default:'

There are many other /* fallthrough */ style comments
that are not actually fallthroughs or used in switch
blocks so this can't really be automated particularly
easily.

Likely these remainders would have to be converted manually.

> the majority of them come from a disagreement between GCC and Clang on
> emitting a warning when falling through to a case statement that is
> either the last one and empty or simply breaks..
> 
> Example: https://godbolt.org/z/xgkvIh
> 
> I have more information on our issue tracker if anyone else wants to
> take a look: https://github.com/ClangBuiltLinux/linux/issues/636
> 
> I personally think that GCC is right and Clang should adapt but I don't
> know enough about the Clang codebase to know how feasible this is.

I think gcc is wrong here and code like

	switch (foo) {
	case 1:
		bar = 1;
	default:
		break;
	}

should emit a fallthrough warning.

> I just know there will be even more churn than necessary if we have to
> annotate all of those places, taking the conversion process from maybe a
> release cycle to several.

Luckily, there's a list so it's not a hard problem
and it's easily scriptable.

There are < 350 entries, not many really.

btw: What does the 1st column mean?
      
              1 fs/xfs/scrub/agheader.c:89:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
           3507 include/linux/jhash.h:113:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]

Number of times emitted?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] kbuild: Change fallthrough comments to attributes
  2019-08-12 21:47 [PATCH] kbuild: Change fallthrough comments to attributes Nathan Huckleberry
  2019-08-12 22:14 ` [PATCH v2] " Nathan Huckleberry
  2019-08-12 23:06 ` [PATCH] " Nick Desaulniers
@ 2019-08-13  7:15 ` Christoph Hellwig
  2019-08-15 21:07 ` kbuild test robot
  3 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2019-08-13  7:15 UTC (permalink / raw)
  To: Nathan Huckleberry
  Cc: yamada.masahiro, michal.lkml, linux-kbuild, linux-kernel,
	linux-mm, clang-built-linux

On Mon, Aug 12, 2019 at 02:47:11PM -0700, Nathan Huckleberry wrote:
> Clang does not support the use of comments to label
> intentional fallthrough. This patch replaces some uses
> of comments to attributesto cut down a significant number
> of warnings on clang (from ~50000 to ~200). Only comments
> in commonly used header files have been replaced.
> 
> Since there is still quite a bit of noise, this
> patch moves -Wimplicit-fallthrough to
> Makefile.extrawarn if you are compiling with
> clang.

That __attribute__ crap looks like a cat barfed over the keyboard.

Please fix up clang and keep the kernel source sane.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-13  7:04         ` Joe Perches
@ 2019-08-13  7:43           ` Joe Perches
  2019-08-13  9:48           ` David Laight
  1 sibling, 0 replies; 12+ messages in thread
From: Joe Perches @ 2019-08-13  7:43 UTC (permalink / raw)
  To: Nathan Chancellor, Nick Desaulniers
  Cc: Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, LKML, Linux Memory Management List,
	clang-built-linux, Gustavo A. R. Silva

On Tue, 2019-08-13 at 00:04 -0700, Joe Perches wrote:
> On Mon, 2019-08-12 at 23:33 -0700, Nathan Chancellor wrote:
[]
> > a disagreement between GCC and Clang on
> > emitting a warning when falling through to a case statement that is
> > either the last one and empty or simply breaks..
[]
> > I personally think that GCC is right and Clang should adapt but I don't
> > know enough about the Clang codebase to know how feasible this is.
> 
> I think gcc is wrong here and code like
> 
> 	switch (foo) {
> 	case 1:
> 		bar = 1;
> 	default:
> 		break;
> 	}
> 
> should emit a fallthrough warning.

btw: I just filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432




^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH v2] kbuild: Change fallthrough comments to attributes
  2019-08-13  7:04         ` Joe Perches
  2019-08-13  7:43           ` Joe Perches
@ 2019-08-13  9:48           ` David Laight
  1 sibling, 0 replies; 12+ messages in thread
From: David Laight @ 2019-08-13  9:48 UTC (permalink / raw)
  To: Joe Perches, Nathan Chancellor, Nick Desaulniers
  Cc: Nathan Huckleberry, Masahiro Yamada, Michal Marek,
	Linux Kbuild mailing list, LKML, Linux Memory Management List,
	clang-built-linux, Gustavo A. R. Silva

From: Joe Perches
> Sent: 13 August 2019 08:05
...
> The afs ones seem to be because the last comment in the block
> is not the fallthrough, but a description of the next case;
> 
> e.g.: from fs/afs/fsclient.c:
> 
> 		/* extract the volume name */
> 	case 3:
> 		_debug("extract volname");

I'd change those to:
	case 3:  /* extract the volume name */

Then the /* fall through */ would be fine.

The /* FALLTHROUGH */ comment has been valid C syntax (for lint)
for over 40 years.
IMHO since C compilers are now doing all the checks that lint used
to do, it should be using the same syntax.
Both the [[]] and attribute forms look horrid.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] kbuild: Change fallthrough comments to attributes
  2019-08-12 21:47 [PATCH] kbuild: Change fallthrough comments to attributes Nathan Huckleberry
                   ` (2 preceding siblings ...)
  2019-08-13  7:15 ` Christoph Hellwig
@ 2019-08-15 21:07 ` kbuild test robot
  3 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2019-08-15 21:07 UTC (permalink / raw)
  To: Nathan Huckleberry
  Cc: kbuild-all, yamada.masahiro, michal.lkml, linux-kbuild,
	linux-kernel, linux-mm, clang-built-linux, Nathan Huckleberry

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

Hi Nathan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc4 next-20190814]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nathan-Huckleberry/kbuild-Change-fallthrough-comments-to-attributes/20190813-123202
config: x86_64-randconfig-a004-201932 (attached as .config)
compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/sched/signal.h:6:0,
                    from fs//notify/fanotify/fanotify.c:11:
   include/linux/signal.h: In function 'sigorsets':
   include/linux/signal.h:147:1: warning: empty declaration
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:147:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandsets':
   include/linux/signal.h:150:1: warning: empty declaration
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:150:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandnsets':
   include/linux/signal.h:153:1: warning: empty declaration
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:153:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'signotset':
   include/linux/signal.h:177:1: warning: empty declaration
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:166:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:177:1: warning: empty declaration
   include/linux/signal.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h: In function 'sigemptyset':
   include/linux/signal.h:189:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:189:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'sigfillset':
   include/linux/signal.h:202:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:202:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitset':
   include/linux/signal.h:233:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:233:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitsetinv':
   include/linux/signal.h:246:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:246:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   In file included from include/linux/pid_namespace.h:7:0,
                    from include/linux/ptrace.h:10,
                    from include/linux/audit.h:13,
                    from fs//notify/fanotify/fanotify.c:14:
   include/linux/mm.h: In function '__mm_zero_struct_page':
   include/linux/mm.h:162:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
>> include/linux/mm.h:162:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:165:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:165:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:168:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
--
   In file included from include/linux/security.h:32:0,
                    from fs//notify/fanotify/fanotify_user.c:12:
   include/linux/mm.h: In function '__mm_zero_struct_page':
   include/linux/mm.h:162:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
>> include/linux/mm.h:162:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:165:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:165:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:168:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   In file included from include/linux/syscalls.h:76:0,
                    from fs//notify/fanotify/fanotify_user.c:13:
   include/linux/signal.h: In function 'sigorsets':
   include/linux/signal.h:147:1: warning: empty declaration
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:147:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandsets':
   include/linux/signal.h:150:1: warning: empty declaration
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:150:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandnsets':
   include/linux/signal.h:153:1: warning: empty declaration
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:153:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'signotset':
   include/linux/signal.h:177:1: warning: empty declaration
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:166:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:177:1: warning: empty declaration
   include/linux/signal.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h: In function 'sigemptyset':
   include/linux/signal.h:189:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:189:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'sigfillset':
   include/linux/signal.h:202:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:202:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitset':
   include/linux/signal.h:233:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:233:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitsetinv':
   include/linux/signal.h:246:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:246:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
--
   In file included from fs//xfs/kmem.h:11:0,
                    from fs//xfs/xfs_linux.h:24,
                    from fs//xfs/xfs.h:22,
                    from fs//xfs/xfs_trace.c:6:
   include/linux/mm.h: In function '__mm_zero_struct_page':
   include/linux/mm.h:162:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
>> include/linux/mm.h:162:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:165:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:165:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/mm.h:168:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/mm.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   In file included from include/linux/sched/signal.h:6:0,
                    from fs//xfs/xfs_linux.h:39,
                    from fs//xfs/xfs.h:22,
                    from fs//xfs/xfs_trace.c:6:
   include/linux/signal.h: In function 'sigorsets':
   include/linux/signal.h:147:1: warning: empty declaration
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:147:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:147:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigorsets, _sig_or)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandsets':
   include/linux/signal.h:150:1: warning: empty declaration
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:150:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:150:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandsets, _sig_and)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'sigandnsets':
   include/linux/signal.h:153:1: warning: empty declaration
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
>> include/linux/signal.h:132:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h:153:1: warning: empty declaration
   include/linux/signal.h:136:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:153:1: note: in expansion of macro '_SIG_SET_BINOP'
    _SIG_SET_BINOP(sigandnsets, _sig_andn)
    ^~~~~~~~~~~~~~
   include/linux/signal.h: In function 'signotset':
   include/linux/signal.h:177:1: warning: empty declaration
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:166:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h:177:1: warning: empty declaration
   include/linux/signal.h:168:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      __attribute__((fallthrough));    \
      ^
   include/linux/signal.h:177:1: note: in expansion of macro '_SIG_SET_OP'
    _SIG_SET_OP(signotset, _sig_not)
    ^~~~~~~~~~~
   include/linux/signal.h: In function 'sigemptyset':
   include/linux/signal.h:189:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:189:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'sigfillset':
   include/linux/signal.h:202:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:202:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitset':
   include/linux/signal.h:233:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:233:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/signal.h: In function 'siginitsetinv':
   include/linux/signal.h:246:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/signal.h:246:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   In file included from include/linux/rhashtable.h:23:0,
                    from fs//xfs/xfs_linux.h:62,
                    from fs//xfs/xfs.h:22,
                    from fs//xfs/xfs_trace.c:6:
   include/linux/jhash.h: In function 'jhash':
>> include/linux/jhash.h:91:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
>> include/linux/jhash.h:91:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:94:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:94:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:97:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:97:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:100:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:100:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:103:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:103:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:106:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:106:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:109:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:109:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:112:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:112:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:115:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:115:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:118:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:118:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:121:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:121:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h: In function 'jhash2':
   include/linux/jhash.h:161:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:161:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   include/linux/jhash.h:164:3: warning: empty declaration
      __attribute__((fallthrough));
      ^~~~~~~~~~~~~
   include/linux/jhash.h:164:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
..

vim +132 include/linux/signal.h

   120	
   121	#define _SIG_SET_BINOP(name, op)					\
   122	static inline void name(sigset_t *r, const sigset_t *a, const sigset_t *b) \
   123	{									\
   124		unsigned long a0, a1, a2, a3, b0, b1, b2, b3;			\
   125										\
   126		switch (_NSIG_WORDS) {						\
   127		case 4:								\
   128			a3 = a->sig[3]; a2 = a->sig[2];				\
   129			b3 = b->sig[3]; b2 = b->sig[2];				\
   130			r->sig[3] = op(a3, b3);					\
   131			r->sig[2] = op(a2, b2);					\
 > 132			__attribute__((fallthrough));				\
   133		case 2:								\
   134			a1 = a->sig[1]; b1 = b->sig[1];				\
   135			r->sig[1] = op(a1, b1);					\
   136			__attribute__((fallthrough));				\
   137		case 1:								\
   138			a0 = a->sig[0]; b0 = b->sig[0];				\
   139			r->sig[0] = op(a0, b0);					\
   140			break;							\
   141		default:							\
   142			BUILD_BUG();						\
   143		}								\
   144	}
   145	
   146	#define _sig_or(x,y)	((x) | (y))
 > 147	_SIG_SET_BINOP(sigorsets, _sig_or)
   148	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35778 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12 21:47 [PATCH] kbuild: Change fallthrough comments to attributes Nathan Huckleberry
2019-08-12 22:14 ` [PATCH v2] " Nathan Huckleberry
2019-08-12 22:40   ` Joe Perches
2019-08-12 23:11     ` Nick Desaulniers
2019-08-12 23:23       ` Joe Perches
2019-08-13  6:33       ` Nathan Chancellor
2019-08-13  7:04         ` Joe Perches
2019-08-13  7:43           ` Joe Perches
2019-08-13  9:48           ` David Laight
2019-08-12 23:06 ` [PATCH] " Nick Desaulniers
2019-08-13  7:15 ` Christoph Hellwig
2019-08-15 21:07 ` kbuild test robot

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org linux-mm@archiver.kernel.org
	public-inbox-index linux-mm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox