build: gcc 10 disable stringop-overflow warnings

  stringop-overflow warns when it sees a possible overflow
in a string operation.

In the rte_memcpy functions different implementations are
used depending on the size. stringop-overflow is raised for
the paths in the function where it sees the static size of the
src could be overflowed.

However, in reality a correct size argument and in some cases
dynamic allocation would ensure that this does not happen.

Disable this warning at the top level as it is being raised on
several components.

For example, in the case below for key, the correct path will be
chosen in rte_memcpy_generic at runtime based on the size argument
but as some paths in the function could lead to a cast to 32 bytes
a warning is raised.

In function ‘_mm256_storeu_si256’,
inlined from ‘rte_memcpy_generic’
at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:315:2,
inlined from ‘iavf_configure_rss_key’
at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:869:10:

/usr/lib/gcc/x86_64-redhat-linux/10/include/avxintrin.h:928:8:
warning: writing 32 bytes into a region of size 1 [-Wstringop-overflow=]
  928 |   *__P = __A;
      |   ~~~~~^~~~~
In file included
from ../drivers/net/iavf/../../common/iavf/iavf_prototype.h:10,
from ../drivers/net/iavf/iavf.h:9,
from ../drivers/net/iavf/iavf_vchnl.c:22:

../drivers/net/iavf/iavf_vchnl.c:
In function ‘iavf_configure_rss_key’:

../drivers/net/iavf/../../common/iavf/virtchnl.h:508:5:
note: at offset 0 to object ‘key’ with size 1 declared here
  508 |  u8 key[1];         /* RSS hash key, packed bytes */
      |     ^~~

Bugzilla ID: 394
Bugzilla ID: 421

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 config/meson.build           | 3 ++-
 mk/toolchain/gcc/rte.vars.mk | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)
  

On 07/04/2020 17:27, Kevin Traynor wrote:
> stringop-overflow warns when it sees a possible overflow
> in a string operation.
> 
> In the rte_memcpy functions different implementations are
> used depending on the size. stringop-overflow is raised for
> the paths in the function where it sees the static size of the
> src could be overflowed.
> 
> However, in reality a correct size argument and in some cases
> dynamic allocation would ensure that this does not happen.
> 
> Disable this warning at the top level as it is being raised on
> several components.
> 
> For example, in the case below for key, the correct path will be
> chosen in rte_memcpy_generic at runtime based on the size argument
> but as some paths in the function could lead to a cast to 32 bytes
> a warning is raised.
> 
> In function ‘_mm256_storeu_si256’,
> inlined from ‘rte_memcpy_generic’
> at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:315:2,
> inlined from ‘iavf_configure_rss_key’
> at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:869:10:
> 
> /usr/lib/gcc/x86_64-redhat-linux/10/include/avxintrin.h:928:8:
> warning: writing 32 bytes into a region of size 1 [-Wstringop-overflow=]
>   928 |   *__P = __A;
>       |   ~~~~~^~~~~
> In file included
> from ../drivers/net/iavf/../../common/iavf/iavf_prototype.h:10,
> from ../drivers/net/iavf/iavf.h:9,
> from ../drivers/net/iavf/iavf_vchnl.c:22:
> 
> ../drivers/net/iavf/iavf_vchnl.c:
> In function ‘iavf_configure_rss_key’:
> 
> ../drivers/net/iavf/../../common/iavf/virtchnl.h:508:5:
> note: at offset 0 to object ‘key’ with size 1 declared here
>   508 |  u8 key[1];         /* RSS hash key, packed bytes */
>       |     ^~~
> 

As an alternative, we could remove these warnings for rte_memcpy.h(s)
only. WDYT?

diff --git a/lib/librte_eal/x86/include/rte_memcpy.h
b/lib/librte_eal/x86/include/rte_memcpy.h
index ba44c4a32..862e0c402 100644
--- a/lib/librte_eal/x86/include/rte_memcpy.h
+++ b/lib/librte_eal/x86/include/rte_memcpy.h
@@ -19,4 +19,6 @@
 #include <rte_config.h>

