On 05/10/2018 09:53 PM, Andy Green wrote:
On 05/10/2018 09:49 PM, Luca Boccassi wrote:
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.
If they want to release known-broken stuff, they can fix your broken
stuff separately how they like.
But your problems are caused by a pile of real breakages (and attempts
to sticking-plaster over them with illegal casts) in this codebase.
I dunno why you find yourself agreeing that isn't the same priority as
fixing anything else with this stuff.
Fixes for your problems are here:
https://github.com/lws-team/dpdk/commits/master
along with the rest of my stuff.
In particular:
https://github.com/lws-team/dpdk/commit/6cead9e1947588ddccf0508acd01c17392cd849c
https://github.com/lws-team/dpdk/commit/e0063507d824bc153ae5fd3f8fe59ccbdf88ef89
https://github.com/lws-team/dpdk/commit/5daea1b9b24e1151ac8416d7b5c1e7d096389c51
https://github.com/lws-team/dpdk/commit/3e8d82bf693e1469e4143297ea4050023d50fa6b
-Andy
-Andy
