Re: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning

[Kevin Traynor <ktraynor@redhat.com>]

On 25/03/2020 14:18, Thomas Monjalon wrote:
> 25/03/2020 15:11, Kevin Traynor:
>> This is a blunt way to remove this warning.
>>
>> Some alternatives are:
>> - disable the warning for individual components
>> - components rework to statically allocate memory for parts of structs impacted
>>
>> Maybe there's some other solutions?
> 
> In general, I am against disabling warnings, neither globally nor specifically.

Me too.

> Is there a way to hide false positives without disabling the warning?
> 

I haven't found one. If anyone has ideas it can be tested on Fedora 32
Beta which is available now, or I can try.

>> Impacted components:
>> ../drivers/crypto/caam_jr/caam_jr_pvt.h
>> ../drivers/crypto/ccp/ccp_crypto.h
>> ../drivers/crypto/dpaa_sec/dpaa_sec.h
>> ../drivers/crypto/virtio/virtio_cryptodev.h
>> ../drivers/net/enic/base/vnic_dev.c
>> ../drivers/net/iavf/../../common/iavf/virtchnl.h
>> ../drivers/net/ice/base/ice_adminq_cmd.h
>> ../drivers/net/mlx4/mlx4_rxtx.c
>> ../drivers/net/qede/base/ecore_mcp.c
>> ../lib/librte_cryptodev/rte_cryptodev.h
>> ../lib/librte_pipeline/rte_table_action.c
>>
>> Full details in the Bugzilla:
>> https://bugs.dpdk.org/show_bug.cgi?id=421
> 
> Would be easier to provide explanations here.
> I think we need at least one false positive example.
> 

If we look at the iavf code for rss_key->key:

struct virtchnl_rss_key {
        u16 vsi_id;
        u16 key_len;
        u8 key[1];         /* RSS hash key, packed bytes */
        ^^^^^^^^^
};

Then in iavf_configure_rss_key()

	len = sizeof(*rss_key) + vf->vf_res->rss_key_size - 1;
                                             ^^^^^^^^^^^^
	rss_key = rte_zmalloc("rss_key", len, 0);
                  ^^^ extra space beyond key[1] allocated here

<snip>
	rte_memcpy(rss_key->key, vf->rss_key, vf->vf_res->rss_key_size);
	                    ^^^                           ^^^^^^^^^^^^

At runtime we have allocated extra space at the end of the struct for
key, and the same size used in the malloc is also considered when
finding the right branches in the memcpy fns. But the compiler does not
know value of size and it simply sees there can be casts of a 1 byte key
to 16 or 32 bytes in some branches of the memcpy fns, so gives a warning.

e.g. _mm256_storeu_si256((__m256i *)dst, ymm0);
That is: Store 256-bits of integer data into memory,
where key is dst so leads to: warning: writing 32 bytes into a region of
size 1 [-Wstringop-overflow=]

Full log for warnings on key below.

>> Kevin Traynor (2):
>>   meson: gcc 10 disable stringop-overflow warnings
>>   mk: gcc 10 disable stringop-overflow warnings
> 
> You don't need to split in 2 patches.
> 

ok, will change if it moves to PATCH.

> 
> 

In file included from
/usr/lib/gcc/x86_64-redhat-linux/10/include/immintrin.h:51,
                 from
/usr/lib/gcc/x86_64-redhat-linux/10/include/x86intrin.h:32,
                 from
../lib/librte_eal/common/include/arch/x86/rte_vect.h:30,
                 from
../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:17,
                 from ../lib/librte_net/rte_ether.h:21,
                 from ../drivers/net/iavf/iavf_vchnl.c:18:
In function ‘_mm256_storeu_si256’,
    inlined from ‘rte_memcpy_aligned’ 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:867: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 */
      |     ^~~
In file included from
../lib/librte_eal/common/include/arch/x86/rte_atomic.h:15,
                 from ../drivers/net/iavf/iavf_vchnl.c:16:
In function ‘_mm_storeu_si128’,
    inlined from ‘rte_memcpy_aligned’ at
../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:302:2,
    inlined from ‘iavf_configure_rss_key’ at
../lib/librte_eal/common/include/arch/x86/rte_memcpy.h:867:10:
/usr/lib/gcc/x86_64-redhat-linux/10/include/emmintrin.h:727:8: warning:
writing 16 bytes into a region of size 1 [-Wstringop-overflow=]
  727 |   *__P = __B;
      |   ~~~~~^~~~~
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 */
      |     ^~~