+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+
 #ifdef __cplusplus
 extern "C" {

> Bugzilla ID: 394
> Bugzilla ID: 421
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> --->  config/meson.build           | 3 ++-
>  mk/toolchain/gcc/rte.vars.mk | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 58421342b..476a667a9 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -200,5 +200,6 @@ warning_flags = [
>  	'-Wno-address-of-packed-member',
>  	'-Wno-packed-not-aligned',
> -	'-Wno-missing-field-initializers'
> +	'-Wno-missing-field-initializers',
> +	'-Wno-stringop-overflow'
>  ]
>  if not dpdk_conf.get('RTE_ARCH_64')
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index f19305e49..fe4b8c705 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -101,4 +101,7 @@ endif
>  WERROR_FLAGS += -Wno-address-of-packed-member
>  
> +# disable stringop-overflow warnings
> +WERROR_FLAGS += -Wno-stringop-overflow
> +
>  export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>  export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
>
  

On Fri, Apr 10, 2020 at 11:51:56AM +0100, Kevin Traynor wrote:
> On 07/04/2020 17:27, Kevin Traynor wrote:
> > stringop-overflow warns when it sees a possible overflow
> > in a string operation.
> > 
> > In the rte_memcpy functions different implementations are
> > used depending on the size. stringop-overflow is raised for
> > the paths in the function where it sees the static size of the
> > src could be overflowed.
> > 
> > However, in reality a correct size argument and in some cases
> > dynamic allocation would ensure that this does not happen.
> > 
> > Disable this warning at the top level as it is being raised on
> > several components.
> > 
> > For example, in the case below for key, the correct path will be
> > chosen in rte_memcpy_generic at runtime based on the size argument
> > but as some paths in the function could lead to a cast to 32 bytes
> > a warning is raised.
> > 
> > In function ‘_mm256_storeu_si256’,
> > inlined from ‘rte_memcpy_generic’
> > at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:315:2,
> > inlined from ‘iavf_configure_rss_key’
> > at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:869:10:
> > 
> > /usr/lib/gcc/x86_64-redhat-linux/10/include/avxintrin.h:928:8:
> > warning: writing 32 bytes into a region of size 1 [-Wstringop-overflow=]
> >   928 |   *__P = __A;
> >       |   ~~~~~^~~~~
> > In file included
> > from ../drivers/net/iavf/../../common/iavf/iavf_prototype.h:10,
> > from ../drivers/net/iavf/iavf.h:9,
> > from ../drivers/net/iavf/iavf_vchnl.c:22:
> > 
> > ../drivers/net/iavf/iavf_vchnl.c:
> > In function ‘iavf_configure_rss_key’:
> > 
> > ../drivers/net/iavf/../../common/iavf/virtchnl.h:508:5:
> > note: at offset 0 to object ‘key’ with size 1 declared here
> >   508 |  u8 key[1];         /* RSS hash key, packed bytes */
> >       |     ^~~
> > 
> 
> As an alternative, we could remove these warnings for rte_memcpy.h(s)
> only. WDYT?
> 
> diff --git a/lib/librte_eal/x86/include/rte_memcpy.h
> b/lib/librte_eal/x86/include/rte_memcpy.h
> index ba44c4a32..862e0c402 100644
> --- a/lib/librte_eal/x86/include/rte_memcpy.h
> +++ b/lib/librte_eal/x86/include/rte_memcpy.h
> @@ -19,4 +19,6 @@
>  #include <rte_config.h>
> 
> +#pragma GCC diagnostic ignored "-Wstringop-overflow"
> +
>  #ifdef __cplusplus
>  extern "C" {

Limited disabling is always better than global disabling, so given the
choice of the two, +1 for this.

/Bruce
  

On 10/04/2020 14:23, Bruce Richardson wrote:
> On Fri, Apr 10, 2020 at 11:51:56AM +0100, Kevin Traynor wrote:
>> On 07/04/2020 17:27, Kevin Traynor wrote:
>>> stringop-overflow warns when it sees a possible overflow
>>> in a string operation.
>>>
>>> In the rte_memcpy functions different implementations are
>>> used depending on the size. stringop-overflow is raised for
>>> the paths in the function where it sees the static size of the
>>> src could be overflowed.
>>>
>>> However, in reality a correct size argument and in some cases
>>> dynamic allocation would ensure that this does not happen.
>>>
>>> Disable this warning at the top level as it is being raised on
>>> several components.
>>>
>>> For example, in the case below for key, the correct path will be
>>> chosen in rte_memcpy_generic at runtime based on the size argument
>>> but as some paths in the function could lead to a cast to 32 bytes
>>> a warning is raised.
>>>
>>> In function ‘_mm256_storeu_si256’,
>>> inlined from ‘rte_memcpy_generic’
>>> at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:315:2,
>>> inlined from ‘iavf_configure_rss_key’
>>> at ../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:869:10:
>>>
>>> /usr/lib/gcc/x86_64-redhat-linux/10/include/avxintrin.h:928:8:
>>> warning: writing 32 bytes into a region of size 1 [-Wstringop-overflow=]
>>>   928 |   *__P = __A;
>>>       |   ~~~~~^~~~~
>>> In file included
>>> from ../drivers/net/iavf/../../common/iavf/iavf_prototype.h:10,
>>> from ../drivers/net/iavf/iavf.h:9,
>>> from ../drivers/net/iavf/iavf_vchnl.c:22:
>>>
>>> ../drivers/net/iavf/iavf_vchnl.c:
>>> In function ‘iavf_configure_rss_key’:
>>>
>>> ../drivers/net/iavf/../../common/iavf/virtchnl.h:508:5:
>>> note: at offset 0 to object ‘key’ with size 1 declared here
>>>   508 |  u8 key[1];         /* RSS hash key, packed bytes */
>>>       |     ^~~
>>>
>>
>> As an alternative, we could remove these warnings for rte_memcpy.h(s)
>> only. WDYT?
>>
>> diff --git a/lib/librte_eal/x86/include/rte_memcpy.h
>> b/lib/librte_eal/x86/include/rte_memcpy.h
>> index ba44c4a32..862e0c402 100644
>> --- a/lib/librte_eal/x86/include/rte_memcpy.h
>> +++ b/lib/librte_eal/x86/include/rte_memcpy.h
>> @@ -19,4 +19,6 @@
>>  #include <rte_config.h>
>>
>> +#pragma GCC diagnostic ignored "-Wstringop-overflow"
>> +
>>  #ifdef __cplusplus
>>  extern "C" {
> 
> Limited disabling is always better than global disabling, so given the
> choice of the two, +1 for this.
> 

Thanks Bruce. The slight disadvantage of this approach in short term is
a similar issue may also occur for gcc10 on ARM, but I don't have a
setup to test that, so an additional arm/eal fix may be required later.
I will limit fixing to what I can test, x86.

Gavin, can you check if a similar change is required for gcc10 on ARM?

> /Bruce
>