[dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs
Andy Green
andy at warmcat.com
Thu May 10 15:53:50 CEST 2018
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.
-Andy
More information about the dev
mailing list