All of lore.kernel.org
 help / color / mirror / Atom feed
* GCC 12 warnings
@ 2022-05-20  2:36 Jakub Kicinski
  2022-05-20 16:46 ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-05-20  2:36 UTC (permalink / raw)
  To: Kees Cook; +Cc: netdev

Hi Kees,

I'm sure you're involved in a number of glorious GCC 12 conversations..

We have a handful of drivers in networking which get hit by
-Warray-bounds because they allocate partial structures (I presume 
to save memory, misguided but more than 15min of work to refactor).

Since -Warray-bounds is included by default now this is making our
lives a little hard [1]. Is there a wider effort to address this?
If not do you have a recommendation on how to deal with it?

My best idea is to try to isolate the bad files and punt -Warray-bounds
to W=1 for those, so we can prevent more of them getting in but not
break WERROR builds on GCC 12. That said, I'm not sure how to achieve
that.. This for example did not work:

--- a/drivers/net/ethernet/mediatek/Makefile
+++ b/drivers/net/ethernet/mediatek/Makefile
@@ -9,5 +9,9 @@ mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed.o
 ifdef CONFIG_DEBUG_FS
 mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_debugfs.o
 endif
 obj-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_ops.o
 obj-$(CONFIG_NET_MEDIATEK_STAR_EMAC) += mtk_star_emac.o
+
+ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
+CFLAGS_mtk_ppe.o += -Wno-array-bounds
+endif

[1]
https://lore.kernel.org/all/20220520012555.2262461-1-kuba@kernel.org/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: GCC 12 warnings
  2022-05-20  2:36 GCC 12 warnings Jakub Kicinski
@ 2022-05-20 16:46 ` Kees Cook
  2022-05-20 17:23   ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2022-05-20 16:46 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev

On Thu, May 19, 2022 at 07:36:18PM -0700, Jakub Kicinski wrote:
> I'm sure you're involved in a number of glorious GCC 12 conversations..

Yeah, and I think I've even found a gcc bug. :|

> We have a handful of drivers in networking which get hit by
> -Warray-bounds because they allocate partial structures (I presume 
> to save memory, misguided but more than 15min of work to refactor).

Yeah, this idiom is pretty common I've noticed -- I fixed a few of these
in the initial work for -Warray-bounds on GCC 11 and earlier, but wow
did GCC 12 do something extra internally.

> Since -Warray-bounds is included by default now this is making our
> lives a little hard [1]. Is there a wider effort to address this?
> If not do you have a recommendation on how to deal with it?

Looks like the issue was this?
https://lore.kernel.org/all/20220520145957.1ec50e44@canb.auug.org.au/

Ah, from cf2df74e202d ("net: fix dev_fill_forward_path with pppoe + bridge")

You mean you missed this particular warning because of the other GCC
12 warnings?

> My best idea is to try to isolate the bad files and punt -Warray-bounds
> to W=1 for those, so we can prevent more of them getting in but not
> break WERROR builds on GCC 12. That said, I'm not sure how to achieve
> that.. This for example did not work:
> 
> --- a/drivers/net/ethernet/mediatek/Makefile
> +++ b/drivers/net/ethernet/mediatek/Makefile
> @@ -9,5 +9,9 @@ mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed.o
>  ifdef CONFIG_DEBUG_FS
>  mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_debugfs.o
>  endif
>  obj-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_ops.o
>  obj-$(CONFIG_NET_MEDIATEK_STAR_EMAC) += mtk_star_emac.o
> +
> +ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
> +CFLAGS_mtk_ppe.o += -Wno-array-bounds
> +endif

This worked for me:

diff --git a/drivers/net/can/usb/kvaser_usb/Makefile b/drivers/net/can/usb/kvaser_usb/Makefile
index cf260044f0b9..43eb921f9102 100644
--- a/drivers/net/can/usb/kvaser_usb/Makefile
+++ b/drivers/net/can/usb/kvaser_usb/Makefile
@@ -1,3 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
 kvaser_usb-y = kvaser_usb_core.o kvaser_usb_leaf.o kvaser_usb_hydra.o
+
+ifeq ($(KBUILD_EXTRA_WARN),)
+CFLAGS_kvaser_usb_hydra.o += -Wno-array-bounds
+endif

-- 
Kees Cook

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: GCC 12 warnings
  2022-05-20 16:46 ` Kees Cook
@ 2022-05-20 17:23   ` Jakub Kicinski
  2022-05-20 21:43     ` David Laight
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-05-20 17:23 UTC (permalink / raw)
  To: Kees Cook; +Cc: netdev

