Re: [dpdk-dev] [PATCH] build: gcc 10 disable stringop-overflow warnings

[Kevin Traynor <ktraynor@redhat.com>]

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
>