diff mbox series

ice/base: fix gcc 12 warning stringop-overflow

Message ID 20220616103304.132368-1-wenxuanx.wu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series ice/base: fix gcc 12 warning stringop-overflow |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Wu, WenxuanX June 16, 2022, 10:33 a.m. UTC 
  From: Wenxuan Wu <wenxuanx.wu@intel.com>

Gcc with -O2 flag, would retrieve the value in one time.
This patch changed the type of fv_idx in struct ice_recp_grp_entry to
align with its callers which is also u8 type.

When u8 idx[5] = a value u16 index[4], gcc12 would give this warning,
because it is not big enough to store the bytes(bigger than 5 bytes)
in one time (-O2 would do it in this way).

Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and defines")
Cc: stable@dpdk.org
Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>
---
 drivers/net/ice/base/ice_protocol_type.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Marchand June 16, 2022, 2:29 p.m. UTC | #1 
Title should start with net/ice/base:.

On Thu, Jun 16, 2022 at 12:35 PM <wenxuanx.wu@intel.com> wrote:
>
> From: Wenxuan Wu <wenxuanx.wu@intel.com>
>
> Gcc with -O2 flag, would retrieve the value in one time.
> This patch changed the type of fv_idx in struct ice_recp_grp_entry to
> align with its callers which is also u8 type.
>
> When u8 idx[5] = a value u16 index[4], gcc12 would give this warning,
> because it is not big enough to store the bytes(bigger than 5 bytes)
> in one time (-O2 would do it in this way).
>
> Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and defines")
> Cc: stable@dpdk.org
Missing empty line.

> Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>

Only build tested.
Tested-by: David Marchand <david.marchand@redhat.com>
Wu, WenxuanX June 17, 2022, 1:29 a.m. UTC | #2 
Thanks, do I need to send a new version to resolve formatting issues?

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: 2022年6月16日 22:29
> To: Wu, WenxuanX <wenxuanx.wu@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dev <dev@dpdk.org>; Thomas Monjalon
> <thomas@monjalon.net>; Stephen Hemminger
> <stephen@networkplumber.org>; dpdk stable <stable@dpdk.org>
> Subject: Re: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> 
> Title should start with net/ice/base:.
> 
> On Thu, Jun 16, 2022 at 12:35 PM <wenxuanx.wu@intel.com> wrote:
> >
> > From: Wenxuan Wu <wenxuanx.wu@intel.com>
> >
> > Gcc with -O2 flag, would retrieve the value in one time.
> > This patch changed the type of fv_idx in struct ice_recp_grp_entry to
> > align with its callers which is also u8 type.
> >
> > When u8 idx[5] = a value u16 index[4], gcc12 would give this warning,
> > because it is not big enough to store the bytes(bigger than 5 bytes)
> > in one time (-O2 would do it in this way).
> >
> > Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and
> > defines")
> > Cc: stable@dpdk.org
> Missing empty line.
> 
> > Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>
> 
> Only build tested.
> Tested-by: David Marchand <david.marchand@redhat.com>
> 
> 
> --
> David Marchand
David Marchand June 17, 2022, 5:04 a.m. UTC | #3 
On Fri, Jun 17, 2022 at 3:29 AM Wu, WenxuanX <wenxuanx.wu@intel.com> wrote:
>
> Thanks, do I need to send a new version to resolve formatting issues?

You are not a new contributor, please pay attention to such details.

