Messages in this thread | | | Date | Fri, 16 Jul 2021 11:47:02 -0700 | From | Nathan Chancellor <> | Subject | Re: [GIT PULL] fallthrough fixes for Clang for 5.14-rc2 |
| |
On Thu, Jul 15, 2021 at 06:04:15PM -0700, Linus Torvalds wrote: > On Wed, Jul 14, 2021 at 1:03 PM Gustavo A. R. Silva > <gustavoars@kernel.org> wrote: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git tags/Wimplicit-fallthrough-clang-5.14-rc2 > > Grr. > > I merged this, but when I actually tested it on my clang build, it > turns out that the clang "-Wimplicit-fallthrough" flag is unbelievable > garbage. > > I get > > warning: fallthrough annotation in unreachable code [-Wimplicit-fallthrough] > > and the stupid warning doesn't even say WHERE THE PROBLEM HAPPENS. > > No file name, no line numbers. Just this pointless garbage warning. > > Honestly, how does a compiler even do something that broken? Am I > supposed to use my sixth sense to guide me in finding the warning? > > I like the concept of the fallthrough warning, but it looks like the > clang implementation of it is so unbelievably broken that it's getting > disabled again. > > Yeah, I can > > (a) build the kernel without any parallelism > > (b) use ">&" to get both output and errors into the same file > > (c) see that it says > > CC kernel/sched/core.o > warning: fallthrough annotation in unreachable code [-Wimplicit-fallthrough] > 1 warning generated. > > and now I see at least which _file_ it is that causes that warning. > > I can then use my incredible powers of deduction (it's almost like a > sixth sense, but helped by the fact that there's only one single > "fallthrough" statement in that file) to figure out that it's > triggered by this code: > > case cpuset: > if (IS_ENABLED(CONFIG_CPUSETS)) { > cpuset_cpus_allowed_fallback(p); > state = possible; > break; > } > fallthrough; > case possible: > > and it all makes it clear that the clang warning is just incredibly > broken garbage not only in that lack of filename and line number, but > just in general.
I commented this on the LLVM bug tracker but I will copy and paste it here for posterity:
"It is actually the fact that
case 1: if (something || !IS_ENABLED(CONFIG_SOMETHING)) return blah; fallthrough; case 2:
looks like
case 1: return blah; fallthrough; case 2:
For example: https://godbolt.org/z/GdPeMbdo8
int foo(int a) { switch (a) { case 0: if (0) return 0; __attribute__((__fallthrough__)); // no warning case 1: if (1) return 1; __attribute__((__fallthrough__)); // warning case 2: return 3; default: return 4; } }
I am not really sure how to resolve that within checkFallThroughIntoBlock() or fillReachableBlocks() but given that this is something specific to the kernel, we could introduce -Wimplicit-fallthrough-unreachable then disable it within the kernel.
The file location not showing up was fixed by commit 1b4800c26259 ("[clang][parser] Set source ranges for GNU-style attributes"). The differential revision mentions this issue specifically."
Hopefully that would be an adequate solution, otherwise someone with more clang internal will have to take a look.
Cheers, Nathan
| |