Message ID | 20200325141137.7088-1-ktraynor@redhat.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 733EDA058B; Wed, 25 Mar 2020 15:13:05 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1BD002BAE; Wed, 25 Mar 2020 15:13:04 +0100 (CET) Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by dpdk.org (Postfix) with ESMTP id C9E5B1E34 for <dev@dpdk.org>; Wed, 25 Mar 2020 15:13:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585145581; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=iG+pO0zlaD5P99RwuMzeZ+hTtCGexu160lT8S9ZxeW8=; b=Jul4u3QJG0OffqSSjKpJuj0OlCpCbAcO/zEXuNQgerdoergOqg3mbhkuxZATdjVSll1ZOF vzqrCYJIPtAe+e17dx8dOTxFiL9uBNFFwXw0v8JbUvoy9JrZ8VgVeYc7CjzvLW9QGJ+w7m MxwaaUCwDHhViGj1yuCKQUxjE8y1qJg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-195-7l-2gQo2Mj2t2l1rQJM9eA-1; Wed, 25 Mar 2020 10:12:44 -0400 X-MC-Unique: 7l-2gQo2Mj2t2l1rQJM9eA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5C67E107ACCA; Wed, 25 Mar 2020 14:12:41 +0000 (UTC) Received: from rh.redhat.com (unknown [10.33.36.50]) by smtp.corp.redhat.com (Postfix) with ESMTP id 89AE35C1B0; Wed, 25 Mar 2020 14:12:12 +0000 (UTC) From: Kevin Traynor <ktraynor@redhat.com> To: dev@dpdk.org, thomas@monjalon.net, bruce.richardson@intel.com Cc: ravi1.kumar@amd.com, g.singh@nxp.com, hemant.agrawal@nxp.com, akhil.goyal@nxp.com, johndale@cisco.com, hyonkim@cisco.com, jingjing.wu@intel.com, wenzhuo.lu@intel.com, rmody@marvell.com, shshaikh@marvell.com, matan@mellanox.com, shahafs@mellanox.com, declan.doherty@intel.com, cristian.dumitrescu@intel.com, Kevin Traynor <ktraynor@redhat.com> Date: Wed, 25 Mar 2020 14:11:35 +0000 Message-Id: <20200325141137.7088-1-ktraynor@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Subject: [dpdk-dev] [RFC 0/2] gcc 10 disable stringop-overflow warning X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
gcc 10 disable stringop-overflow warning
|
|
Message
Kevin Traynor
March 25, 2020, 2:11 p.m. UTC
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(-)
Comments
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.
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 */ | ^~~
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.
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