On 05/10/2018 09:36 PM, 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.
Sorry, but your codebase just keeps coming up with new things to fix.
Unfortunately I have yet to see gcc8 complain about something that was
not a real problem in the code.
If I were maintaining this, what I would do is look through the freebies
I am getting from that awesome guy who is donating his time fixing my
code, and anything that I could understand was useful, I would apply,
let him rebase out the stuff that is in, reissue, until everything that
is going in, is in.
You don't have to apply the whole series, that is just how I am posting
them because that is how they are in my tree. Likewise, if I post 2 x
20 patches, instead of 1 x 40, it makes no difference what you choose to
cherrypick.
The problems Luca pointed our are caused by problems in your code, not
gcc8, and not me fixing more things in your code for free.
I get it you want to make a release but am I the only person throwing
gcc8 quality-related patches at you? Then maybe you should take a pause
and absorb the quality improvements instead of releasing known-broken
code (let us pretend there are not 166 coverity breakages).
My 2c.
...and I would get rid of completely pointless "quality theater"
roadblocks like this git subject grep stuff.
-Andy
/Bruce
