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:
> > >
> > > 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.
> > > > >
> > > > > 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. The first workaround patch for the hash
> > > > > function cast problem is dropped since something has already
> > > > > been applied in master since yesterday to address it.
> > > > >
> > > > > 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.
> > > > >
> > > > > ---
> > > > >
> > > > > Andy Green (40):
> > > > > drivers/bus/pci: fix strncpy dangerous code
> > > > > drivers/bus/dpaa: fix inconsistent struct alignment
> > > > > drivers/net/axgbe: fix broken eeprom string comp
> > > > > drivers/net/nfp/nfpcore: fix strncpy misuse
> > > > > drivers/net/nfp/nfpcore: fix off-by-one and no NUL on
> > > > > strncpy
> > > > > use
> > > > > drivers/net/nfp: don't memcpy out of source range
> > > > > drivers/net/nfp: fix buffer overflow in fw_name
> > > > > drivers/net/qede: fix strncpy constant and NUL
> > > > > drivers/net/qede: fix broken strncpy
> > > > > drivers/net/sfc: fix strncpy length
> > > > > drivers/net/sfc: fix strncpy size and NUL
> > > > > drivers/net/vdev: readlink inputs cannot be aliased
> > > > > drivers/net/vdev: fix 3 x strncpy misuse
> > > > > app/test-pmd: can't find include
> > > > > app/proc-info: fix sprintf overrun bug
> > > > > app/test-bbdev: test-bbdev: strcpy ok for allocated
> > > > > string
> > > > > app/test-bbdev: strcpy ok for allocated string
> > > > > rte_common.h: cast gcc builtin result to avoid
> > > > > complaints
> > > > > rte_memcpy.h: explicit tmp cast
> > > > > lib/librte_eal/common/include/rte_lcore.h: explicit
> > > > > cast
> > > > > for
> > > > > signed change
> > > > > /lib/librte_eal/common/include/rte_random.h: stage
> > > > > cast
> > > > > from
> > > > > uint64_t to long
> > > > > rte_spinlock.h: stack declarations before code
> > > > > rte_ring_generic.h: stack declarations before code
> > > > > rte_ring.h: remove signed type flipflopping
> > > > > rte_dev.h: stack declaration at top of own basic block
> > > > > rte_mbuf.h: avoid truncation warnings from inadvertant
> > > > > int16_t
> > > > > to int promotion
> > > > > rte_mbuf.h: explicit casts for flipping between
> > > > > int16_t
> > > > > and
> > > > > uint16_t
> > > > > rte_mbuf.h: make sure RTE_MIN compares same types
> > > > > rte_mbuf.h: explicit cast restricting ptrdiff to
> > > > > uint16_t
> > > > > rte_mbuf.h: explicit cast for size_t to uint32_t
> > > > > rte_mbuf.h: explicit casts to uint16_t to avoid
> > > > > truncation
> > > > > warnings
> > > > > rte_byteorder.h: explicit cast for return promotion
> > > > > rte_ether.h: explicit cast avoiding truncation warning
> > > > > rte_ether.h: stack vars declared at top of function
> > > > > rte_ethdev.h: fix sign and scope of temp var
> > > > > rte_ethdev.h: explicit cast for return type
> > > > > rte_ethdev.h: explicit cast for truncation
> > > > > rte_hash_crc.h: stack vars declared at top of function
> > > > > rte_hash_crc.h: explicit casts for truncation
> > > > > rte_string_fns.h: explicit cast for int return to
> > > > > size_t
> > > >
> > > > Hi,
> > > >
> > > > I've built-tested this series on Debian Stretch (gcc 6.3) and
> > > > Debian
> > > > Sid (gcc 8.1).
> > > >
> > > > The series builds fine with the default config, but the bnx2x
> > > > and
> > > > mlx5
> > > > PMDs still have errors with gcc-8:
> > >
> > > Yes I just built it with defconfig for x86_64 on Fedora 28 with
> > > default
> > > tools and cleared out everything that came up.
> > >
> > > > /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=format-overflow=]
> > > > sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, sc->pcie_device,
> > > > msg,
> > > > ^~
> > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:8874:7:
> > > > if (bnx2x_dma_alloc(sc, sizeof(union
> > > > bnx2x_host_hc_status_block),
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~~~
> > > > ~~
> > > > &fp->sb_dma, buf, RTE_CACHE_LINE_SIZE) != 0) {
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:176:3: note: 'sprintf'
> > > > output
> > > > between 10 and 66 bytes into a destination of size 32
> > > > sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, sc->pcie_device,
> > > > msg,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~
> > > > rte_get_timer_cycles());
> > > > ~~~~~~~~~~~~~~~~~~~~~~~
> > > > /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=format-overflow=]
> > > > sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, SC_ABS_FUNC(sc),
> > > > msg,
> > > > ^~
> > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:8874:7:
> > > > if (bnx2x_dma_alloc(sc, sizeof(union
> > > > bnx2x_host_hc_status_block),
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~~~
> > > > ~~
> > > > &fp->sb_dma, buf, RTE_CACHE_LINE_SIZE) != 0) {
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /tmp/dpdk/drivers/net/bnx2x/bnx2x.c:173:3: note: 'sprintf'
> > > > output
> > > > between 10 and 58 bytes into a destination of size 32
> > > > sprintf(mz_name, "bnx2x%d_%s_%" PRIx64, SC_ABS_FUNC(sc),
> > > > msg,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > ~~
> > > > rte_get_timer_cycles());
> > > >
> > > >
> > > > /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=maybe-uninitialized]
> > > > config.vf = vf;
> > > >
> > > > Hope this can be useful.
> > >
> > > I think gcc 8.0.1 is capable to show that and I am willing to
> > > look
> > > at
> > > them. But can you help me with exactly what changes you made so
> > > these
> > > things built and made trouble, compared to the defconfig I have
> > > used
> > > until now?
> >
> > 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
> > =n
> > to =y:
> >
> > CONFIG_RTE_LIBRTE_MLX4_PMD
> > CONFIG_RTE_LIBRTE_MLX5_PMD
> > CONFIG_RTE_LIBRTE_BNX2X_PMD
> >
> > Then rebuild and you should see the errors.
> >
>
> 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.
>
> My 2c.
>
> /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.
--
Kind regards,
Luca Boccassi