To answer your question, I don't think there is a need for a new
version unless someone has a better idea for fixing.
Maintainer who applies the fix will handle it.
Qi Zhang June 19, 2022, 12:34 p.m. UTC | #4 
> -----Original Message-----
> From: Wu, WenxuanX <wenxuanx.wu@intel.com>
> Sent: Thursday, June 16, 2022 6:33 PM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> Cc: stephen@networkplumber.org; Wu, WenxuanX <wenxuanx.wu@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> 
> From: Wenxuan Wu <wenxuanx.wu@intel.com>
> 
> Gcc with -O2 flag, would retrieve the value in one time.
> This patch changed the type of fv_idx in struct ice_recp_grp_entry to align with
> its callers which is also u8 type.
> 
> When u8 idx[5] = a value u16 index[4], gcc12 would give this warning, because
> it is not big enough to store the bytes(bigger than 5 bytes) in one time (-O2
> would do it in this way).
> 
> Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and defines")
> Cc: stable@dpdk.org
> Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>
> ---
>  drivers/net/ice/base/ice_protocol_type.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ice/base/ice_protocol_type.h
> b/drivers/net/ice/base/ice_protocol_type.h
> index 0e6e5990be..cfe3b62630 100644
> --- a/drivers/net/ice/base/ice_protocol_type.h
> +++ b/drivers/net/ice/base/ice_protocol_type.h
> @@ -421,7 +421,7 @@ struct ice_recp_grp_entry {  #define
> ICE_INVAL_CHAIN_IND 0xFF
>  	u16 rid;
>  	u8 chain_idx;
> -	u16 fv_idx[ICE_NUM_WORDS_RECIPE];
> +	u8 fv_idx[ICE_NUM_WORDS_RECIPE];

If you change the data type from u16 to u8 for the FV index, you'd better also change the type of all the symbols to store the same thing.  e.g. the parameter "fv_idx" of ice_find_prot_off.
So, what's exactly the issue that GCC 12 reported? can we fix it from the other side without changing a type in the base code?


>  	u16 fv_mask[ICE_NUM_WORDS_RECIPE];
>  	struct ice_pref_recipe_group r_group;
>  };
> --
> 2.25.1
Wu, WenxuanX June 20, 2022, 1:38 a.m. UTC | #5 
Hi all, 

The warning  below gcc 12 :
    inlined from ‘ice_add_adv_recipe’ at ../drivers/net/ice/base/ice_switch.c:7951:11,
    inlined from ‘ice_add_adv_rule’ at ../drivers/net/ice/base/ice_switch.c:8911:11:
../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
In file included from ../drivers/net/ice/base/ice_controlq.h:8,
                 from ../drivers/net/ice/base/ice_type.h:54,
                 from ../drivers/net/ice/base/ice_common.h:8,
                 from ../drivers/net/ice/base/ice_switch.h:8,
                 from ../drivers/net/ice/base/ice_switch.c:5:
../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset 5 into destination object ‘lkup_indx’ of size 5
  744 |         u8 lkup_indx[5];
      |            ^~~~~~~~~
In function ‘ice_add_sw_recipe’,
    inlined from ‘ice_add_adv_recipe’ at ../drivers/net/ice/base/ice_switch.c:7951:11,
    inlined from ‘ice_add_adv_rule’ at ../drivers/net/ice/base/ice_switch.c:8911:11:
../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset [6, 254] into destination object ‘lkup_indx’ of size 5
  744 |         u8 lkup_indx[5];
      |            ^~~~~~~~~
In function ‘ice_add_sw_recipe’,
    inlined from ‘ice_add_adv_recipe’ at ../drivers/net/ice/base/ice_switch.c:7951:11,
    inlined from ‘ice_add_adv_rule’ at ../drivers/net/ice/base/ice_switch.c:8911:11:
../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset [7, 255] into destination object ‘lkup_indx’ of size 5
  744 |         u8 lkup_indx[5];
      |            ^~~~~~~~~
cc1: all warnings being treated as errors

