* [PATCH 0/1] fix GCC 13 build errors on 32-bit targets @ 2024-04-10 15:21 Luca Vizzarro 2024-04-10 15:21 ` [PATCH 1/1] vhost: fix GCC 13 build error Luca Vizzarro 0 siblings, 1 reply; 3+ messages in thread From: Luca Vizzarro @ 2024-04-10 15:21 UTC (permalink / raw) To: dev; +Cc: luca.boccassi, paul.szczepanek, nick.connolly, Luca Vizzarro, stable Hi everyone, sending in a patch to resolve some build issues encountered with GCC 13.2 when building against 32-bit targets. These issues were originally reported by Luca Boccassi in one of his stable builds[1]. Although, I have noticed that this problem presents itself on more versions (e.g. 21.11) as well, and it's not specific to armv7-a as the build suggests. I haven't actually noticed the problem present itself on mainline, but it doesn't mean that it won't appear in the future. This patch should correctly fix this problem. This patch is meant to target mainline, and all the currently supported stable versions. [1] https://build.opensuse.org/package/live_build_log/home:bluca:dpdk/dpdk-22.11/Debian_Next/armv7l Luca Vizzarro (1): vhost: fix GCC 13 build error lib/vhost/virtio_net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] vhost: fix GCC 13 build error 2024-04-10 15:21 [PATCH 0/1] fix GCC 13 build errors on 32-bit targets Luca Vizzarro @ 2024-04-10 15:21 ` Luca Vizzarro 2024-04-25 13:44 ` Maxime Coquelin 0 siblings, 1 reply; 3+ messages in thread From: Luca Vizzarro @ 2024-04-10 15:21 UTC (permalink / raw) To: dev; +Cc: luca.boccassi, paul.szczepanek, nick.connolly, Luca Vizzarro, stable This patch resolves a build error with GCC 13 and arm/aarch32 as targets: In function ‘mbuf_to_desc’, inlined from ‘vhost_enqueue_async_packed’ at ../lib/vhost/virtio_net.c:1828:6, inlined from ‘virtio_dev_rx_async_packed’ at ../lib/vhost/virtio_net.c:1842:6, inlined from ‘virtio_dev_rx_async_submit_packed’ at ../lib/vhost/virtio_net.c:1900:7: ../lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized] 1159 | buf_addr = buf_vec[vec_idx].buf_addr; | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ <snip> ../lib/vhost/virtio_net.c:1160:18: error: ‘buf_vec[0].buf_iova’ may be used uninitialized [-Werror=maybe-uninitialized] 1160 | buf_iova = buf_vec[vec_idx].buf_iova; | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ <snip> ../lib/vhost/virtio_net.c:1161:35: error: ‘buf_vec[0].buf_len’ may be used uninitialized [-Werror=maybe-uninitialized] 1161 | buf_len = buf_vec[vec_idx].buf_len; | ~~~~~~~~~~~~~~~~^~~~~~~~ GCC complains about the possible runtime path where the while loop which fills buf_vec (in vhost_enqueue_async_packed) is not run. As a consequence it correctly thinks that buf_vec is not initialized while being accessed anyways. This scenario is actually very unlikely as the only way this can occur is if size has overflowed to 0. Meaning that the total packet length would be close to UINT64_MAX (or actually UINT32_MAX). At first glance, the code suggests that this may never happen as the type of size has been changed to 64-bit. For a 32-bit architecture such as arm (e.g. armv7-a) and aarch32, this still happens because the operand types (pkt->pkt_len and sizeof) are 32-bit wide, performing 32-bit arithmetic first (where the overflow can happen) and widening to 64-bit later. The proposed fix simply guarantees to the compiler that the scope which fills buf_vec is accessed at least once, while not disrupting the actual logic. This is based on the assumption that size will always be greater than 0, as suggested by the sizeof, and the packet length will never be as big as UINT32_MAX, and causing an overflow. Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath") Cc: stable@dpdk.org Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com> Reviewed-by: Nick Connolly <nick.connolly@arm.com> --- lib/vhost/virtio_net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 1359c5fb1f..6a2ca295f5 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -1935,7 +1935,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, else max_tries = 1; - while (size > 0) { + do { /* * if we tried all available ring items, and still * can't get enough buf, it means something abnormal @@ -1962,7 +1962,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, avail_idx += desc_count; if (avail_idx >= vq->size) avail_idx -= vq->size; - } + } while (size > 0); if (unlikely(mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, *nr_buffers, true) < 0)) return -1; -- 2.34.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] vhost: fix GCC 13 build error 2024-04-10 15:21 ` [PATCH 1/1] vhost: fix GCC 13 build error Luca Vizzarro @ 2024-04-25 13:44 ` Maxime Coquelin 0 siblings, 0 replies; 3+ messages in thread From: Maxime Coquelin @ 2024-04-25 13:44 UTC (permalink / raw) To: Luca Vizzarro, dev; +Cc: luca.boccassi, paul.szczepanek, nick.connolly, stable On 4/10/24 17:21, Luca Vizzarro wrote: > This patch resolves a build error with GCC 13 and arm/aarch32 as > targets: > > In function ‘mbuf_to_desc’, > inlined from ‘vhost_enqueue_async_packed’ at > ../lib/vhost/virtio_net.c:1828:6, > inlined from ‘virtio_dev_rx_async_packed’ at > ../lib/vhost/virtio_net.c:1842:6, > inlined from ‘virtio_dev_rx_async_submit_packed’ at > ../lib/vhost/virtio_net.c:1900:7: > ../lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may > be used uninitialized [-Werror=maybe-uninitialized] > 1159 | buf_addr = buf_vec[vec_idx].buf_addr; > | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > <snip> > ../lib/vhost/virtio_net.c:1160:18: error: ‘buf_vec[0].buf_iova’ may > be used uninitialized [-Werror=maybe-uninitialized] > 1160 | buf_iova = buf_vec[vec_idx].buf_iova; > | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > <snip> > ../lib/vhost/virtio_net.c:1161:35: error: ‘buf_vec[0].buf_len’ may > be used uninitialized [-Werror=maybe-uninitialized] > 1161 | buf_len = buf_vec[vec_idx].buf_len; > | ~~~~~~~~~~~~~~~~^~~~~~~~ > > GCC complains about the possible runtime path where the while loop > which fills buf_vec (in vhost_enqueue_async_packed) is not run. As a > consequence it correctly thinks that buf_vec is not initialized while > being accessed anyways. > > This scenario is actually very unlikely as the only way this can occur > is if size has overflowed to 0. Meaning that the total packet length > would be close to UINT64_MAX (or actually UINT32_MAX). At first glance, > the code suggests that this may never happen as the type of size has > been changed to 64-bit. For a 32-bit architecture such as arm > (e.g. armv7-a) and aarch32, this still happens because the operand types > (pkt->pkt_len and sizeof) are 32-bit wide, performing 32-bit arithmetic > first (where the overflow can happen) and widening to 64-bit later. > > The proposed fix simply guarantees to the compiler that the scope which > fills buf_vec is accessed at least once, while not disrupting the actual > logic. This is based on the assumption that size will always be greater > than 0, as suggested by the sizeof, and the packet length will never be > as big as UINT32_MAX, and causing an overflow. > > Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath") > Cc: stable@dpdk.org > > Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com> > Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com> > Reviewed-by: Nick Connolly <nick.connolly@arm.com> > --- > lib/vhost/virtio_net.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-25 13:44 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-10 15:21 [PATCH 0/1] fix GCC 13 build errors on 32-bit targets Luca Vizzarro 2024-04-10 15:21 ` [PATCH 1/1] vhost: fix GCC 13 build error Luca Vizzarro 2024-04-25 13:44 ` Maxime Coquelin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).