From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Boccassi Subject: Re: [PATCH v3 00/40] Fix build on gcc8 and various bugs Date: Thu, 10 May 2018 14:49:54 +0100 Message-ID: <1525960194.23337.105.camel@debian.org> References: <152591991920.119328.14523975619615362920.stgit@localhost.localdomain> <1525947662.23337.101.camel@debian.org> <86b6bbc1-6ce8-cd51-7f55-6ec967cff647@warmcat.com> <1525955749.23337.103.camel@debian.org> <20180510133650.GB27828@bricha3-MOBL.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: Andy Green , dev@dpdk.org To: Bruce Richardson Return-path: Received: from mail-wr0-f181.google.com (mail-wr0-f181.google.com [209.85.128.181]) by dpdk.org (Postfix) with ESMTP id C8BA71BAF1 for ; Thu, 10 May 2018 15:49:56 +0200 (CEST) Received: by mail-wr0-f181.google.com with SMTP id g21-v6so2089091wrb.8 for ; Thu, 10 May 2018 06:49:56 -0700 (PDT) In-Reply-To: <20180510133650.GB27828@bricha3-MOBL.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, 2018-05-10 at 14:36 +0100, Bruce Richardson wrote: > On Thu, May 10, 2018 at 01:35:49PM +0100, Luca Boccassi wrote: > > On Thu, 2018-05-10 at 20:23 +0800, Andy Green wrote: > > >=20 > > > On 05/10/2018 06:21 PM, Luca Boccassi wrote: > > > > On Thu, 2018-05-10 at 10:46 +0800, Andy Green wrote: > > > > > The following series gets current master able to build > > > > > itself, and allow lagopus to build against it, on Fedora 28 + > > > > > x86_64 using gcc 8.0.1. > > > > >=20 > > > > > The first 17 patches have already been through two spins and > > > > > this time are corrected for all the comment (thanks to > > > > > everybody who commented) since v2, and have tested-by / > > > > > acked-bys applied.=C2=A0=C2=A0The first workaround patch for the = hash > > > > > function cast problem is dropped since something has already > > > > > been applied in master since yesterday to address it. > > > > >=20 > > > > > The additional 23 patches are fixes for problems found > > > > > actually trying to build lagopus using current master. > > > > > These are almost entirely related to signed / unsigned > > > > > or truncation without explicit casts inside dpdk > > > > > headers. > > > > >=20 > > > > > --- > > > > >=20 > > > > > Andy Green (40): > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/bus/pci: fix st= rncpy dangerous code > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/bus/dpaa: fix i= nconsistent struct alignment > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/axgbe: fix = broken eeprom string comp > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/nfp/nfpcore= : fix strncpy misuse > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/nfp/nfpcore= : fix off-by-one and no NUL on > > > > > strncpy > > > > > use > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/nfp: don't = memcpy out of source range > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/nfp: fix bu= ffer overflow in fw_name > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/qede: fix s= trncpy constant and NUL > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/qede: fix b= roken strncpy > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/sfc: fix st= rncpy length > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/sfc: fix st= rncpy size and NUL > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/vdev: readl= ink inputs cannot be aliased > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0drivers/net/vdev: fix 3= x strncpy misuse > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0app/test-pmd: can't fin= d include > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0app/proc-info: fix spri= ntf overrun bug > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0app/test-bbdev: test-bb= dev: strcpy ok for allocated > > > > > string > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0app/test-bbdev: strcpy = ok for allocated string > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_common.h: cast gcc = builtin result to avoid > > > > > complaints > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_memcpy.h: explicit = tmp cast > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0lib/librte_eal/common/i= nclude/rte_lcore.h: explicit > > > > > cast > > > > > for > > > > > signed change > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/lib/librte_eal/common/= include/rte_random.h: stage > > > > > cast > > > > > from > > > > > uint64_t to long > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_spinlock.h: stack d= eclarations before code > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ring_generic.h: sta= ck declarations before code > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ring.h: remove sign= ed type flipflopping > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_dev.h: stack declar= ation at top of own basic block > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: avoid trunc= ation warnings from inadvertant > > > > > int16_t > > > > > to int promotion > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: explicit ca= sts for flipping between > > > > > int16_t > > > > > and > > > > > uint16_t > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: make sure R= TE_MIN compares same types > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: explicit ca= st restricting ptrdiff to > > > > > uint16_t > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: explicit ca= st for size_t to uint32_t > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_mbuf.h: explicit ca= sts to uint16_t to avoid > > > > > truncation > > > > > warnings > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_byteorder.h: explic= it cast for return promotion > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ether.h: explicit c= ast avoiding truncation warning > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ether.h: stack vars= declared at top of function > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ethdev.h: fix sign = and scope of temp var > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ethdev.h: explicit = cast for return type > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_ethdev.h: explicit = cast for truncation > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_hash_crc.h: stack v= ars declared at top of function > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_hash_crc.h: explici= t casts for truncation > > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_string_fns.h: expli= cit cast for int return to > > > > > size_t > > > >=20 > > > > Hi, > > > >=20 > > > > I've built-tested this series on Debian Stretch (gcc 6.3) and > > > > Debian > > > > Sid (gcc 8.1). > > > >=20 > > > > The series builds fine with the default config, but the bnx2x > > > > and > > > > mlx5 > > > > PMDs still have errors with gcc-8: > > >=20 > > > Yes I just built it with defconfig for x86_64 on Fedora 28 with > > > default=C2=A0 > > > tools and cleared out everything that came up. > > >=20 > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c: In function > > > > 'bnx2x_alloc_hsi_mem': > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:176:29: error: '%s' > > > > directive > > > > writing up to 31 bytes into a region of size between 15 and 25 > > > > [- > > > > Werror=3Dformat-overflow=3D] > > > > =C2=A0=C2=A0=C2=A0=C2=A0sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, sc-= >pcie_device, > > > > msg, > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0^~ > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:8874:7: > > > > =C2=A0=C2=A0=C2=A0=C2=A0if (bnx2x_dma_alloc(sc, sizeof(union > > > > bnx2x_host_hc_status_block), > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > ~~~~ > > > > ~~ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0&fp->sb_dma, buf, R= TE_CACHE_LINE_SIZE) !=3D 0) { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~~~~ > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:176:3: note: 'sprintf' > > > > output > > > > between 10 and 66 bytes into a destination of size 32 > > > > =C2=A0=C2=A0=C2=A0=C2=A0sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, sc-= >pcie_device, > > > > msg, > > > > =C2=A0=C2=A0=C2=A0=C2=A0^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~ > > > > ~~ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_get_timer_cycles()); > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0~~~~~~~~~~~~~~~~~~~~~~~ > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:173:29: error: '%s' > > > > directive > > > > writing up to 31 bytes into a region of size between 23 and 25 > > > > [- > > > > Werror=3Dformat-overflow=3D] > > > > =C2=A0=C2=A0=C2=A0=C2=A0sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, SC_= ABS_FUNC(sc), > > > > msg, > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0^~ > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:8874:7: > > > > =C2=A0=C2=A0=C2=A0=C2=A0if (bnx2x_dma_alloc(sc, sizeof(union > > > > bnx2x_host_hc_status_block), > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > ~~~~ > > > > ~~ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0&fp->sb_dma, buf, R= TE_CACHE_LINE_SIZE) !=3D 0) { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~~~~ > > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:173:3: note: 'sprintf' > > > > output > > > > between 10 and 58 bytes into a destination of size 32 > > > > =C2=A0=C2=A0=C2=A0=C2=A0sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, SC_= ABS_FUNC(sc), > > > > msg, > > > > =C2=A0=C2=A0=C2=A0=C2=A0^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~= ~~~~~~~~~~~~~~~~ > > > > ~~ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rte_get_timer_cycles()); > > > >=20 > > > >=20 > > > > /tmp/dpdk/drivers/net/mlx5/mlx5.c: In function > > > > 'mlx5_pci_probe': > > > > /tmp/dpdk/drivers/net/mlx5/mlx5.c:920:13: error: 'vf' may be > > > > used > > > > uninitialized in this function [-Werror=3Dmaybe-uninitialized] > > > > =C2=A0=C2=A0=C2=A0=C2=A0config.vf =3D vf; > > > >=20 > > > > Hope this can be useful. > > >=20 > > > I think gcc 8.0.1 is capable to show that and I am willing to > > > look > > > at=C2=A0 > > > them.=C2=A0=C2=A0But can you help me with exactly what changes you ma= de so > > > these=C2=A0 > > > things built and made trouble, compared to the defconfig I have > > > used=C2=A0 > > > until now? > >=20 > > If you already have a build directory you are using, the simplest > > way > > is to edit the .config file in there and change the following from > > =3Dn > > to =3Dy: > >=20 > > CONFIG_RTE_LIBRTE_MLX4_PMD > > CONFIG_RTE_LIBRTE_MLX5_PMD > > CONFIG_RTE_LIBRTE_BNX2X_PMD > >=20 > > Then rebuild and you should see the errors. > >=20 >=20 > Personally, though I wouldn't view it as necessary to get those extra > fixes > into this set. The set is big enough as it is, so I'd like to see the > existing gcc 8 fixes we have merged to make some progress, rather > than > constantly spinning ever bigger sets to try and fix them all in one > go. >=20 > My 2c. >=20 > /Bruce Yes I agree, they can be done separately, I just reported them because I had been asked to give the patchset a spin, and I have those PMDs enabled so I noticed those errors.=20 --=20 Kind regards, Luca Boccassi