| Message ID | 20220602150834.643745-1-bruce.richardson@intel.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 mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B608DA0548; Thu, 2 Jun 2022 17:08:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5814840691; Thu, 2 Jun 2022 17:08:49 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id C14424021E for <dev@dpdk.org>; Thu, 2 Jun 2022 17:08:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1654182527; x=1685718527; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=kFNbmqv5sBd93a+7GFhWgB2sppMcHgDjOgtWYYuQ6KQ=; b=NA3APLdVJaRXDJCdTZ2k1ky6Y8y4EKGR5zkya5e4jeooWmRNMFmJ2IOq AdxFoMJn63I/Sj5dp/N7MZrMSq+H5q6jcX/PpTvBMIgiqT5gUa0c4dGuF riUdvGVUgWG/SoWa69Ple0Hmbl3CpElpcRcvwPb/mzhviQAviRJilBFDX uX/QlNvOY5QaGgjL6Ja7uejSJaFjDN4VicxZtx0HCIrw6yXsp9Et1MRJb 6MBQnd9MpW9PlPX4SM3rRqBTJ0Fv9XpTQ89q6zEZwzcImiC9OxelKLcjt UTsmdbPOlD0/cUeHFGidNt8XrhjwuIiZAcTwRncOClsBrbtYUFwbgJfKU w==; X-IronPort-AV: E=McAfee;i="6400,9594,10365"; a="273544142" X-IronPort-AV: E=Sophos;i="5.91,271,1647327600"; d="scan'208";a="273544142" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Jun 2022 08:08:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,271,1647327600"; d="scan'208";a="905025831" Received: from silpixa00401385.ir.intel.com (HELO silpixa00401385.ger.corp.intel.com.) ([10.237.222.171]) by fmsmga005.fm.intel.com with ESMTP; 02 Jun 2022 08:08:45 -0700 From: Bruce Richardson <bruce.richardson@intel.com> To: dev@dpdk.org Cc: david.marchand@redhat.com, stephen@networkplumber.org, Bruce Richardson <bruce.richardson@intel.com> Subject: [PATCH 0/4] clean up zero-length arrays Date: Thu, 2 Jun 2022 16:08:30 +0100 Message-Id: <20220602150834.643745-1-bruce.richardson@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 |
| Series | clean up zero-length arrays | |
Message
Bruce Richardson
June 2, 2022, 3:08 p.m. UTC
This patchset adds a coccinelle script to clean-up zero-length arrays in structures. The final patches are the result of running that script on the DPDK repository. Bruce Richardson (4): cocci: add script for zero-length arrays in structs drivers: replace zero-length arrays with undimensioned ones lib: replace zero-length arrays with undimensioned ones app: examples: replace zero-length arrays with undimensioned ones app/test/test_table_tables.c | 2 +- devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ drivers/bus/dpaa/include/netcfg.h | 4 +-- drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- drivers/common/cnxk/roc_se.h | 2 +- drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- drivers/common/dpaax/dpaax_iova_table.h | 2 +- drivers/common/mlx5/mlx5_prm.h | 10 +++---- drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- drivers/crypto/virtio/virtio_ring.h | 4 +-- drivers/crypto/virtio/virtqueue.h | 2 +- drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- drivers/net/cxgbe/clip_tbl.h | 2 +- drivers/net/cxgbe/l2t.h | 2 +- drivers/net/cxgbe/mps_tcam.h | 2 +- drivers/net/cxgbe/smt.h | 2 +- drivers/net/enic/base/vnic_devcmd.h | 2 +- drivers/net/hinic/hinic_pmd_tx.h | 2 +- drivers/net/mlx5/mlx5_tx.h | 2 +- drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- drivers/net/virtio/virtio_ring.h | 4 +-- drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- drivers/net/virtio/virtqueue.h | 2 +- drivers/regex/mlx5/mlx5_rxp.h | 4 +-- examples/ip_reassembly/main.c | 2 +- examples/ptpclient/ptpclient.c | 4 +-- lib/cryptodev/cryptodev_pmd.h | 2 +- lib/cryptodev/rte_cryptodev.h | 2 +- lib/eventdev/rte_event_timer_adapter.h | 2 +- lib/ip_frag/ip_reassembly.h | 2 +- lib/ipsec/sa.h | 2 +- lib/rib/rte_rib.c | 2 +- lib/rib/rte_rib6.c | 2 +- lib/table/rte_swx_table_learner.c | 4 +-- lib/table/rte_table_hash_key16.c | 4 +-- lib/table/rte_table_hash_key32.c | 4 +-- lib/table/rte_table_hash_key8.c | 4 +-- lib/vhost/rte_vhost.h | 4 +-- 40 files changed, 101 insertions(+), 54 deletions(-) create mode 100644 devtools/cocci/zero_length_array.cocci create mode 100644 lib/count_comments.py -- 2.34.1
Comments
> From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Thursday, 2 June 2022 17.09 > > This patchset adds a coccinelle script to clean-up zero-length > arrays in structures. The final patches are the result of running > that script on the DPDK repository. > > Bruce Richardson (4): > cocci: add script for zero-length arrays in structs > drivers: replace zero-length arrays with undimensioned ones > lib: replace zero-length arrays with undimensioned ones > app: examples: replace zero-length arrays with undimensioned ones > > app/test/test_table_tables.c | 2 +- > devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ > drivers/bus/dpaa/include/netcfg.h | 4 +-- > drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- > drivers/common/cnxk/roc_se.h | 2 +- > drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- > drivers/common/dpaax/dpaax_iova_table.h | 2 +- > drivers/common/mlx5/mlx5_prm.h | 10 +++---- > drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- > drivers/crypto/virtio/virtio_ring.h | 4 +-- > drivers/crypto/virtio/virtqueue.h | 2 +- > drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- > drivers/net/cxgbe/clip_tbl.h | 2 +- > drivers/net/cxgbe/l2t.h | 2 +- > drivers/net/cxgbe/mps_tcam.h | 2 +- > drivers/net/cxgbe/smt.h | 2 +- > drivers/net/enic/base/vnic_devcmd.h | 2 +- > drivers/net/hinic/hinic_pmd_tx.h | 2 +- > drivers/net/mlx5/mlx5_tx.h | 2 +- > drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- > drivers/net/virtio/virtio_ring.h | 4 +-- > drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- > drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- > drivers/net/virtio/virtqueue.h | 2 +- > drivers/regex/mlx5/mlx5_rxp.h | 4 +-- > examples/ip_reassembly/main.c | 2 +- > examples/ptpclient/ptpclient.c | 4 +-- > lib/cryptodev/cryptodev_pmd.h | 2 +- > lib/cryptodev/rte_cryptodev.h | 2 +- > lib/eventdev/rte_event_timer_adapter.h | 2 +- > lib/ip_frag/ip_reassembly.h | 2 +- > lib/ipsec/sa.h | 2 +- > lib/rib/rte_rib.c | 2 +- > lib/rib/rte_rib6.c | 2 +- > lib/table/rte_swx_table_learner.c | 4 +-- > lib/table/rte_table_hash_key16.c | 4 +-- > lib/table/rte_table_hash_key32.c | 4 +-- > lib/table/rte_table_hash_key8.c | 4 +-- > lib/vhost/rte_vhost.h | 4 +-- > 40 files changed, 101 insertions(+), 54 deletions(-) > create mode 100644 devtools/cocci/zero_length_array.cocci > create mode 100644 lib/count_comments.py > > -- > 2.34.1 > Series-Acked-by: Morten Brørup <mb@smartsharesystems.com>
On Thu, 2 Jun 2022 16:08:30 +0100 Bruce Richardson <bruce.richardson@intel.com> wrote: > This patchset adds a coccinelle script to clean-up zero-length > arrays in structures. The final patches are the result of running > that script on the DPDK repository. > > Bruce Richardson (4): > cocci: add script for zero-length arrays in structs > drivers: replace zero-length arrays with undimensioned ones > lib: replace zero-length arrays with undimensioned ones > app: examples: replace zero-length arrays with undimensioned ones > > app/test/test_table_tables.c | 2 +- > devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ > drivers/bus/dpaa/include/netcfg.h | 4 +-- > drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- > drivers/common/cnxk/roc_se.h | 2 +- > drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- > drivers/common/dpaax/dpaax_iova_table.h | 2 +- > drivers/common/mlx5/mlx5_prm.h | 10 +++---- > drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- > drivers/crypto/virtio/virtio_ring.h | 4 +-- > drivers/crypto/virtio/virtqueue.h | 2 +- > drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- > drivers/net/cxgbe/clip_tbl.h | 2 +- > drivers/net/cxgbe/l2t.h | 2 +- > drivers/net/cxgbe/mps_tcam.h | 2 +- > drivers/net/cxgbe/smt.h | 2 +- > drivers/net/enic/base/vnic_devcmd.h | 2 +- > drivers/net/hinic/hinic_pmd_tx.h | 2 +- > drivers/net/mlx5/mlx5_tx.h | 2 +- > drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- > drivers/net/virtio/virtio_ring.h | 4 +-- > drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- > drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- > drivers/net/virtio/virtqueue.h | 2 +- > drivers/regex/mlx5/mlx5_rxp.h | 4 +-- > examples/ip_reassembly/main.c | 2 +- > examples/ptpclient/ptpclient.c | 4 +-- > lib/cryptodev/cryptodev_pmd.h | 2 +- > lib/cryptodev/rte_cryptodev.h | 2 +- > lib/eventdev/rte_event_timer_adapter.h | 2 +- > lib/ip_frag/ip_reassembly.h | 2 +- > lib/ipsec/sa.h | 2 +- > lib/rib/rte_rib.c | 2 +- > lib/rib/rte_rib6.c | 2 +- > lib/table/rte_swx_table_learner.c | 4 +-- > lib/table/rte_table_hash_key16.c | 4 +-- > lib/table/rte_table_hash_key32.c | 4 +-- > lib/table/rte_table_hash_key8.c | 4 +-- > lib/vhost/rte_vhost.h | 4 +-- > 40 files changed, 101 insertions(+), 54 deletions(-) > create mode 100644 devtools/cocci/zero_length_array.cocci > create mode 100644 lib/count_comments.py > > -- > 2.34.1 > Series-acked-by: Stephen Hemminger <stephen@networkplumber.org> Would it be possible to add check for zero length arrays in checkpatch? So that new drivers don't bring the problem back.
On Thu, 2 Jun 2022 16:08:30 +0100 Bruce Richardson <bruce.richardson@intel.com> wrote: > This patchset adds a coccinelle script to clean-up zero-length > arrays in structures. The final patches are the result of running > that script on the DPDK repository. > > Bruce Richardson (4): > cocci: add script for zero-length arrays in structs > drivers: replace zero-length arrays with undimensioned ones > lib: replace zero-length arrays with undimensioned ones > app: examples: replace zero-length arrays with undimensioned ones > > app/test/test_table_tables.c | 2 +- > devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ > drivers/bus/dpaa/include/netcfg.h | 4 +-- > drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- > drivers/common/cnxk/roc_se.h | 2 +- > drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- > drivers/common/dpaax/dpaax_iova_table.h | 2 +- > drivers/common/mlx5/mlx5_prm.h | 10 +++---- > drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- > drivers/crypto/virtio/virtio_ring.h | 4 +-- > drivers/crypto/virtio/virtqueue.h | 2 +- > drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- > drivers/net/cxgbe/clip_tbl.h | 2 +- > drivers/net/cxgbe/l2t.h | 2 +- > drivers/net/cxgbe/mps_tcam.h | 2 +- > drivers/net/cxgbe/smt.h | 2 +- > drivers/net/enic/base/vnic_devcmd.h | 2 +- > drivers/net/hinic/hinic_pmd_tx.h | 2 +- > drivers/net/mlx5/mlx5_tx.h | 2 +- > drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- > drivers/net/virtio/virtio_ring.h | 4 +-- > drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- > drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- > drivers/net/virtio/virtqueue.h | 2 +- > drivers/regex/mlx5/mlx5_rxp.h | 4 +-- > examples/ip_reassembly/main.c | 2 +- > examples/ptpclient/ptpclient.c | 4 +-- > lib/cryptodev/cryptodev_pmd.h | 2 +- > lib/cryptodev/rte_cryptodev.h | 2 +- > lib/eventdev/rte_event_timer_adapter.h | 2 +- > lib/ip_frag/ip_reassembly.h | 2 +- > lib/ipsec/sa.h | 2 +- > lib/rib/rte_rib.c | 2 +- > lib/rib/rte_rib6.c | 2 +- > lib/table/rte_swx_table_learner.c | 4 +-- > lib/table/rte_table_hash_key16.c | 4 +-- > lib/table/rte_table_hash_key32.c | 4 +-- > lib/table/rte_table_hash_key8.c | 4 +-- > lib/vhost/rte_vhost.h | 4 +-- > 40 files changed, 101 insertions(+), 54 deletions(-) > create mode 100644 devtools/cocci/zero_length_array.cocci > create mode 100644 lib/count_comments.py > > -- > 2.34.1 > Bruce, looking at this commit, it looks like the underlying cause of the problem with iavf was it is using array size of one when flex array should be used: commit b5b3ea803e4741ad6a46a38d8227c78226d9054d Author: Kevin Traynor <ktraynor@redhat.com> Date: Fri Apr 17 16:43:35 2020 +0100 eal/x86: ignore gcc 10 stringop-overflow warnings stringop-overflow warns when it sees a possible overflow in a string operation. In the rte_memcpy functions different branches are taken depending on the size. stringop-overflow is raised for the branches 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. 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 */ | ^~~ Ignore the stringop-overflow warnings for rte_memcpy.h functions. Bugzilla ID: 394 Bugzilla ID: 421 Cc: stable@dpdk.org Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
On Wed, Jun 08, 2022 at 08:23:02AM -0700, Stephen Hemminger wrote: > On Thu, 2 Jun 2022 16:08:30 +0100 > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > This patchset adds a coccinelle script to clean-up zero-length > > arrays in structures. The final patches are the result of running > > that script on the DPDK repository. > > > > Bruce Richardson (4): > > cocci: add script for zero-length arrays in structs > > drivers: replace zero-length arrays with undimensioned ones > > lib: replace zero-length arrays with undimensioned ones > > app: examples: replace zero-length arrays with undimensioned ones > > > > app/test/test_table_tables.c | 2 +- > > devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ > > drivers/bus/dpaa/include/netcfg.h | 4 +-- > > drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- > > drivers/common/cnxk/roc_se.h | 2 +- > > drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- > > drivers/common/dpaax/dpaax_iova_table.h | 2 +- > > drivers/common/mlx5/mlx5_prm.h | 10 +++---- > > drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- > > drivers/crypto/virtio/virtio_ring.h | 4 +-- > > drivers/crypto/virtio/virtqueue.h | 2 +- > > drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- > > drivers/net/cxgbe/clip_tbl.h | 2 +- > > drivers/net/cxgbe/l2t.h | 2 +- > > drivers/net/cxgbe/mps_tcam.h | 2 +- > > drivers/net/cxgbe/smt.h | 2 +- > > drivers/net/enic/base/vnic_devcmd.h | 2 +- > > drivers/net/hinic/hinic_pmd_tx.h | 2 +- > > drivers/net/mlx5/mlx5_tx.h | 2 +- > > drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- > > drivers/net/virtio/virtio_ring.h | 4 +-- > > drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- > > drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- > > drivers/net/virtio/virtqueue.h | 2 +- > > drivers/regex/mlx5/mlx5_rxp.h | 4 +-- > > examples/ip_reassembly/main.c | 2 +- > > examples/ptpclient/ptpclient.c | 4 +-- > > lib/cryptodev/cryptodev_pmd.h | 2 +- > > lib/cryptodev/rte_cryptodev.h | 2 +- > > lib/eventdev/rte_event_timer_adapter.h | 2 +- > > lib/ip_frag/ip_reassembly.h | 2 +- > > lib/ipsec/sa.h | 2 +- > > lib/rib/rte_rib.c | 2 +- > > lib/rib/rte_rib6.c | 2 +- > > lib/table/rte_swx_table_learner.c | 4 +-- > > lib/table/rte_table_hash_key16.c | 4 +-- > > lib/table/rte_table_hash_key32.c | 4 +-- > > lib/table/rte_table_hash_key8.c | 4 +-- > > lib/vhost/rte_vhost.h | 4 +-- > > 40 files changed, 101 insertions(+), 54 deletions(-) > > create mode 100644 devtools/cocci/zero_length_array.cocci > > create mode 100644 lib/count_comments.py > > > > -- > > 2.34.1 > > > > Bruce, looking at this commit, it looks like the underlying cause > of the problem with iavf was it is using array size of one > when flex array should be used: > > commit b5b3ea803e4741ad6a46a38d8227c78226d9054d > Author: Kevin Traynor <ktraynor@redhat.com> > Date: Fri Apr 17 16:43:35 2020 +0100 > > eal/x86: ignore gcc 10 stringop-overflow warnings > > stringop-overflow warns when it sees a possible overflow > in a string operation. > > In the rte_memcpy functions different branches are taken > depending on the size. stringop-overflow is raised for the > branches 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. > > 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 */ > | ^~~ > I would tend to agree with your assessment. It looks like the "u8 key[1]" value should probably be "u8 key[]", and also in the following structure in the file, "u8 lut[1]" should probably be "u8 lut[]". Adding maintainers for driver on CC Beilei, Jingjing, in "common/iavf/virtchnl.h", there are quite a number of values at the end of structs which are defined as arrays of size 1. We suspect that many of these are placeholder arrays which should be given as unsigned arrays. Is this assessment correct? /Bruce
On Tue, Jun 14, 2022 at 10:40:50AM +0100, Bruce Richardson wrote: > On Wed, Jun 08, 2022 at 08:23:02AM -0700, Stephen Hemminger wrote: > > On Thu, 2 Jun 2022 16:08:30 +0100 > > Bruce Richardson <bruce.richardson@intel.com> wrote: > > > > > This patchset adds a coccinelle script to clean-up zero-length > > > arrays in structures. The final patches are the result of running > > > that script on the DPDK repository. > > > > > > Bruce Richardson (4): > > > cocci: add script for zero-length arrays in structs > > > drivers: replace zero-length arrays with undimensioned ones > > > lib: replace zero-length arrays with undimensioned ones > > > app: examples: replace zero-length arrays with undimensioned ones > > > > > > app/test/test_table_tables.c | 2 +- > > > devtools/cocci/zero_length_array.cocci | 21 +++++++++++++++ > > > drivers/bus/dpaa/include/netcfg.h | 4 +-- > > > drivers/bus/vmbus/rte_vmbus_reg.h | 4 +-- > > > drivers/common/cnxk/roc_se.h | 2 +- > > > drivers/common/dpaax/caamflib/desc/ipsec.h | 2 +- > > > drivers/common/dpaax/dpaax_iova_table.h | 2 +- > > > drivers/common/mlx5/mlx5_prm.h | 10 +++---- > > > drivers/crypto/ipsec_mb/ipsec_mb_private.h | 4 +-- > > > drivers/crypto/virtio/virtio_ring.h | 4 +-- > > > drivers/crypto/virtio/virtqueue.h | 2 +- > > > drivers/net/atlantic/hw_atl/hw_atl_utils.h | 2 +- > > > drivers/net/cxgbe/clip_tbl.h | 2 +- > > > drivers/net/cxgbe/l2t.h | 2 +- > > > drivers/net/cxgbe/mps_tcam.h | 2 +- > > > drivers/net/cxgbe/smt.h | 2 +- > > > drivers/net/enic/base/vnic_devcmd.h | 2 +- > > > drivers/net/hinic/hinic_pmd_tx.h | 2 +- > > > drivers/net/mlx5/mlx5_tx.h | 2 +- > > > drivers/net/nfp/nfpcore/nfp_nsp.h | 2 +- > > > drivers/net/virtio/virtio_ring.h | 4 +-- > > > drivers/net/virtio/virtio_user/vhost_kernel.c | 2 +- > > > drivers/net/virtio/virtio_user/vhost_vdpa.c | 2 +- > > > drivers/net/virtio/virtqueue.h | 2 +- > > > drivers/regex/mlx5/mlx5_rxp.h | 4 +-- > > > examples/ip_reassembly/main.c | 2 +- > > > examples/ptpclient/ptpclient.c | 4 +-- > > > lib/cryptodev/cryptodev_pmd.h | 2 +- > > > lib/cryptodev/rte_cryptodev.h | 2 +- > > > lib/eventdev/rte_event_timer_adapter.h | 2 +- > > > lib/ip_frag/ip_reassembly.h | 2 +- > > > lib/ipsec/sa.h | 2 +- > > > lib/rib/rte_rib.c | 2 +- > > > lib/rib/rte_rib6.c | 2 +- > > > lib/table/rte_swx_table_learner.c | 4 +-- > > > lib/table/rte_table_hash_key16.c | 4 +-- > > > lib/table/rte_table_hash_key32.c | 4 +-- > > > lib/table/rte_table_hash_key8.c | 4 +-- > > > lib/vhost/rte_vhost.h | 4 +-- > > > 40 files changed, 101 insertions(+), 54 deletions(-) > > > create mode 100644 devtools/cocci/zero_length_array.cocci > > > create mode 100644 lib/count_comments.py > > > > > > -- > > > 2.34.1 > > > > > > > Bruce, looking at this commit, it looks like the underlying cause > > of the problem with iavf was it is using array size of one > > when flex array should be used: > > > > commit b5b3ea803e4741ad6a46a38d8227c78226d9054d > > Author: Kevin Traynor <ktraynor@redhat.com> > > Date: Fri Apr 17 16:43:35 2020 +0100 > > > > eal/x86: ignore gcc 10 stringop-overflow warnings > > > > stringop-overflow warns when it sees a possible overflow > > in a string operation. > > > > In the rte_memcpy functions different branches are taken > > depending on the size. stringop-overflow is raised for the > > branches 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. > > > > 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 */ > > | ^~~ > > > > I would tend to agree with your assessment. It looks like the "u8 key[1]" > value should probably be "u8 key[]", and also in the following structure in > the file, "u8 lut[1]" should probably be "u8 lut[]". > > Adding maintainers for driver on CC > > Beilei, Jingjing, > > in "common/iavf/virtchnl.h", there are quite a number of values at the end > of structs which are defined as arrays of size 1. We suspect that many of > these are placeholder arrays which should be given as unsigned arrays. Is > this assessment correct? > s/unsigned arrays/flexible array members/ i.e. arrays without a given size "[]"