* [PATCH] vhost: compilation fix for GCC-12
@ 2022-09-01 8:49 Amit Prakash Shukla
2022-09-02 7:23 ` Ruifeng Wang
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-09-01 8:49 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia; +Cc: dev, jerinj, stable, Amit Prakash Shukla
../lib/vhost/virtio_net.c:941:35: error:
'buf_vec[0].buf_len' may be used uninitialized
[-Werror=maybe-uninitialized]
941 | buf_len = buf_vec[vec_idx].buf_len;
| ~~~~~~~~~~~~~~~~^~~~~~~~
../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
cc1: all warnings being treated as errors
Fixes: 93520085efda ("vhost: add packed ring single enqueue")
Cc: stable@dpdk.org
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
lib/vhost/virtio_net.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index b3d954aab4..0220bc923c 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1069,6 +1069,12 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
else
max_tries = 1;
+ /* To avoid GCC-12 warning.
+ * GCC-12 is not evaluating sizeof at compile time.
+ */
+ if (unlikely(size == 0))
+ return -1;
+
while (size > 0) {
/*
* if we tried all available ring items, and still
@@ -1574,6 +1580,12 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
else
max_tries = 1;
+ /* To avoid GCC-12 warning.
+ * GCC-12 is not evaluating sizeof at compile time.
+ */
+ if (unlikely(size == 0))
+ return -1;
+
while (size > 0) {
/*
* if we tried all available ring items, and still
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] vhost: compilation fix for GCC-12
2022-09-01 8:49 [PATCH] vhost: compilation fix for GCC-12 Amit Prakash Shukla
@ 2022-09-02 7:23 ` Ruifeng Wang
2022-09-02 11:57 ` Amit Prakash Shukla
2022-09-02 12:41 ` [PATCH v2] " Amit Prakash Shukla
2022-09-02 15:06 ` [PATCH v3] " Amit Prakash Shukla
2 siblings, 1 reply; 11+ messages in thread
From: Ruifeng Wang @ 2022-09-02 7:23 UTC (permalink / raw)
To: Amit Prakash Shukla, Maxime Coquelin, Chenbo Xia; +Cc: dev, jerinj, stable, nd
> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Thursday, September 1, 2022 4:50 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; stable@dpdk.org; Amit Prakash Shukla
> <amitprakashs@marvell.com>
> Subject: [PATCH] vhost: compilation fix for GCC-12
>
> ../lib/vhost/virtio_net.c:941:35: error:
> 'buf_vec[0].buf_len' may be used uninitialized
> [-Werror=maybe-uninitialized]
> 941 | buf_len = buf_vec[vec_idx].buf_len;
> | ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> | ^~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
> lib/vhost/virtio_net.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index b3d954aab4..0220bc923c
> 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1069,6 +1069,12 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
> else
> max_tries = 1;
>
> + /* To avoid GCC-12 warning.
> + * GCC-12 is not evaluating sizeof at compile time.
Is this a compiler behavior change against previous versions?
I tried to find some clue from gcc-12 doc but got nothing. Can you point me to any material?
> + */
> + if (unlikely(size == 0))
> + return -1;
> +
> while (size > 0) {
Change 'while(){}' to 'do{}while()' can be a simpler solution. What do you think?
Thanks.
> /*
> * if we tried all available ring items, and still @@ -1574,6 +1580,12 @@
> vhost_enqueue_async_packed(struct virtio_net *dev,
> else
> max_tries = 1;
>
> + /* To avoid GCC-12 warning.
> + * GCC-12 is not evaluating sizeof at compile time.
> + */
> + if (unlikely(size == 0))
> + return -1;
> +
> while (size > 0) {
> /*
> * if we tried all available ring items, and still
> --
> 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] vhost: compilation fix for GCC-12
2022-09-02 7:23 ` Ruifeng Wang
@ 2022-09-02 11:57 ` Amit Prakash Shukla
0 siblings, 0 replies; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-09-02 11:57 UTC (permalink / raw)
To: Ruifeng Wang, Maxime Coquelin, Chenbo Xia
Cc: dev, Jerin Jacob Kollanukkaran, stable, nd
Thanks Ruifeng for the code review and feedback. Please find my response inline.
> -----Original Message-----
> From: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Sent: Friday, September 2, 2022 12:54 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Chenbo Xia <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> stable@dpdk.org; nd <nd@arm.com>
> Subject: [EXT] RE: [PATCH] vhost: compilation fix for GCC-12
>
> External Email
>
> ----------------------------------------------------------------------
> > -----Original Message-----
> > From: Amit Prakash Shukla <amitprakashs@marvell.com>
> > Sent: Thursday, September 1, 2022 4:50 PM
> > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia
> > <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; jerinj@marvell.com; stable@dpdk.org; Amit Prakash
> > Shukla <amitprakashs@marvell.com>
> > Subject: [PATCH] vhost: compilation fix for GCC-12
> >
> > ../lib/vhost/virtio_net.c:941:35: error:
> > 'buf_vec[0].buf_len' may be used uninitialized
> > [-Werror=maybe-uninitialized]
> > 941 | buf_len = buf_vec[vec_idx].buf_len;
> > | ~~~~~~~~~~~~~~~~^~~~~~~~
> > ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> > ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> > 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> > | ^~~~~~~
> > cc1: all warnings being treated as errors
> >
> > Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > ---
> > lib/vhost/virtio_net.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index
> > b3d954aab4..0220bc923c
> > 100644
> > --- a/lib/vhost/virtio_net.c
> > +++ b/lib/vhost/virtio_net.c
> > @@ -1069,6 +1069,12 @@ vhost_enqueue_single_packed(struct virtio_net
> *dev,
> > else
> > max_tries = 1;
> >
> > + /* To avoid GCC-12 warning.
> > + * GCC-12 is not evaluating sizeof at compile time.
> Is this a compiler behavior change against previous versions?
> I tried to find some clue from gcc-12 doc but got nothing. Can you point me to
> any material?
Apologies for the wrong wordings in the comment. In the comment I mean, it seems like
point at which sizeof gets evaluated during compilation has changed. I am not sure on it though.
I too could not find documentation regarding the same.
>
> > + */
> > + if (unlikely(size == 0))
> > + return -1;
> > +
> > while (size > 0) {
> Change 'while(){}' to 'do{}while()' can be a simpler solution. What do you
> think?
I agree, solution suggested by you is better than the one in patch. I will make the suggested
changes as part of v2. Thanks.
>
> Thanks.
>
> > /*
> > * if we tried all available ring items, and still @@ -1574,6
> > +1580,12 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
> > else
> > max_tries = 1;
> >
> > + /* To avoid GCC-12 warning.
> > + * GCC-12 is not evaluating sizeof at compile time.
> > + */
> > + if (unlikely(size == 0))
> > + return -1;
> > +
> > while (size > 0) {
> > /*
> > * if we tried all available ring items, and still
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] vhost: compilation fix for GCC-12
2022-09-01 8:49 [PATCH] vhost: compilation fix for GCC-12 Amit Prakash Shukla
2022-09-02 7:23 ` Ruifeng Wang
@ 2022-09-02 12:41 ` Amit Prakash Shukla
2022-09-02 12:56 ` Bagas Sanjaya
2022-09-02 15:06 ` [PATCH v3] " Amit Prakash Shukla
2 siblings, 1 reply; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-09-02 12:41 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia
Cc: dev, jerinj, stable, Ruifeng.Wang, Amit Prakash Shukla
../lib/vhost/virtio_net.c:941:35: error:
'buf_vec[0].buf_len' may be used uninitialized
[-Werror=maybe-uninitialized]
941 | buf_len = buf_vec[vec_idx].buf_len;
| ~~~~~~~~~~~~~~~~^~~~~~~~
../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
cc1: all warnings being treated as errors
Fixes: 93520085efda ("vhost: add packed ring single enqueue")
Cc: stable@dpdk.org
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
v2:
- Changes for code review suggestion
lib/vhost/virtio_net.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index b3d954aab4..9b77d3d10f 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1069,7 +1069,7 @@ vhost_enqueue_single_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
@@ -1097,7 +1097,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
avail_idx += desc_count;
if (avail_idx >= vq->size)
avail_idx -= vq->size;
- }
+ } while (size > 0);
if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers, false) < 0)
return -1;
@@ -1574,7 +1574,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
@@ -1601,7 +1601,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.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] vhost: compilation fix for GCC-12
2022-09-02 12:41 ` [PATCH v2] " Amit Prakash Shukla
@ 2022-09-02 12:56 ` Bagas Sanjaya
2022-09-02 15:11 ` [EXT] " Amit Prakash Shukla
0 siblings, 1 reply; 11+ messages in thread
From: Bagas Sanjaya @ 2022-09-02 12:56 UTC (permalink / raw)
To: Amit Prakash Shukla
Cc: Maxime Coquelin, Chenbo Xia, dev, jerinj, stable, Ruifeng.Wang
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
On Fri, Sep 02, 2022 at 06:11:52PM +0530, Amit Prakash Shukla wrote:
> ../lib/vhost/virtio_net.c:941:35: error:
> 'buf_vec[0].buf_len' may be used uninitialized
> [-Werror=maybe-uninitialized]
> 941 | buf_len = buf_vec[vec_idx].buf_len;
> | ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> | ^~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> Cc: stable@dpdk.org
Please describe what this patch is doing (the current state of code,
why it errored, and how it is fixing the error). Write the description
in imperative mood. I don't see the description other than error message
above.
Also, for stable patches submission, Cc stable@vger.kernel.org.
Thanks.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] vhost: compilation fix for GCC-12
2022-09-01 8:49 [PATCH] vhost: compilation fix for GCC-12 Amit Prakash Shukla
2022-09-02 7:23 ` Ruifeng Wang
2022-09-02 12:41 ` [PATCH v2] " Amit Prakash Shukla
@ 2022-09-02 15:06 ` Amit Prakash Shukla
2022-10-06 7:22 ` Amit Prakash Shukla
2 siblings, 1 reply; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-09-02 15:06 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia
Cc: dev, jerinj, stable, Ruifeng.Wang, Amit Prakash Shukla
GCC-12 complains about the possible use of un-initialized array. At
compile time it seems like it is not able to evaluate the size as it
involves run-time variable and at compile time it seems like gcc assumes
value of "size" variable to be zero which makes gcc-12 to jump the while
loop.
"size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);"
As part of the fix, "while (){}" is replaced by "do {} while()" which make
the compiler to generate a code in which buf_vec will never be used
un-initialized.
../lib/vhost/virtio_net.c:941:35: error:
'buf_vec[0].buf_len' may be used uninitialized
[-Werror=maybe-uninitialized]
941 | buf_len = buf_vec[vec_idx].buf_len;
| ~~~~~~~~~~~~~~~~^~~~~~~~
../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
cc1: all warnings being treated as errors
Fixes: 93520085efda ("vhost: add packed ring single enqueue")
Cc: stable@dpdk.org
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
v2:
- Changes for code review suggestion
v3:
- Added a description
lib/vhost/virtio_net.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index b3d954aab4..9b77d3d10f 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1069,7 +1069,7 @@ vhost_enqueue_single_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
@@ -1097,7 +1097,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
avail_idx += desc_count;
if (avail_idx >= vq->size)
avail_idx -= vq->size;
- }
+ } while (size > 0);
if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers, false) < 0)
return -1;
@@ -1574,7 +1574,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
@@ -1601,7 +1601,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.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH v2] vhost: compilation fix for GCC-12
2022-09-02 12:56 ` Bagas Sanjaya
@ 2022-09-02 15:11 ` Amit Prakash Shukla
0 siblings, 0 replies; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-09-02 15:11 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Maxime Coquelin, Chenbo Xia, dev, Jerin Jacob Kollanukkaran,
stable, Ruifeng.Wang
Thanks for the feedback.
> -----Original Message-----
> From: Bagas Sanjaya <bagasdotme@gmail.com>
> Sent: Friday, September 2, 2022 6:26 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia
> <chenbo.xia@intel.com>; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org; Ruifeng.Wang@arm.com
> Subject: [EXT] Re: [PATCH v2] vhost: compilation fix for GCC-12
>
> External Email
>
> ----------------------------------------------------------------------
> On Fri, Sep 02, 2022 at 06:11:52PM +0530, Amit Prakash Shukla wrote:
> > ../lib/vhost/virtio_net.c:941:35: error:
> > 'buf_vec[0].buf_len' may be used uninitialized
> > [-Werror=maybe-uninitialized]
> > 941 | buf_len = buf_vec[vec_idx].buf_len;
> > | ~~~~~~~~~~~~~~~~^~~~~~~~
> > ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> > ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> > 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> > | ^~~~~~~
> > cc1: all warnings being treated as errors
> >
> > Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> > Cc: stable@dpdk.org
>
> Please describe what this patch is doing (the current state of code, why it
> errored, and how it is fixing the error). Write the description in imperative
> mood. I don't see the description other than error message above.
I have pushed v3 incorporating your suggestion regarding description.
>
> Also, for stable patches submission, Cc stable@vger.kernel.org.
>
> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3] vhost: compilation fix for GCC-12
2022-09-02 15:06 ` [PATCH v3] " Amit Prakash Shukla
@ 2022-10-06 7:22 ` Amit Prakash Shukla
2022-10-06 7:24 ` Maxime Coquelin
0 siblings, 1 reply; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-10-06 7:22 UTC (permalink / raw)
To: Amit Prakash Shukla, Maxime Coquelin, Chenbo Xia
Cc: dev, Jerin Jacob Kollanukkaran, stable, Ruifeng.Wang
Ping.
> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Friday, September 2, 2022 8:36 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> stable@dpdk.org; Ruifeng.Wang@arm.com; Amit Prakash Shukla
> <amitprakashs@marvell.com>
> Subject: [PATCH v3] vhost: compilation fix for GCC-12
>
> GCC-12 complains about the possible use of un-initialized array. At compile
> time it seems like it is not able to evaluate the size as it involves run-time
> variable and at compile time it seems like gcc assumes value of "size" variable
> to be zero which makes gcc-12 to jump the while loop.
> "size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);"
>
> As part of the fix, "while (){}" is replaced by "do {} while()" which make the
> compiler to generate a code in which buf_vec will never be used un-
> initialized.
>
> ../lib/vhost/virtio_net.c:941:35: error:
> 'buf_vec[0].buf_len' may be used uninitialized
> [-Werror=maybe-uninitialized]
> 941 | buf_len = buf_vec[vec_idx].buf_len;
> | ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> | ^~~~~~~
> cc1: all warnings being treated as errors
>
> Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> Cc: stable@dpdk.org
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
> v2:
> - Changes for code review suggestion
>
> v3:
> - Added a description
>
> lib/vhost/virtio_net.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index
> b3d954aab4..9b77d3d10f 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1069,7 +1069,7 @@ vhost_enqueue_single_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 @@ -
> 1097,7 +1097,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
> avail_idx += desc_count;
> if (avail_idx >= vq->size)
> avail_idx -= vq->size;
> - }
> + } while (size > 0);
>
> if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers, false)
> < 0)
> return -1;
> @@ -1574,7 +1574,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 @@ -
> 1601,7 +1601,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.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] vhost: compilation fix for GCC-12
2022-10-06 7:22 ` Amit Prakash Shukla
@ 2022-10-06 7:24 ` Maxime Coquelin
2022-10-06 8:05 ` [EXT] " Amit Prakash Shukla
0 siblings, 1 reply; 11+ messages in thread
From: Maxime Coquelin @ 2022-10-06 7:24 UTC (permalink / raw)
To: Amit Prakash Shukla, Chenbo Xia
Cc: dev, Jerin Jacob Kollanukkaran, stable, Ruifeng.Wang
Hi Amit,
On 10/6/22 09:22, Amit Prakash Shukla wrote:
> Ping.
I posted a patch yesterday that fixes this issue without having to
change the runtime behavior:
http://patches.dpdk.org/project/dpdk/patch/20221005203524.89336-1-maxime.coquelin@redhat.com/
Could you test it and provide ack if OK on your side?
Thanks,
Maxime
>> -----Original Message-----
>> From: Amit Prakash Shukla <amitprakashs@marvell.com>
>> Sent: Friday, September 2, 2022 8:36 PM
>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia
>> <chenbo.xia@intel.com>
>> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> stable@dpdk.org; Ruifeng.Wang@arm.com; Amit Prakash Shukla
>> <amitprakashs@marvell.com>
>> Subject: [PATCH v3] vhost: compilation fix for GCC-12
>>
>> GCC-12 complains about the possible use of un-initialized array. At compile
>> time it seems like it is not able to evaluate the size as it involves run-time
>> variable and at compile time it seems like gcc assumes value of "size" variable
>> to be zero which makes gcc-12 to jump the while loop.
>> "size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);"
>>
>> As part of the fix, "while (){}" is replaced by "do {} while()" which make the
>> compiler to generate a code in which buf_vec will never be used un-
>> initialized.
>>
>> ../lib/vhost/virtio_net.c:941:35: error:
>> 'buf_vec[0].buf_len' may be used uninitialized
>> [-Werror=maybe-uninitialized]
>> 941 | buf_len = buf_vec[vec_idx].buf_len;
>> | ~~~~~~~~~~~~~~~~^~~~~~~~
>> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
>> ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
>> 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
>> | ^~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Fixes: 93520085efda ("vhost: add packed ring single enqueue")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
>> ---
>> v2:
>> - Changes for code review suggestion
>>
>> v3:
>> - Added a description
>>
>> lib/vhost/virtio_net.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index
>> b3d954aab4..9b77d3d10f 100644
>> --- a/lib/vhost/virtio_net.c
>> +++ b/lib/vhost/virtio_net.c
>> @@ -1069,7 +1069,7 @@ vhost_enqueue_single_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 @@ -
>> 1097,7 +1097,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
>> avail_idx += desc_count;
>> if (avail_idx >= vq->size)
>> avail_idx -= vq->size;
>> - }
>> + } while (size > 0);
>>
>> if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers, false)
>> < 0)
>> return -1;
>> @@ -1574,7 +1574,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 @@ -
>> 1601,7 +1601,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.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH v3] vhost: compilation fix for GCC-12
2022-10-06 7:24 ` Maxime Coquelin
@ 2022-10-06 8:05 ` Amit Prakash Shukla
2022-10-06 12:35 ` David Marchand
0 siblings, 1 reply; 11+ messages in thread
From: Amit Prakash Shukla @ 2022-10-06 8:05 UTC (permalink / raw)
To: Maxime Coquelin, Chenbo Xia
Cc: dev, Jerin Jacob Kollanukkaran, stable, Ruifeng.Wang
Hi Maxime,
Sure, I will test and reply on your patch.
Thanks,
Amit Shukla
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, October 6, 2022 12:55 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Chenbo Xia
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> stable@dpdk.org; Ruifeng.Wang@arm.com
> Subject: [EXT] Re: [PATCH v3] vhost: compilation fix for GCC-12
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Amit,
>
> On 10/6/22 09:22, Amit Prakash Shukla wrote:
> > Ping.
>
> I posted a patch yesterday that fixes this issue without having to change the
> runtime behavior:
>
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__patches.dpdk.org_project_dpdk_patch_20221005203524.89336-2D1-
> 2Dmaxime.coquelin-
> 40redhat.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFG
> R69VnJLdSnADun7zLaXG1p5Rs7pXihE&m=vDZyV-
> pKSmtssm0L8YUPR6doPgIw44tOh3xWgNEwD3FGjqEomaP9lcYLhrbqwGKb&s
> =g54ujCkU-mDT0CQoJgqYlDllGLuzWs8lLa6YMzPfqkw&e=
>
> Could you test it and provide ack if OK on your side?
>
> Thanks,
> Maxime
>
> >> -----Original Message-----
> >> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> >> Sent: Friday, September 2, 2022 8:36 PM
> >> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Chenbo Xia
> >> <chenbo.xia@intel.com>
> >> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> >> stable@dpdk.org; Ruifeng.Wang@arm.com; Amit Prakash Shukla
> >> <amitprakashs@marvell.com>
> >> Subject: [PATCH v3] vhost: compilation fix for GCC-12
> >>
> >> GCC-12 complains about the possible use of un-initialized array. At
> >> compile time it seems like it is not able to evaluate the size as it
> >> involves run-time variable and at compile time it seems like gcc
> >> assumes value of "size" variable to be zero which makes gcc-12 to jump
> the while loop.
> >> "size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);"
> >>
> >> As part of the fix, "while (){}" is replaced by "do {} while()" which
> >> make the compiler to generate a code in which buf_vec will never be
> >> used un- initialized.
> >>
> >> ../lib/vhost/virtio_net.c:941:35: error:
> >> 'buf_vec[0].buf_len' may be used uninitialized
> >> [-Werror=maybe-uninitialized]
> >> 941 | buf_len = buf_vec[vec_idx].buf_len;
> >> | ~~~~~~~~~~~~~~~~^~~~~~~~
> >> ../lib/vhost/virtio_net.c: In function 'virtio_dev_rx_packed':
> >> ../lib/vhost/virtio_net.c:1285:27: note: 'buf_vec' declared here
> >> 1285 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
> >> | ^~~~~~~
> >> cc1: all warnings being treated as errors
> >>
> >> Fixes: 93520085efda ("vhost: add packed ring single enqueue")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> >> ---
> >> v2:
> >> - Changes for code review suggestion
> >>
> >> v3:
> >> - Added a description
> >>
> >> lib/vhost/virtio_net.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index
> >> b3d954aab4..9b77d3d10f 100644
> >> --- a/lib/vhost/virtio_net.c
> >> +++ b/lib/vhost/virtio_net.c
> >> @@ -1069,7 +1069,7 @@ vhost_enqueue_single_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 @@ -
> >> 1097,7 +1097,7 @@ vhost_enqueue_single_packed(struct virtio_net
> *dev,
> >> avail_idx += desc_count;
> >> if (avail_idx >= vq->size)
> >> avail_idx -= vq->size;
> >> - }
> >> + } while (size > 0);
> >>
> >> if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers,
> >> false) < 0)
> >> return -1;
> >> @@ -1574,7 +1574,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 @@ -
> >> 1601,7 +1601,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.25.1
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [EXT] Re: [PATCH v3] vhost: compilation fix for GCC-12
2022-10-06 8:05 ` [EXT] " Amit Prakash Shukla
@ 2022-10-06 12:35 ` David Marchand
0 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2022-10-06 12:35 UTC (permalink / raw)
To: Amit Prakash Shukla
Cc: Maxime Coquelin, Chenbo Xia, dev, Jerin Jacob Kollanukkaran,
stable, Ruifeng.Wang
On Thu, Oct 6, 2022 at 10:05 AM Amit Prakash Shukla
<amitprakashs@marvell.com> wrote:
>
> Hi Maxime,
>
> Sure, I will test and reply on your patch.
Thomas merged Maxime alternative.
Marking this patch as rejected.
Thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-10-06 12:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 8:49 [PATCH] vhost: compilation fix for GCC-12 Amit Prakash Shukla
2022-09-02 7:23 ` Ruifeng Wang
2022-09-02 11:57 ` Amit Prakash Shukla
2022-09-02 12:41 ` [PATCH v2] " Amit Prakash Shukla
2022-09-02 12:56 ` Bagas Sanjaya
2022-09-02 15:11 ` [EXT] " Amit Prakash Shukla
2022-09-02 15:06 ` [PATCH v3] " Amit Prakash Shukla
2022-10-06 7:22 ` Amit Prakash Shukla
2022-10-06 7:24 ` Maxime Coquelin
2022-10-06 8:05 ` [EXT] " Amit Prakash Shukla
2022-10-06 12:35 ` David Marchand
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).