* [PATCH for-8.0 v2 0/3] Fixes for GCC13
@ 2023-03-21 16:16 Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-03-21 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Cédric Le Goater
Hello,
I activated a GH workflow using fedora rawhide and found out that
there were a couple of compile breakage with the new GCC. Here are
fixes, the first requiring more attention.
Thanks,
C.
Changes in v2:
- replaced helper routine by pragmas in util/async.c to suppress GCC
warning
Cédric Le Goater (3):
async: Suppress GCC13 false positive in aio_bh_poll()
target/s390x: Fix float_comp_to_cc() prototype
target/ppc: Fix helper_pminsn() prototype
target/s390x/s390x-internal.h | 3 ++-
target/ppc/excp_helper.c | 2 +-
util/async.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 2 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-21 16:16 [PATCH for-8.0 v2 0/3] Fixes for GCC13 Cédric Le Goater
@ 2023-03-21 16:16 ` Cédric Le Goater
2023-03-22 7:11 ` Thomas Huth
2023-04-20 18:43 ` Daniel Henrique Barboza
2023-03-21 16:16 ` [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype Cédric Le Goater
2 siblings, 2 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-03-21 16:16 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini,
Daniel P . Berrangé
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype
2023-03-21 16:16 [PATCH for-8.0 v2 0/3] Fixes for GCC13 Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
@ 2023-03-21 16:16 ` Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:52 ` Thomas Huth
2023-03-21 16:16 ` [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype Cédric Le Goater
2 siblings, 2 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-03-21 16:16 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Thomas Huth, Richard Henderson,
David Hildenbrand, Ilya Leoshkevich, Philippe Mathieu-Daudé
From: Cédric Le Goater <clg@redhat.com>
GCC13 reports an error :
../target/s390x/tcg/fpu_helper.c:123:5: error: conflicting types for ‘float_comp_to_cc’ due to enum/integer mismatch; have ‘int(CPUS390XState *, FloatRelation)’ {aka ‘int(struct CPUArchState *, FloatRelation)’} [-Werror=enum-int-mismatch]
123 | int float_comp_to_cc(CPUS390XState *env, FloatRelation float_compare)
| ^~~~~~~~~~~~~~~~
In file included from ../target/s390x/tcg/fpu_helper.c:23:
../target/s390x/s390x-internal.h:302:5: note: previous declaration of ‘float_comp_to_cc’ with type ‘int(CPUS390XState *, int)’ {aka ‘int(struct CPUArchState *, int)’}
302 | int float_comp_to_cc(CPUS390XState *env, int float_compare);
| ^~~~~~~~~~~~~~~~
Cc: Thomas Huth <thuth@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Fixes: 71bfd65c5f ("softfloat: Name compare relation enum")
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/s390x/s390x-internal.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 5d4361d35b..825252d728 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -11,6 +11,7 @@
#define S390X_INTERNAL_H
#include "cpu.h"
+#include "fpu/softfloat.h"
#ifndef CONFIG_USER_ONLY
typedef struct LowCore {
@@ -299,7 +300,7 @@ uint32_t set_cc_nz_f128(float128 v);
uint8_t s390_softfloat_exc_to_ieee(unsigned int exc);
int s390_swap_bfp_rounding_mode(CPUS390XState *env, int m3);
void s390_restore_bfp_rounding_mode(CPUS390XState *env, int old_mode);
-int float_comp_to_cc(CPUS390XState *env, int float_compare);
+int float_comp_to_cc(CPUS390XState *env, FloatRelation float_compare);
#define DCMASK_ZERO 0x0c00
#define DCMASK_NORMAL 0x0300
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype
2023-03-21 16:16 [PATCH for-8.0 v2 0/3] Fixes for GCC13 Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype Cédric Le Goater
@ 2023-03-21 16:16 ` Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:55 ` Thomas Huth
2 siblings, 2 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-03-21 16:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Cédric Le Goater, Daniel Henrique Barboza
From: Cédric Le Goater <clg@redhat.com>
GCC13 reports an error:
../target/ppc/excp_helper.c:2625:6: error: conflicting types for ‘helper_pminsn’ due to enum/integer mismatch; have ‘void(CPUPPCState *, powerpc_pm_insn_t)’ {aka ‘void(struct CPUArchState *, powerpc_pm_insn_t)’} [-Werror=enum-int-mismatch]
2625 | void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
| ^~~~~~~~~~~~~
In file included from /home/legoater/work/qemu/qemu.git/include/qemu/osdep.h:49,
from ../target/ppc/excp_helper.c:19:
/home/legoater/work/qemu/qemu.git/include/exec/helper-head.h:23:27: note: previous declaration of ‘helper_pminsn’ with type ‘void(CPUArchState *, uint32_t)’ {aka ‘void(CPUArchState *, unsigned int)’}
23 | #define HELPER(name) glue(helper_, name)
| ^~~~~~~
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
Fixes: 7778a575c7 ("ppc: Add P7/P8 Power Management instructions")
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/excp_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 287659c74d..199328f4b6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2622,7 +2622,7 @@ void helper_scv(CPUPPCState *env, uint32_t lev)
}
}
-void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
+void helper_pminsn(CPUPPCState *env, uint32_t insn)
{
CPUState *cs;
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype
2023-03-21 16:16 ` [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype Cédric Le Goater
@ 2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:52 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2023-03-22 3:54 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Thomas Huth, David Hildenbrand,
Ilya Leoshkevich, Philippe Mathieu-Daudé
On 3/21/23 09:16, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> GCC13 reports an error :
>
> ../target/s390x/tcg/fpu_helper.c:123:5: error: conflicting types for ‘float_comp_to_cc’ due to enum/integer mismatch; have ‘int(CPUS390XState *, FloatRelation)’ {aka ‘int(struct CPUArchState *, FloatRelation)’} [-Werror=enum-int-mismatch]
>
> 123 | int float_comp_to_cc(CPUS390XState *env, FloatRelation float_compare)
> | ^~~~~~~~~~~~~~~~
> In file included from ../target/s390x/tcg/fpu_helper.c:23:
> ../target/s390x/s390x-internal.h:302:5: note: previous declaration of ‘float_comp_to_cc’ with type ‘int(CPUS390XState *, int)’ {aka ‘int(struct CPUArchState *, int)’}
> 302 | int float_comp_to_cc(CPUS390XState *env, int float_compare);
> | ^~~~~~~~~~~~~~~~
>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Fixes: 71bfd65c5f ("softfloat: Name compare relation enum")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype
2023-03-21 16:16 ` [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype Cédric Le Goater
@ 2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:55 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2023-03-22 3:54 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Daniel Henrique Barboza
On 3/21/23 09:16, Cédric Le Goater wrote:
> From: Cédric Le Goater<clg@redhat.com>
>
> GCC13 reports an error:
>
> ../target/ppc/excp_helper.c:2625:6: error: conflicting types for ‘helper_pminsn’ due to enum/integer mismatch; have ‘void(CPUPPCState *, powerpc_pm_insn_t)’ {aka ‘void(struct CPUArchState *, powerpc_pm_insn_t)’} [-Werror=enum-int-mismatch]
> 2625 | void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
> | ^~~~~~~~~~~~~
> In file included from /home/legoater/work/qemu/qemu.git/include/qemu/osdep.h:49,
> from ../target/ppc/excp_helper.c:19:
> /home/legoater/work/qemu/qemu.git/include/exec/helper-head.h:23:27: note: previous declaration of ‘helper_pminsn’ with type ‘void(CPUArchState *, uint32_t)’ {aka ‘void(CPUArchState *, unsigned int)’}
> 23 | #define HELPER(name) glue(helper_, name)
> | ^~~~~~~
>
> Cc: Daniel Henrique Barboza<danielhb413@gmail.com>
> Fixes: 7778a575c7 ("ppc: Add P7/P8 Power Management instructions")
> Signed-off-by: Cédric Le Goater<clg@redhat.com>
> Reviewed-by: Daniel Henrique Barboza<danielhb413@gmail.com>
> ---
> target/ppc/excp_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype
2023-03-21 16:16 ` [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
@ 2023-03-22 6:52 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2023-03-22 6:52 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, Philippe Mathieu-Daudé,
QEMU Trivial
On 21/03/2023 17.16, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> GCC13 reports an error :
>
> ../target/s390x/tcg/fpu_helper.c:123:5: error: conflicting types for ‘float_comp_to_cc’ due to enum/integer mismatch; have ‘int(CPUS390XState *, FloatRelation)’ {aka ‘int(struct CPUArchState *, FloatRelation)’} [-Werror=enum-int-mismatch]
>
> 123 | int float_comp_to_cc(CPUS390XState *env, FloatRelation float_compare)
> | ^~~~~~~~~~~~~~~~
> In file included from ../target/s390x/tcg/fpu_helper.c:23:
> ../target/s390x/s390x-internal.h:302:5: note: previous declaration of ‘float_comp_to_cc’ with type ‘int(CPUS390XState *, int)’ {aka ‘int(struct CPUArchState *, int)’}
> 302 | int float_comp_to_cc(CPUS390XState *env, int float_compare);
> | ^~~~~~~~~~~~~~~~
>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Fixes: 71bfd65c5f ("softfloat: Name compare relation enum")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> target/s390x/s390x-internal.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index 5d4361d35b..825252d728 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -11,6 +11,7 @@
> #define S390X_INTERNAL_H
>
> #include "cpu.h"
> +#include "fpu/softfloat.h"
>
> #ifndef CONFIG_USER_ONLY
> typedef struct LowCore {
> @@ -299,7 +300,7 @@ uint32_t set_cc_nz_f128(float128 v);
> uint8_t s390_softfloat_exc_to_ieee(unsigned int exc);
> int s390_swap_bfp_rounding_mode(CPUS390XState *env, int m3);
> void s390_restore_bfp_rounding_mode(CPUS390XState *env, int old_mode);
> -int float_comp_to_cc(CPUS390XState *env, int float_compare);
> +int float_comp_to_cc(CPUS390XState *env, FloatRelation float_compare);
>
> #define DCMASK_ZERO 0x0c00
> #define DCMASK_NORMAL 0x0300
Better to respond to v2:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype
2023-03-21 16:16 ` [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
@ 2023-03-22 6:55 ` Thomas Huth
1 sibling, 0 replies; 15+ messages in thread
From: Thomas Huth @ 2023-03-22 6:55 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Daniel Henrique Barboza, QEMU Trivial
On 21/03/2023 17.16, Cédric Le Goater wrote:
> From: Cédric Le Goater <clg@redhat.com>
>
> GCC13 reports an error:
>
> ../target/ppc/excp_helper.c:2625:6: error: conflicting types for ‘helper_pminsn’ due to enum/integer mismatch; have ‘void(CPUPPCState *, powerpc_pm_insn_t)’ {aka ‘void(struct CPUArchState *, powerpc_pm_insn_t)’} [-Werror=enum-int-mismatch]
> 2625 | void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
> | ^~~~~~~~~~~~~
> In file included from /home/legoater/work/qemu/qemu.git/include/qemu/osdep.h:49,
> from ../target/ppc/excp_helper.c:19:
> /home/legoater/work/qemu/qemu.git/include/exec/helper-head.h:23:27: note: previous declaration of ‘helper_pminsn’ with type ‘void(CPUArchState *, uint32_t)’ {aka ‘void(CPUArchState *, unsigned int)’}
> 23 | #define HELPER(name) glue(helper_, name)
> | ^~~~~~~
>
> Cc: Daniel Henrique Barboza <danielhb413@gmail.com>
> Fixes: 7778a575c7 ("ppc: Add P7/P8 Power Management instructions")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> target/ppc/excp_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 287659c74d..199328f4b6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2622,7 +2622,7 @@ void helper_scv(CPUPPCState *env, uint32_t lev)
> }
> }
>
> -void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
> +void helper_pminsn(CPUPPCState *env, uint32_t insn)
> {
> CPUState *cs;
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
@ 2023-03-22 7:11 ` Thomas Huth
2023-03-22 8:47 ` Philippe Mathieu-Daudé
2023-03-22 13:27 ` Stefan Hajnoczi
2023-04-20 18:43 ` Daniel Henrique Barboza
1 sibling, 2 replies; 15+ messages in thread
From: Thomas Huth @ 2023-03-22 7:11 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini,
Daniel P . Berrangé
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-22 7:11 ` Thomas Huth
@ 2023-03-22 8:47 ` Philippe Mathieu-Daudé
2023-03-22 10:21 ` Cédric Le Goater
2023-03-22 13:27 ` Stefan Hajnoczi
1 sibling, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-22 8:47 UTC (permalink / raw)
To: Thomas Huth, Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini,
Daniel P . Berrangé
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="
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-22 8:47 ` Philippe Mathieu-Daudé
@ 2023-03-22 10:21 ` Cédric Le Goater
0 siblings, 0 replies; 15+ messages in thread
From: Cédric Le Goater @ 2023-03-22 10:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Thomas Huth, Cédric Le Goater, qemu-devel
Cc: Stefan Hajnoczi, Paolo Bonzini, Daniel P . Berrangé
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-22 7:11 ` Thomas Huth
2023-03-22 8:47 ` Philippe Mathieu-Daudé
@ 2023-03-22 13:27 ` Stefan Hajnoczi
2023-03-22 14:42 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2023-03-22 13:27 UTC (permalink / raw)
To: Thomas Huth
Cc: Cédric Le Goater, qemu-devel, Cédric Le Goater,
Paolo Bonzini, Daniel P . Berrangé
[-- Attachment #1: Type: text/plain, Size: 4221 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-22 13:27 ` Stefan Hajnoczi
@ 2023-03-22 14:42 ` Philippe Mathieu-Daudé
2023-04-18 7:31 ` Marc-André Lureau
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-22 14:42 UTC (permalink / raw)
To: Stefan Hajnoczi, Thomas Huth
Cc: Cédric Le Goater, qemu-devel, Cédric Le Goater,
Paolo Bonzini, Daniel P. Berrangé
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-22 14:42 ` Philippe Mathieu-Daudé
@ 2023-04-18 7:31 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2023-04-18 7:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Stefan Hajnoczi, Thomas Huth, Cédric Le Goater, qemu-devel,
Cédric Le Goater, Paolo Bonzini, Daniel P. Berrangé
[-- Attachment #1: Type: text/plain, Size: 4920 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 6878 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
2023-03-22 7:11 ` Thomas Huth
@ 2023-04-20 18:43 ` Daniel Henrique Barboza
1 sibling, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2023-04-20 18:43 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Cédric Le Goater, Stefan Hajnoczi, Paolo Bonzini,
Daniel P . Berrangé
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;
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-04-20 18:43 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-21 16:16 [PATCH for-8.0 v2 0/3] Fixes for GCC13 Cédric Le Goater
2023-03-21 16:16 ` [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll() Cédric Le Goater
2023-03-22 7:11 ` Thomas Huth
2023-03-22 8:47 ` Philippe Mathieu-Daudé
2023-03-22 10:21 ` Cédric Le Goater
2023-03-22 13:27 ` Stefan Hajnoczi
2023-03-22 14:42 ` Philippe Mathieu-Daudé
2023-04-18 7:31 ` Marc-André Lureau
2023-04-20 18:43 ` Daniel Henrique Barboza
2023-03-21 16:16 ` [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:52 ` Thomas Huth
2023-03-21 16:16 ` [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype Cédric Le Goater
2023-03-22 3:54 ` Richard Henderson
2023-03-22 6:55 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.