[PATCH for-8.0 v2 0/3] Fixes for GCC13

[PATCH for-8.0 v2 0/3] Fixes for GCC13

[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

[PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Cédric Le Goater]

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

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Thomas Huth]

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

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Philippe Mathieu-Daudé]

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="

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Cédric Le Goater]

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.

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Stefan Hajnoczi]

[-- 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 --]

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Philippe Mathieu-Daudé]

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.

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Marc-André Lureau]

[-- 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 --]

Re: [PATCH for-8.0 v2 1/3] async: Suppress GCC13 false positive in aio_bh_poll()

[Daniel Henrique Barboza]

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;

[PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype

[Cédric Le Goater]

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

Re: [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype

[Richard Henderson]

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~

Re: [PATCH for-8.0 v2 2/3] target/s390x: Fix float_comp_to_cc() prototype

[Thomas Huth]

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>

[PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype

[Cédric Le Goater]

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

Re: [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype

[Richard Henderson]

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~

Re: [PATCH for-8.0 v2 3/3] target/ppc: Fix helper_pminsn() prototype

[Thomas Huth]

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>