On Fri, 20 May 2022 09:46:36 -0700 Kees Cook wrote:
> > Since -Warray-bounds is included by default now this is making our
> > lives a little hard [1]. Is there a wider effort to address this?
> > If not do you have a recommendation on how to deal with it?  
> 
> Looks like the issue was this?
> https://lore.kernel.org/all/20220520145957.1ec50e44@canb.auug.org.au/
> 
> Ah, from cf2df74e202d ("net: fix dev_fill_forward_path with pppoe + bridge")
> 
> You mean you missed this particular warning because of the other GCC
> 12 warnings?

Yup :(

> > My best idea is to try to isolate the bad files and punt -Warray-bounds
> > to W=1 for those, so we can prevent more of them getting in but not
> > break WERROR builds on GCC 12. That said, I'm not sure how to achieve
> > that.. This for example did not work:
> > 
> > --- a/drivers/net/ethernet/mediatek/Makefile
> > +++ b/drivers/net/ethernet/mediatek/Makefile
> > @@ -9,5 +9,9 @@ mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed.o
> >  ifdef CONFIG_DEBUG_FS
> >  mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_debugfs.o
> >  endif
> >  obj-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_ops.o
> >  obj-$(CONFIG_NET_MEDIATEK_STAR_EMAC) += mtk_star_emac.o
> > +
> > +ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
> > +CFLAGS_mtk_ppe.o += -Wno-array-bounds
> > +endif  
> 
> This worked for me:
> 
> diff --git a/drivers/net/can/usb/kvaser_usb/Makefile b/drivers/net/can/usb/kvaser_usb/Makefile
> index cf260044f0b9..43eb921f9102 100644
> --- a/drivers/net/can/usb/kvaser_usb/Makefile
> +++ b/drivers/net/can/usb/kvaser_usb/Makefile
> @@ -1,3 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
>  kvaser_usb-y = kvaser_usb_core.o kvaser_usb_leaf.o kvaser_usb_hydra.o
> +
> +ifeq ($(KBUILD_EXTRA_WARN),)
> +CFLAGS_kvaser_usb_hydra.o += -Wno-array-bounds
> +endif

Ah, thanks, I must have tried -Wno-array-bounds before I figured out
the condition and reverted back to full $(call cc-disable-warning, ..)
Let me redo the patches.

Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: GCC 12 warnings
  2022-05-20 17:23   ` Jakub Kicinski
@ 2022-05-20 21:43     ` David Laight
  2022-05-20 23:30       ` Kees Cook
  0 siblings, 1 reply; 5+ messages in thread
From: David Laight @ 2022-05-20 21:43 UTC (permalink / raw)
  To: 'Jakub Kicinski', Kees Cook; +Cc: netdev

From: Jakub Kicinski
> Sent: 20 May 2022 18:24
...
> > +ifeq ($(KBUILD_EXTRA_WARN),)
> > +CFLAGS_kvaser_usb_hydra.o += -Wno-array-bounds
> > +endif
> 
> Ah, thanks, I must have tried -Wno-array-bounds before I figured out
> the condition and reverted back to full $(call cc-disable-warning, ..)
> Let me redo the patches.

I think you need a check that the compiler actually supports
-Wno-array-bounds.
But that only needs be done once, not for every file
that needs the option.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: GCC 12 warnings
  2022-05-20 21:43     ` David Laight
@ 2022-05-20 23:30       ` Kees Cook
  0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2022-05-20 23:30 UTC (permalink / raw)
  To: David Laight; +Cc: 'Jakub Kicinski', netdev

On Fri, May 20, 2022 at 09:43:03PM +0000, David Laight wrote:
> From: Jakub Kicinski
> > Sent: 20 May 2022 18:24
> ...
> > > +ifeq ($(KBUILD_EXTRA_WARN),)
> > > +CFLAGS_kvaser_usb_hydra.o += -Wno-array-bounds
> > > +endif
> > 
> > Ah, thanks, I must have tried -Wno-array-bounds before I figured out
> > the condition and reverted back to full $(call cc-disable-warning, ..)
> > Let me redo the patches.
> 
> I think you need a check that the compiler actually supports
> -Wno-array-bounds.
> But that only needs be done once, not for every file
> that needs the option.

godbolt tells me that all kernel supported versions of GCC and Clang
handle -Wno-array-bounds. Let me know if there is a combination that
doesn't?

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-05-20 23:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20  2:36 GCC 12 warnings Jakub Kicinski
2022-05-20 16:46 ` Kees Cook
2022-05-20 17:23   ` Jakub Kicinski
2022-05-20 21:43     ` David Laight
2022-05-20 23:30       ` Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.