* [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 +++----
| 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:;
}
--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 related [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 ++---
| 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:;
}
--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 related [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, other threads:[~2019-08-15 21:08 UTC | newest]
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).