Thanks 
Wenxuan

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: 2022年6月19日 20:35
> To: Wu, WenxuanX <wenxuanx.wu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> Cc: stephen@networkplumber.org; stable@dpdk.org
> Subject: RE: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> 
> 
> 
> > -----Original Message-----
> > From: Wu, WenxuanX <wenxuanx.wu@intel.com>
> > Sent: Thursday, June 16, 2022 6:33 PM
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> > Cc: stephen@networkplumber.org; Wu, WenxuanX
> <wenxuanx.wu@intel.com>;
> > stable@dpdk.org
> > Subject: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> >
> > From: Wenxuan Wu <wenxuanx.wu@intel.com>
> >
> > Gcc with -O2 flag, would retrieve the value in one time.
> > This patch changed the type of fv_idx in struct ice_recp_grp_entry to
> > align with its callers which is also u8 type.
> >
> > When u8 idx[5] = a value u16 index[4], gcc12 would give this warning,
> > because it is not big enough to store the bytes(bigger than 5 bytes)
> > in one time (-O2 would do it in this way).
> >
> > Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and
> > defines")
> > Cc: stable@dpdk.org
> > Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>
> > ---
> >  drivers/net/ice/base/ice_protocol_type.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ice/base/ice_protocol_type.h
> > b/drivers/net/ice/base/ice_protocol_type.h
> > index 0e6e5990be..cfe3b62630 100644
> > --- a/drivers/net/ice/base/ice_protocol_type.h
> > +++ b/drivers/net/ice/base/ice_protocol_type.h
> > @@ -421,7 +421,7 @@ struct ice_recp_grp_entry {  #define
> > ICE_INVAL_CHAIN_IND 0xFF
> >  	u16 rid;
> >  	u8 chain_idx;
> > -	u16 fv_idx[ICE_NUM_WORDS_RECIPE];
> > +	u8 fv_idx[ICE_NUM_WORDS_RECIPE];
> 
> If you change the data type from u16 to u8 for the FV index, you'd better also
> change the type of all the symbols to store the same thing.  e.g. the
> parameter "fv_idx" of ice_find_prot_off.
> So, what's exactly the issue that GCC 12 reported? can we fix it from the
> other side without changing a type in the base code?
> 
> 
> >  	u16 fv_mask[ICE_NUM_WORDS_RECIPE];
> >  	struct ice_pref_recipe_group r_group;  };
> > --
> > 2.25.1
Wu, WenxuanX June 20, 2022, 5:06 a.m. UTC | #6 
> -----Original Message-----
> From: Wu, WenxuanX <wenxuanx.wu@intel.com>
> Sent: 2022年6月20日 9:39
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> Cc: stephen@networkplumber.org; stable@dpdk.org
> Subject: RE: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> 
> Hi all,
> 
> The warning  below gcc 12 :
>     inlined from ‘ice_add_adv_recipe’
> at ../drivers/net/ice/base/ice_switch.c:7951:11,
>     inlined from ‘ice_add_adv_rule’
> at ../drivers/net/ice/base/ice_switch.c:8911:11:
> ../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a
> region of size 0 [-Werror=stringop-overflow=]
>  7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
>       |
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> In file included from ../drivers/net/ice/base/ice_controlq.h:8,
>                  from ../drivers/net/ice/base/ice_type.h:54,
>                  from ../drivers/net/ice/base/ice_common.h:8,
>                  from ../drivers/net/ice/base/ice_switch.h:8,
>                  from ../drivers/net/ice/base/ice_switch.c:5:
> ../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
> ../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset 5 into
> destination object ‘lkup_indx’ of size 5
>   744 |         u8 lkup_indx[5];
>       |            ^~~~~~~~~
> In function ‘ice_add_sw_recipe’,
>     inlined from ‘ice_add_adv_recipe’
> at ../drivers/net/ice/base/ice_switch.c:7951:11,
>     inlined from ‘ice_add_adv_rule’
> at ../drivers/net/ice/base/ice_switch.c:8911:11:
> ../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a
> region of size 0 [-Werror=stringop-overflow=]
>  7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
>       |
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> ../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
> ../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset [6, 254]
> into destination object ‘lkup_indx’ of size 5
>   744 |         u8 lkup_indx[5];
>       |            ^~~~~~~~~
> In function ‘ice_add_sw_recipe’,
>     inlined from ‘ice_add_adv_recipe’
> at ../drivers/net/ice/base/ice_switch.c:7951:11,
>     inlined from ‘ice_add_adv_rule’
> at ../drivers/net/ice/base/ice_switch.c:8911:11:
> ../drivers/net/ice/base/ice_switch.c:7220:61: error: writing 1 byte into a
> region of size 0 [-Werror=stringop-overflow=]
>  7220 |                         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
>       |
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> ../drivers/net/ice/base/ice_adminq_cmd.h: In function ‘ice_add_adv_rule’:
> ../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset [7, 255]
> into destination object ‘lkup_indx’ of size 5
>   744 |         u8 lkup_indx[5];
>       |            ^~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Thanks
> Wenxuan
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: 2022年6月19日 20:35
> > To: Wu, WenxuanX <wenxuanx.wu@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> > Cc: stephen@networkplumber.org; stable@dpdk.org
> > Subject: RE: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> >
> >
> >
> > > -----Original Message-----
> > > From: Wu, WenxuanX <wenxuanx.wu@intel.com>
> > > Sent: Thursday, June 16, 2022 6:33 PM
> > > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; dev@dpdk.org; thomas@monjalon.net
> > > Cc: stephen@networkplumber.org; Wu, WenxuanX
> > <wenxuanx.wu@intel.com>;
> > > stable@dpdk.org
> > > Subject: [PATCH] ice/base: fix gcc 12 warning stringop-overflow
> > >
> > > From: Wenxuan Wu <wenxuanx.wu@intel.com>
> > >
> > > Gcc with -O2 flag, would retrieve the value in one time.
> > > This patch changed the type of fv_idx in struct ice_recp_grp_entry
> > > to align with its callers which is also u8 type.
> > >
> > > When u8 idx[5] = a value u16 index[4], gcc12 would give this
> > > warning, because it is not big enough to store the bytes(bigger than
> > > 5 bytes) in one time (-O2 would do it in this way).
> > >
> > > Fixes: 04b8ec1ea807 ("net/ice/base: add protocol structures and
> > > defines")
> > > Cc: stable@dpdk.org
> > > Signed-off-by: Wenxuan Wu <wenxuanx.wu@intel.com>
> > > ---
> > >  drivers/net/ice/base/ice_protocol_type.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ice/base/ice_protocol_type.h
> > > b/drivers/net/ice/base/ice_protocol_type.h
> > > index 0e6e5990be..cfe3b62630 100644
> > > --- a/drivers/net/ice/base/ice_protocol_type.h
> > > +++ b/drivers/net/ice/base/ice_protocol_type.h
> > > @@ -421,7 +421,7 @@ struct ice_recp_grp_entry {  #define
> > > ICE_INVAL_CHAIN_IND 0xFF
> > >  	u16 rid;
> > >  	u8 chain_idx;
> > > -	u16 fv_idx[ICE_NUM_WORDS_RECIPE];
> > > +	u8 fv_idx[ICE_NUM_WORDS_RECIPE];
> >
> > If you change the data type from u16 to u8 for the FV index, you'd
> > better also change the type of all the symbols to store the same
> > thing.  e.g. the parameter "fv_idx" of ice_find_prot_off.
> > So, what's exactly the issue that GCC 12 reported? can we fix it from
> > the other side without changing a type in the base code?
        change from u8 lkup_indx[5] to u16 lkup_indx[5] can also resolve this issue. 
> >
> >
> > >  	u16 fv_mask[ICE_NUM_WORDS_RECIPE];
> > >  	struct ice_pref_recipe_group r_group;  };
> > > --
> > > 2.25.1
David Marchand June 23, 2022, 7:42 a.m. UTC | #7 
On Mon, Jun 20, 2022 at 7:06 AM Wu, WenxuanX <wenxuanx.wu@intel.com> wrote:
> > > > diff --git a/drivers/net/ice/base/ice_protocol_type.h
> > > > b/drivers/net/ice/base/ice_protocol_type.h
> > > > index 0e6e5990be..cfe3b62630 100644
> > > > --- a/drivers/net/ice/base/ice_protocol_type.h
> > > > +++ b/drivers/net/ice/base/ice_protocol_type.h
> > > > @@ -421,7 +421,7 @@ struct ice_recp_grp_entry {  #define
> > > > ICE_INVAL_CHAIN_IND 0xFF
> > > >   u16 rid;
> > > >   u8 chain_idx;
> > > > - u16 fv_idx[ICE_NUM_WORDS_RECIPE];
> > > > + u8 fv_idx[ICE_NUM_WORDS_RECIPE];
> > >
> > > If you change the data type from u16 to u8 for the FV index, you'd
> > > better also change the type of all the symbols to store the same
> > > thing.  e.g. the parameter "fv_idx" of ice_find_prot_off.
> > > So, what's exactly the issue that GCC 12 reported? can we fix it from
> > > the other side without changing a type in the base code?
>         change from u8 lkup_indx[5] to u16 lkup_indx[5] can also resolve this issue.

Any update?
Thanks.
diff mbox series

Patch

diff --git a/drivers/net/ice/base/ice_protocol_type.h b/drivers/net/ice/base/ice_protocol_type.h
index 0e6e5990be..cfe3b62630 100644
--- a/drivers/net/ice/base/ice_protocol_type.h
+++ b/drivers/net/ice/base/ice_protocol_type.h
@@ -421,7 +421,7 @@  struct ice_recp_grp_entry {
 #define ICE_INVAL_CHAIN_IND 0xFF
 	u16 rid;
 	u8 chain_idx;
-	u16 fv_idx[ICE_NUM_WORDS_RECIPE];
+	u8 fv_idx[ICE_NUM_WORDS_RECIPE];
 	u16 fv_mask[ICE_NUM_WORDS_RECIPE];
 	struct ice_pref_recipe_group r_group;
 };