From: Cédric Le Goater <clg@redhat.com>
GCC13 reports an error :
../util/async.c: In function ‘aio_bh_poll’:
include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
303 | (head)->sqh_last = &(elm)->field.sqe_next; \
| ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
| ^~~~~~~~~~~~~~~~~~~~
../util/async.c:161:17: note: ‘slice’ declared here
161 | BHListSlice slice;
| ^~~~~
../util/async.c:161:17: note: ‘ctx’ declared here
But the local variable 'slice' is removed from the global context list
in following loop of the same routine. Add a pragma to silent GCC.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
util/async.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/util/async.c b/util/async.c
index 21016a1ac7..de9b431236 100644
--- a/util/async.c
+++ b/util/async.c
@@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
/* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
+
+ /*
+ * GCC13 [-Werror=dangling-pointer=] complains that the local variable
+ * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
+ * list is emptied before this function returns.
+ */
+#if !defined(__clang__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdangling-pointer="
+#endif
QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
+#if !defined(__clang__)
+#pragma GCC diagnostic pop
+#endif
while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
QEMUBH *bh;
--
2.39.2
Hi everyone,
Fedora 38 rolled out recently. I did a distro upgrade, and Fedora 38 is now using
GCC 13 ... and now I can't build QEMU without this patch.
I'll use it as is for now. It would be nice to fix this upstream though :D
Thanks,
Daniel
On 3/21/23 13:16, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> GCC13 reports an error :
>
> ../util/async.c: In function ‘aio_bh_poll’:
> include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
> 303 | (head)->sqh_last = &(elm)->field.sqe_next; \
> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> | ^~~~~~~~~~~~~~~~~~~~
> ../util/async.c:161:17: note: ‘slice’ declared here
> 161 | BHListSlice slice;
> | ^~~~~
> ../util/async.c:161:17: note: ‘ctx’ declared here
>
> But the local variable 'slice' is removed from the global context list
> in following loop of the same routine. Add a pragma to silent GCC.
>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> util/async.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/util/async.c b/util/async.c
> index 21016a1ac7..de9b431236 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
>
> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> +
> + /*
> + * GCC13 [-Werror=dangling-pointer=] complains that the local variable
> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
> + * list is emptied before this function returns.
> + */
> +#if !defined(__clang__)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdangling-pointer="
> +#endif
> QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> +#if !defined(__clang__)
> +#pragma GCC diagnostic pop
> +#endif
>
> while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
> QEMUBH *bh;
On 21/03/2023 17.16, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> GCC13 reports an error :
>
> ../util/async.c: In function ‘aio_bh_poll’:
> include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
> 303 | (head)->sqh_last = &(elm)->field.sqe_next; \
> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> | ^~~~~~~~~~~~~~~~~~~~
> ../util/async.c:161:17: note: ‘slice’ declared here
> 161 | BHListSlice slice;
> | ^~~~~
> ../util/async.c:161:17: note: ‘ctx’ declared here
>
> But the local variable 'slice' is removed from the global context list
> in following loop of the same routine. Add a pragma to silent GCC.
>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> util/async.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/util/async.c b/util/async.c
> index 21016a1ac7..de9b431236 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
>
> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> +
> + /*
> + * GCC13 [-Werror=dangling-pointer=] complains that the local variable
> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
> + * list is emptied before this function returns.
> + */
> +#if !defined(__clang__)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdangling-pointer="
That warning parameter looks like a new one in GCC 13 ?
... so you have to check whether it's available before disabling
it, otherwise this will fail with older versions of GCC. I just
gave it a try with my GCC 8.5 and got this:
../../devel/qemu/util/async.c: In function ‘aio_bh_poll’:
../../devel/qemu/util/async.c:175:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
#pragma GCC diagnostic ignored "-Wdangling-pointer="
^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Thomas
What about reworking the code like this:
diff --git a/util/async.c b/util/async.c
index 21016a1ac7..b236bdfbd8 100644
--- a/util/async.c
+++ b/util/async.c
@@ -156,15 +156,14 @@ void aio_bh_call(QEMUBH *bh)
}
/* Multiple occurrences of aio_bh_poll cannot be called concurrently. */
-int aio_bh_poll(AioContext *ctx)
+static int aio_bh_poll_slice(AioContext *ctx, BHListSlice *slice)
{
- BHListSlice slice;
BHListSlice *s;
int ret = 0;
/* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
- QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
- QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
+ QSLIST_MOVE_ATOMIC(&slice->bh_list, &ctx->bh_list);
+ QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, slice, next);
while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
QEMUBH *bh;
@@ -191,6 +190,13 @@ int aio_bh_poll(AioContext *ctx)
return ret;
}
+int aio_bh_poll(AioContext *ctx)
+{
+ BHListSlice slice;
+
+ return aio_bh_poll_slice(ctx, &slice);
+}
+
void qemu_bh_schedule_idle(QEMUBH *bh)
{
aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
Would that work with GCC 13 and be acceptable?
Thomas
On Wed, Mar 22, 2023 at 08:11:37AM +0100, Thomas Huth wrote:
> On 21/03/2023 17.16, Cédric Le Goater wrote:
> > From: Cédric Le Goater <clg@redhat.com>
> >
> > GCC13 reports an error :
> >
> > ../util/async.c: In function ‘aio_bh_poll’:
> > include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
> > 303 | (head)->sqh_last = &(elm)->field.sqe_next; \
> > | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> > ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
> > 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> > | ^~~~~~~~~~~~~~~~~~~~
> > ../util/async.c:161:17: note: ‘slice’ declared here
> > 161 | BHListSlice slice;
> > | ^~~~~
> > ../util/async.c:161:17: note: ‘ctx’ declared here
> >
> > But the local variable 'slice' is removed from the global context list
> > in following loop of the same routine. Add a pragma to silent GCC.
> >
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > ---
> > util/async.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/util/async.c b/util/async.c
> > index 21016a1ac7..de9b431236 100644
> > --- a/util/async.c
> > +++ b/util/async.c
> > @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
> > /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
> > QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> > +
> > + /*
> > + * GCC13 [-Werror=dangling-pointer=] complains that the local variable
> > + * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
> > + * list is emptied before this function returns.
> > + */
> > +#if !defined(__clang__)
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wdangling-pointer="
>
> That warning parameter looks like a new one in GCC 13 ?
> ... so you have to check whether it's available before disabling
> it, otherwise this will fail with older versions of GCC. I just
> gave it a try with my GCC 8.5 and got this:
>
> ../../devel/qemu/util/async.c: In function ‘aio_bh_poll’:
> ../../devel/qemu/util/async.c:175:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
> #pragma GCC diagnostic ignored "-Wdangling-pointer="
> ^~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Thomas
>
> What about reworking the code like this:
>
> diff --git a/util/async.c b/util/async.c
> index 21016a1ac7..b236bdfbd8 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -156,15 +156,14 @@ void aio_bh_call(QEMUBH *bh)
> }
> /* Multiple occurrences of aio_bh_poll cannot be called concurrently. */
> -int aio_bh_poll(AioContext *ctx)
> +static int aio_bh_poll_slice(AioContext *ctx, BHListSlice *slice)
> {
> - BHListSlice slice;
> BHListSlice *s;
> int ret = 0;
> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
> - QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> - QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> + QSLIST_MOVE_ATOMIC(&slice->bh_list, &ctx->bh_list);
> + QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, slice, next);
> while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
> QEMUBH *bh;
> @@ -191,6 +190,13 @@ int aio_bh_poll(AioContext *ctx)
> return ret;
> }
> +int aio_bh_poll(AioContext *ctx)
> +{
> + BHListSlice slice;
> +
> + return aio_bh_poll_slice(ctx, &slice);
> +}
> +
> void qemu_bh_schedule_idle(QEMUBH *bh)
> {
> aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
>
> Would that work with GCC 13 and be acceptable?
Fine by me. Please add a comment into aio_bh_poll() explaining that this
wrapper function exists to silence the gcc dangling-pointer warning.
Otherwise someone may be tempted to remove the function.
Stefan
On 22/3/23 14:27, Stefan Hajnoczi wrote:
> On Wed, Mar 22, 2023 at 08:11:37AM +0100, Thomas Huth wrote:
>> On 21/03/2023 17.16, Cédric Le Goater wrote:
>>> From: Cédric Le Goater <clg@redhat.com>
>>>
>>> GCC13 reports an error :
>>>
>>> ../util/async.c: In function ‘aio_bh_poll’:
>>> include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=]
>>> 303 | (head)->sqh_last = &(elm)->field.sqe_next; \
>>> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
>>> ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’
>>> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
>>> | ^~~~~~~~~~~~~~~~~~~~
>>> ../util/async.c:161:17: note: ‘slice’ declared here
>>> 161 | BHListSlice slice;
>>> | ^~~~~
>>> ../util/async.c:161:17: note: ‘ctx’ declared here
>>>
>>> But the local variable 'slice' is removed from the global context list
>>> in following loop of the same routine. Add a pragma to silent GCC.
>>>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>> util/async.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/util/async.c b/util/async.c
>>> index 21016a1ac7..de9b431236 100644
>>> --- a/util/async.c
>>> +++ b/util/async.c
>>> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
>>> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
>>> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
>>> +
>>> + /*
>>> + * GCC13 [-Werror=dangling-pointer=] complains that the local variable
>>> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but the
>>> + * list is emptied before this function returns.
>>> + */
>>> +#if !defined(__clang__)
>>> +#pragma GCC diagnostic push
>>> +#pragma GCC diagnostic ignored "-Wdangling-pointer="
>>
>> That warning parameter looks like a new one in GCC 13 ?
>> ... so you have to check whether it's available before disabling
>> it, otherwise this will fail with older versions of GCC. I just
>> gave it a try with my GCC 8.5 and got this:
>>
>> ../../devel/qemu/util/async.c: In function ‘aio_bh_poll’:
>> ../../devel/qemu/util/async.c:175:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
>> #pragma GCC diagnostic ignored "-Wdangling-pointer="
>> ^~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Thomas
>>
>> What about reworking the code like this:
>>
>> diff --git a/util/async.c b/util/async.c
>> index 21016a1ac7..b236bdfbd8 100644
>> --- a/util/async.c
>> +++ b/util/async.c
>> @@ -156,15 +156,14 @@ void aio_bh_call(QEMUBH *bh)
>> }
>> /* Multiple occurrences of aio_bh_poll cannot be called concurrently. */
>> -int aio_bh_poll(AioContext *ctx)
>> +static int aio_bh_poll_slice(AioContext *ctx, BHListSlice *slice)
>> {
>> - BHListSlice slice;
>> BHListSlice *s;
>> int ret = 0;
>> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */
>> - QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
>> - QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
>> + QSLIST_MOVE_ATOMIC(&slice->bh_list, &ctx->bh_list);
>> + QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, slice, next);
>> while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
>> QEMUBH *bh;
>> @@ -191,6 +190,13 @@ int aio_bh_poll(AioContext *ctx)
>> return ret;
>> }
>> +int aio_bh_poll(AioContext *ctx)
>> +{
>> + BHListSlice slice;
>> +
>> + return aio_bh_poll_slice(ctx, &slice);
>> +}
>> +
>> void qemu_bh_schedule_idle(QEMUBH *bh)
>> {
>> aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
>>
>> Would that work with GCC 13 and be acceptable?
>
> Fine by me. Please add a comment into aio_bh_poll() explaining that this
> wrapper function exists to silence the gcc dangling-pointer warning.
> Otherwise someone may be tempted to remove the function.
IMO by using #pragmas it is clearer this is a kludge. Also we can
revert this commit adding the pragmas/comment once the compiler
are fixed.
Hi
On Wed, Mar 22, 2023 at 6:42 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:
> On 22/3/23 14:27, Stefan Hajnoczi wrote:
> > On Wed, Mar 22, 2023 at 08:11:37AM +0100, Thomas Huth wrote:
> >> On 21/03/2023 17.16, Cédric Le Goater wrote:
> >>> From: Cédric Le Goater <clg@redhat.com>
> >>>
> >>> GCC13 reports an error :
> >>>
> >>> ../util/async.c: In function ‘aio_bh_poll’:
> >>> include/qemu/queue.h:303:22: error: storing the address of local
> variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’
> [-Werror=dangling-pointer=]
> >>> 303 | (head)->sqh_last = &(elm)->field.sqe_next;
> \
> >>> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> >>> ../util/async.c:169:5: note: in expansion of macro
> ‘QSIMPLEQ_INSERT_TAIL’
> >>> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> >>> | ^~~~~~~~~~~~~~~~~~~~
> >>> ../util/async.c:161:17: note: ‘slice’ declared here
> >>> 161 | BHListSlice slice;
> >>> | ^~~~~
> >>> ../util/async.c:161:17: note: ‘ctx’ declared here
> >>>
> >>> But the local variable 'slice' is removed from the global context list
> >>> in following loop of the same routine. Add a pragma to silent GCC.
> >>>
> >>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Cc: Daniel P. Berrangé <berrange@redhat.com>
> >>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> >>> ---
> >>> util/async.c | 13 +++++++++++++
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/util/async.c b/util/async.c
> >>> index 21016a1ac7..de9b431236 100644
> >>> --- a/util/async.c
> >>> +++ b/util/async.c
> >>> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx)
> >>> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in
> aio_bh_enqueue(). */
> >>> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> >>> +
> >>> + /*
> >>> + * GCC13 [-Werror=dangling-pointer=] complains that the local
> variable
> >>> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but
> the
> >>> + * list is emptied before this function returns.
> >>> + */
> >>> +#if !defined(__clang__)
> >>> +#pragma GCC diagnostic push
> >>> +#pragma GCC diagnostic ignored "-Wdangling-pointer="
> >>
> >> That warning parameter looks like a new one in GCC 13 ?
> >> ... so you have to check whether it's available before disabling
> >> it, otherwise this will fail with older versions of GCC. I just
> >> gave it a try with my GCC 8.5 and got this:
> >>
> >> ../../devel/qemu/util/async.c: In function ‘aio_bh_poll’:
> >> ../../devel/qemu/util/async.c:175:32: error: unknown option after
> ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
> >> #pragma GCC diagnostic ignored "-Wdangling-pointer="
> >> ^~~~~~~~~~~~~~~~~~~~~
> >> cc1: all warnings being treated as errors
> >>
> >> Thomas
> >>
> >> What about reworking the code like this:
> >>
> >> diff --git a/util/async.c b/util/async.c
> >> index 21016a1ac7..b236bdfbd8 100644
> >> --- a/util/async.c
> >> +++ b/util/async.c
> >> @@ -156,15 +156,14 @@ void aio_bh_call(QEMUBH *bh)
> >> }
> >> /* Multiple occurrences of aio_bh_poll cannot be called concurrently.
> */
> >> -int aio_bh_poll(AioContext *ctx)
> >> +static int aio_bh_poll_slice(AioContext *ctx, BHListSlice *slice)
> >> {
> >> - BHListSlice slice;
> >> BHListSlice *s;
> >> int ret = 0;
> >> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in
> aio_bh_enqueue(). */
> >> - QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list);
> >> - QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next);
> >> + QSLIST_MOVE_ATOMIC(&slice->bh_list, &ctx->bh_list);
> >> + QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, slice, next);
> >> while ((s = QSIMPLEQ_FIRST(&ctx->bh_slice_list))) {
> >> QEMUBH *bh;
> >> @@ -191,6 +190,13 @@ int aio_bh_poll(AioContext *ctx)
> >> return ret;
> >> }
> >> +int aio_bh_poll(AioContext *ctx)
> >> +{
> >> + BHListSlice slice;
> >> +
> >> + return aio_bh_poll_slice(ctx, &slice);
> >> +}
> >> +
> >> void qemu_bh_schedule_idle(QEMUBH *bh)
> >> {
> >> aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE);
> >>
> >> Would that work with GCC 13 and be acceptable?
> >
> > Fine by me. Please add a comment into aio_bh_poll() explaining that this
> > wrapper function exists to silence the gcc dangling-pointer warning.
> > Otherwise someone may be tempted to remove the function.
>
> IMO by using #pragmas it is clearer this is a kludge. Also we can
> revert this commit adding the pragmas/comment once the compiler
> are fixed.
>
>
up
fwiw,bBoth solutions look fine to me, as long as there is a comment
explaining it.
--
Marc-André Lureau
On 22/3/23 08:11, Thomas Huth wrote: > On 21/03/2023 17.16, Cédric Le Goater wrote: >> From: Cédric Le Goater <clg@redhat.com> >> >> GCC13 reports an error : >> >> ../util/async.c: In function ‘aio_bh_poll’: >> include/qemu/queue.h:303:22: error: storing the address of local >> variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ >> [-Werror=dangling-pointer=] >> 303 | (head)->sqh_last = >> &(elm)->field.sqe_next; \ >> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ >> ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’ >> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next); >> | ^~~~~~~~~~~~~~~~~~~~ >> ../util/async.c:161:17: note: ‘slice’ declared here >> 161 | BHListSlice slice; >> | ^~~~~ >> ../util/async.c:161:17: note: ‘ctx’ declared here >> >> But the local variable 'slice' is removed from the global context list >> in following loop of the same routine. Add a pragma to silent GCC. >> >> Cc: Stefan Hajnoczi <stefanha@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Daniel P. Berrangé <berrange@redhat.com> >> Signed-off-by: Cédric Le Goater <clg@redhat.com> >> --- >> util/async.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/util/async.c b/util/async.c >> index 21016a1ac7..de9b431236 100644 >> --- a/util/async.c >> +++ b/util/async.c >> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx) >> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in >> aio_bh_enqueue(). */ >> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list); >> + >> + /* >> + * GCC13 [-Werror=dangling-pointer=] complains that the local >> variable >> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but >> the >> + * list is emptied before this function returns. >> + */ >> +#if !defined(__clang__) >> +#pragma GCC diagnostic push >> +#pragma GCC diagnostic ignored "-Wdangling-pointer=" > > That warning parameter looks like a new one in GCC 13 ? > ... so you have to check whether it's available before disabling > it, otherwise this will fail with older versions of GCC. I just > gave it a try with my GCC 8.5 and got this: > > ../../devel/qemu/util/async.c: In function ‘aio_bh_poll’: > ../../devel/qemu/util/async.c:175:32: error: unknown option after > ‘#pragma GCC diagnostic’ kind [-Werror=pragmas] > #pragma GCC diagnostic ignored "-Wdangling-pointer=" > ^~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors Just curious, does this work? (I don't have a GCC 8.5 handy) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wpragmas" #pragma GCC diagnostic ignored "-Wdangling-pointer="
On 3/22/23 09:47, Philippe Mathieu-Daudé wrote: > On 22/3/23 08:11, Thomas Huth wrote: >> On 21/03/2023 17.16, Cédric Le Goater wrote: >>> From: Cédric Le Goater <clg@redhat.com> >>> >>> GCC13 reports an error : >>> >>> ../util/async.c: In function ‘aio_bh_poll’: >>> include/qemu/queue.h:303:22: error: storing the address of local variable ‘slice’ in ‘*ctx.bh_slice_list.sqh_last’ [-Werror=dangling-pointer=] >>> 303 | (head)->sqh_last = &(elm)->field.sqe_next; \ >>> | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ >>> ../util/async.c:169:5: note: in expansion of macro ‘QSIMPLEQ_INSERT_TAIL’ >>> 169 | QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next); >>> | ^~~~~~~~~~~~~~~~~~~~ >>> ../util/async.c:161:17: note: ‘slice’ declared here >>> 161 | BHListSlice slice; >>> | ^~~~~ >>> ../util/async.c:161:17: note: ‘ctx’ declared here >>> >>> But the local variable 'slice' is removed from the global context list >>> in following loop of the same routine. Add a pragma to silent GCC. >>> >>> Cc: Stefan Hajnoczi <stefanha@redhat.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Daniel P. Berrangé <berrange@redhat.com> >>> Signed-off-by: Cédric Le Goater <clg@redhat.com> >>> --- >>> util/async.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/util/async.c b/util/async.c >>> index 21016a1ac7..de9b431236 100644 >>> --- a/util/async.c >>> +++ b/util/async.c >>> @@ -164,7 +164,20 @@ int aio_bh_poll(AioContext *ctx) >>> /* Synchronizes with QSLIST_INSERT_HEAD_ATOMIC in aio_bh_enqueue(). */ >>> QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list); >>> + >>> + /* >>> + * GCC13 [-Werror=dangling-pointer=] complains that the local variable >>> + * 'slice' is being stored in the global 'ctx->bh_slice_list' but the >>> + * list is emptied before this function returns. >>> + */ >>> +#if !defined(__clang__) >>> +#pragma GCC diagnostic push >>> +#pragma GCC diagnostic ignored "-Wdangling-pointer=" >> >> That warning parameter looks like a new one in GCC 13 ? >> ... so you have to check whether it's available before disabling >> it, otherwise this will fail with older versions of GCC. I just >> gave it a try with my GCC 8.5 and got this: >> >> ../../devel/qemu/util/async.c: In function ‘aio_bh_poll’: >> ../../devel/qemu/util/async.c:175:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas] >> #pragma GCC diagnostic ignored "-Wdangling-pointer=" >> ^~~~~~~~~~~~~~~~~~~~~ >> cc1: all warnings being treated as errors > > Just curious, does this work? (I don't have a GCC 8.5 handy) > > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wpragmas" > #pragma GCC diagnostic ignored "-Wdangling-pointer=" Yep. That works too. I like Thomas's proposal too. It only lacks a comment on the compile issue. Let's see what others have to say about it. Thanks, C.
© 2016 - 2026 Red Hat, Inc.