[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 000/141] Fix fall-through warnings for Clang
- To: Jakub Kicinski <kuba@xxxxxxxxxx>
- From: Kees Cook <keescook@xxxxxxxxxxxx>
- Date: Sun, 22 Nov 2020 08:17:03 -0800
- Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, alsa-devel@xxxxxxxxxxxxxxxx, amd-gfx@xxxxxxxxxxxxxxxxxxxxx, bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx, ceph-devel@xxxxxxxxxxxxxxx, cluster-devel@xxxxxxxxxx, coreteam@xxxxxxxxxxxxx, devel@xxxxxxxxxxxxxxxxxxxx, dm-devel@xxxxxxxxxx, drbd-dev@xxxxxxxxxxxxxxxx, dri-devel@xxxxxxxxxxxxxxxxxxxxx, GR-everest-linux-l2@xxxxxxxxxxx, GR-Linux-NIC-Dev@xxxxxxxxxxx, intel-gfx@xxxxxxxxxxxxxxxxxxxxx, intel-wired-lan@xxxxxxxxxxxxxxxx, keyrings@xxxxxxxxxxxxxxx, linux1394-devel@xxxxxxxxxxxxxxxxxxxxx, linux-acpi@xxxxxxxxxxxxxxx, linux-afs@xxxxxxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux-arm-msm@xxxxxxxxxxxxxxx, linux-atm-general@xxxxxxxxxxxxxxxxxxxxx, linux-block@xxxxxxxxxxxxxxx, linux-can@xxxxxxxxxxxxxxx, linux-cifs@xxxxxxxxxxxxxxx, linux-crypto@xxxxxxxxxxxxxxx, linux-decnet-user@xxxxxxxxxxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx, linux-fbdev@xxxxxxxxxxxxxxx, linux-geode@xxxxxxxxxxxxxxxxxxx, linux-gpio@xxxxxxxxxxxxxxx, linux-hams@xxxxxxxxxxxxxxx, linux-hwmon@xxxxxxxxxxxxxxx, linux-i3c@xxxxxxxxxxxxxxxxxxx, linux-ide@xxxxxxxxxxxxxxx, linux-iio@xxxxxxxxxxxxxxx, linux-input@xxxxxxxxxxxxxxx, linux-integrity@xxxxxxxxxxxxxxx, linux-mediatek@xxxxxxxxxxxxxxxxxxx, linux-media@xxxxxxxxxxxxxxx, linux-mmc@xxxxxxxxxxxxxxx, linux-mm@xxxxxxxxx, linux-mtd@xxxxxxxxxxxxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, linux-rdma@xxxxxxxxxxxxxxx, linux-renesas-soc@xxxxxxxxxxxxxxx, linux-scsi@xxxxxxxxxxxxxxx, linux-sctp@xxxxxxxxxxxxxxx, linux-security-module@xxxxxxxxxxxxxxx, linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx, linux-usb@xxxxxxxxxxxxxxx, linux-watchdog@xxxxxxxxxxxxxxx, linux-wireless@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, nouveau@xxxxxxxxxxxxxxxxxxxxx, op-tee@xxxxxxxxxxxxxxxxxxxxxxxxx, oss-drivers@xxxxxxxxxxxxx, patches@xxxxxxxxxxxxxxxxxxxxx, rds-devel@xxxxxxxxxxxxxx, reiserfs-devel@xxxxxxxxxxxxxxx, samba-technical@xxxxxxxxxxxxxxx, selinux@xxxxxxxxxxxxxxx, target-devel@xxxxxxxxxxxxxxx, tipc-discussion@xxxxxxxxxxxxxxxxxxxxx, usb-storage@xxxxxxxxxxxxxxxxxxxxxxxx, virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx, wcn36xx@xxxxxxxxxxxxxxxxxxx, x86@xxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, linux-hardening@xxxxxxxxxxxxxxx, Nick Desaulniers <ndesaulniers@xxxxxxxxxx>, Nathan Chancellor <natechancellor@xxxxxxxxx>, Miguel Ojeda <ojeda@xxxxxxxxxx>, Joe Perches <joe@xxxxxxxxxxx>
- Delivery-date: Sun, 22 Nov 2020 16:17:11 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> On Fri, 20 Nov 2020 11:30:40 -0800 Kees Cook wrote:
> > On Fri, Nov 20, 2020 at 10:53:44AM -0800, 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?!
> >
> > It's certainly a place where the intent is not always clear. I think
> > this makes all the cases unambiguous, and doesn't impact the machine
> > code, since the compiler will happily optimize away any behavioral
> > redundancy.
>
> If none of the 140 patches here fix a real bug, and there is no change
> to machine code then it sounds to me like a W=2 kind of a warning.
FWIW, this series has found at least one bug so far:
https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=Kr_CQ@xxxxxxxxxxxxxx/
--
Kees Cook
|