| Message ID | 20190812221416.139678-1-nhuck@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: Change fallthrough comments to attributes | expand |
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/
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?
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.
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
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?
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
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)
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
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(-)