| Message ID | 20230321161609.716474-2-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes for GCC13 | expand |
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 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.
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.
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;
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;