DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning
@ 2020-03-25 14:11 Kevin Traynor
  2020-03-25 14:11 ` [dpdk-dev] [RFC 1/2] meson: gcc 10 disable stringop-overflow warnings Kevin Traynor
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Kevin Traynor @ 2020-03-25 14:11 UTC (permalink / raw)
  To: dev, thomas, bruce.richardson
  Cc: ravi1.kumar, g.singh, hemant.agrawal, akhil.goyal, johndale,
	hyonkim, jingjing.wu, wenzhuo.lu, rmody, shshaikh, matan,
	shahafs, declan.doherty, cristian.dumitrescu, 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?

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

Kevin Traynor (2):
  meson: gcc 10 disable stringop-overflow warnings
  mk: gcc 10 disable stringop-overflow warnings

 config/meson.build           | 3 ++-
 mk/toolchain/gcc/rte.vars.mk | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

-- 
2.21.1


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

* [dpdk-dev] [RFC 1/2] meson: gcc 10 disable stringop-overflow warnings
  2020-03-25 14:11 [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Kevin Traynor
@ 2020-03-25 14:11 ` Kevin Traynor
  2020-03-25 14:11 ` [dpdk-dev] [RFC 2/2] mk: " Kevin Traynor
  2020-03-25 14:18 ` [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Kevin Traynor @ 2020-03-25 14:11 UTC (permalink / raw)
  To: dev, thomas, bruce.richardson
  Cc: ravi1.kumar, g.singh, hemant.agrawal, akhil.goyal, johndale,
	hyonkim, jingjing.wu, wenzhuo.lu, rmody, shshaikh, matan,
	shahafs, declan.doherty, cristian.dumitrescu, Kevin Traynor

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

Bugzilla ID: 421

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 config/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/config/meson.build b/config/meson.build
index abedd76f2..522368a29 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')
-- 
2.21.1


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

* [dpdk-dev] [RFC 2/2] mk: gcc 10 disable stringop-overflow warnings
  2020-03-25 14:11 [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Kevin Traynor
  2020-03-25 14:11 ` [dpdk-dev] [RFC 1/2] meson: gcc 10 disable stringop-overflow warnings Kevin Traynor
@ 2020-03-25 14:11 ` " Kevin Traynor
  2020-03-25 14:18 ` [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Thomas Monjalon
  2 siblings, 0 replies; 7+ messages in thread
From: Kevin Traynor @ 2020-03-25 14:11 UTC (permalink / raw)
  To: dev, thomas, bruce.richardson
  Cc: ravi1.kumar, g.singh, hemant.agrawal, akhil.goyal, johndale,
	hyonkim, jingjing.wu, wenzhuo.lu, rmody, shshaikh, matan,
	shahafs, declan.doherty, cristian.dumitrescu, Kevin Traynor

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

Bugzilla ID: 421

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 mk/toolchain/gcc/rte.vars.mk | 3 +++
 1 file changed, 3 insertions(+)

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
-- 
2.21.1


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

* Re: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning
  2020-03-25 14:11 [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Kevin Traynor
  2020-03-25 14:11 ` [dpdk-dev] [RFC 1/2] meson: gcc 10 disable stringop-overflow warnings Kevin Traynor
  2020-03-25 14:11 ` [dpdk-dev] [RFC 2/2] mk: " Kevin Traynor
@ 2020-03-25 14:18 ` Thomas Monjalon
  2020-03-31 14:08   ` Kevin Traynor
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2020-03-25 14:18 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: dev, bruce.richardson, ravi1.kumar, g.singh, hemant.agrawal,
	akhil.goyal, johndale, hyonkim, jingjing.wu, wenzhuo.lu, rmody,
	shshaikh, matan, shahafs, declan.doherty, cristian.dumitrescu,
	Kevin Traynor

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.
Is there a way to hide false positives without disabling the warning?

> 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.

> 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.




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

* Re: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning
  2020-03-25 14:18 ` [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Thomas Monjalon
@ 2020-03-31 14:08   ` Kevin Traynor
  2020-03-31 14:53     ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Traynor @ 2020-03-31 14:08 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, bruce.richardson, ravi1.kumar, g.singh, hemant.agrawal,
	akhil.goyal, johndale, hyonkim, jingjing.wu, wenzhuo.lu, rmody,
	shshaikh, matan, shahafs, declan.doherty, cristian.dumitrescu

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 */
      |     ^~~


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

* Re: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning
  2020-03-31 14:08   ` Kevin Traynor
@ 2020-03-31 14:53     ` Stephen Hemminger
  2020-04-01 11:33       ` Kevin Traynor
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2020-03-31 14:53 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: Thomas Monjalon, dev, bruce.richardson, ravi1.kumar, g.singh,
	hemant.agrawal, akhil.goyal, johndale, hyonkim, jingjing.wu,
	wenzhuo.lu, rmody, shshaikh, matan, shahafs, declan.doherty,
	cristian.dumitrescu

On Tue, 31 Mar 2020 15:08:26 +0100
Kevin Traynor <ktraynor@redhat.com> wrote:

> 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.

The standard way to do such a thing is to use an empty
array. The Linux kernel has just gone through cleaning up
all these zero length (and it this case one) array at
end of function.

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

* Re: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning
  2020-03-31 14:53     ` Stephen Hemminger
@ 2020-04-01 11:33       ` Kevin Traynor
  0 siblings, 0 replies; 7+ messages in thread
From: Kevin Traynor @ 2020-04-01 11:33 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Thomas Monjalon, dev, bruce.richardson, ravi1.kumar, g.singh,
	hemant.agrawal, akhil.goyal, johndale, hyonkim, jingjing.wu,
	wenzhuo.lu, rmody, shshaikh, matan, shahafs, declan.doherty,
	cristian.dumitrescu

On 31/03/2020 15:53, Stephen Hemminger wrote:
> On Tue, 31 Mar 2020 15:08:26 +0100
> Kevin Traynor <ktraynor@redhat.com> wrote:
> 
>> 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.
> 
> The standard way to do such a thing is to use an empty
> array. The Linux kernel has just gone through cleaning up
> all these zero length (and it this case one) array at
> end of function.
> 

Thanks for the tip. There is only one occurrence of a zero-length array
in these warning (sess_data[0] in ice) and changing to [] does indeed
remove the warning for that.

For the others they have a defined length so it cannot just be removed,
some don't have any dynamic allocation at all, sizes are checked etc.

I don't think adding dynamic allocation for those structs just to get
around this warning is the right way to go.

Crypto/iavf/ip_pipeline maintainers - any comment?

../drivers/crypto/caam_jr/caam_jr_pvt.h:187:10: note: at offset 0 to
object ‘digest’ with size 32 declared here
../drivers/crypto/ccp/ccp_crypto.h:272:11: note: at offset 0 to object
‘key’ with size 32 declared here
../drivers/crypto/dpaa_sec/dpaa_sec.h:217:10: note: at offset 0 to
object ‘digest’ with size 32 declared here
../drivers/crypto/virtio/virtio_cryptodev.h:36:10: note: at offset 0 to
object ‘iv’ with size 16 declared here

../drivers/net/iavf/../../common/iavf/virtchnl.h:508:5: note: at offset
0 to object ‘key’ with size 1 declared here
../drivers/net/iavf/../../common/iavf/virtchnl.h:516:5: note: at offset
0 to object ‘lut’ with size 1 declared here

../lib/librte_pipeline/rte_table_action.c:1648:12: note: at offset 0 to
object ‘cipher_iv’ with size 16 declared here
../lib/librte_pipeline/rte_table_action.c:1650:12: note: at offset 0 to
object ‘auth_iv’ with size 16 declared here
../lib/librte_pipeline/rte_table_action.c:1655:12: note: at offset 0 to
object ‘iv’ with size 16 declared here
../lib/librte_pipeline/rte_table_action.c:1656:12: note: at offset 0 to
object ‘aad’ with size 16 declared here
../lib/librte_pipeline/rte_table_action.c:1728:10: note: at offset 0 to
object ‘iv_aad_data’ with size 32 declared here


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 14:11 [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Kevin Traynor
2020-03-25 14:11 ` [dpdk-dev] [RFC 1/2] meson: gcc 10 disable stringop-overflow warnings Kevin Traynor
2020-03-25 14:11 ` [dpdk-dev] [RFC 2/2] mk: " Kevin Traynor
2020-03-25 14:18 ` [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning Thomas Monjalon
2020-03-31 14:08   ` Kevin Traynor
2020-03-31 14:53     ` Stephen Hemminger
2020-04-01 11:33       ` Kevin Traynor

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox