[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4609d49b-4dd3-c017-b76e-a8a536871c05@embeddedor.com>
Date: Fri, 20 Nov 2020 13:04:55 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Jakub Kicinski <kuba@...nel.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
amd-gfx@...ts.freedesktop.org, bridge@...ts.linux-foundation.org,
ceph-devel@...r.kernel.org, cluster-devel@...hat.com,
coreteam@...filter.org, devel@...verdev.osuosl.org,
dm-devel@...hat.com, drbd-dev@...ts.linbit.com,
dri-devel@...ts.freedesktop.org, GR-everest-linux-l2@...vell.com,
GR-Linux-NIC-Dev@...vell.com, intel-gfx@...ts.freedesktop.org,
intel-wired-lan@...ts.osuosl.org, keyrings@...r.kernel.org,
linux1394-devel@...ts.sourceforge.net, linux-acpi@...r.kernel.org,
linux-afs@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org,
linux-atm-general@...ts.sourceforge.net,
linux-block@...r.kernel.org, linux-can@...r.kernel.org,
linux-cifs@...r.kernel.org, linux-crypto@...r.kernel.org,
linux-decnet-user@...ts.sourceforge.net,
linux-ext4@...r.kernel.org, linux-fbdev@...r.kernel.org,
linux-geode@...ts.infradead.org, linux-gpio@...r.kernel.org,
linux-hams@...r.kernel.org, linux-hwmon@...r.kernel.org,
linux-i3c@...ts.infradead.org, linux-ide@...r.kernel.org,
linux-iio@...r.kernel.org, linux-input@...r.kernel.org,
linux-integrity@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-media@...r.kernel.org,
linux-mmc@...r.kernel.org, linux-mm@...ck.org,
linux-mtd@...ts.infradead.org, linux-nfs@...r.kernel.org,
linux-rdma@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
linux-scsi@...r.kernel.org, linux-sctp@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-usb@...r.kernel.org, linux-watchdog@...r.kernel.org,
linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
netfilter-devel@...r.kernel.org, nouveau@...ts.freedesktop.org,
op-tee@...ts.trustedfirmware.org, oss-drivers@...ronome.com,
patches@...nsource.cirrus.com, rds-devel@....oracle.com,
reiserfs-devel@...r.kernel.org, samba-technical@...ts.samba.org,
selinux@...r.kernel.org, target-devel@...r.kernel.org,
tipc-discussion@...ts.sourceforge.net,
usb-storage@...ts.one-eyed-alien.net,
virtualization@...ts.linux-foundation.org,
wcn36xx@...ts.infradead.org, x86@...nel.org,
xen-devel@...ts.xenproject.org, linux-hardening@...r.kernel.org,
Nick Desaulniers <ndesaulniers@...gle.com>,
Nathan Chancellor <natechancellor@...il.com>,
Miguel Ojeda <ojeda@...nel.org>, Joe Perches <joe@...ches.com>,
Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH 000/141] Fix fall-through warnings for Clang
Hi,
On 11/20/20 12:53, Jakub Kicinski wrote:
> On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:
>> This series aims to fix almost all remaining fall-through warnings in
>> order to enable -Wimplicit-fallthrough for Clang.
>>
>> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
>> add multiple break/goto/return/fallthrough statements instead of just
>> letting the code fall through to the next case.
>>
>> Notice that in order to enable -Wimplicit-fallthrough for Clang, this
>> change[1] is meant to be reverted at some point. So, this patch helps
>> to move in that direction.
>>
>> Something important to mention is that there is currently a discrepancy
>> between GCC and Clang when dealing with switch fall-through to empty case
>> statements or to cases that only contain a break/continue/return
>> statement[2][3][4].
>
> Are we sure we want to make this change? Was it discussed before?
>
> Are there any bugs Clangs puritanical definition of fallthrough helped
> find?
>
> IMVHO compiler warnings are supposed to warn about issues that could
> be bugs. Falling through to default: break; can hardly be a bug?!
The justification for this is explained in this same changelog text:
Now that the -Wimplicit-fallthrough option has been globally enabled[5],
any compiler should really warn on missing either a fallthrough annotation
or any of the other case-terminating statements (break/continue/return/
goto) when falling through to the next case statement. Making exceptions
to this introduces variation in case handling which may continue to lead
to bugs, misunderstandings, and a general lack of robustness. The point
of enabling options like -Wimplicit-fallthrough is to prevent human error
and aid developers in spotting bugs before their code is even built/
submitted/committed, therefore eliminating classes of bugs. So, in order
to really accomplish this, we should, and can, move in the direction of
addressing any error-prone scenarios and get rid of the unintentional
fallthrough bug-class in the kernel, entirely, even if there is some minor
redundancy. Better to have explicit case-ending statements than continue to
have exceptions where one must guess as to the right result. The compiler
will eliminate any actual redundancy.
Note that there is already a patch in mainline that addresses almost
40,000 of these issues[6].
[1] commit e2079e93f562c ("kbuild: Do not enable -Wimplicit-fallthrough for clang for now")
[2] ClangBuiltLinux#636
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432
[4] https://godbolt.org/z/xgkvIh
[5] commit a035d552a93b ("Makefile: Globally enable fall-through warning")
[6] commit 4169e889e588 ("include: jhash/signal: Fix fall-through warnings for Clang")
Thanks
--
Gustavo
Powered by blists - more mailing lists