[PATCH v2 0/5] Vs clang-10 and gcc-9 warnings

[PATCH v2 0/5] Vs clang-10 and gcc-9 warnings

[Richard Henderson]

Three of these patches are for cleaning up warnings vs clang-10.

The -Wtautological-type-limit-compare patch has been improved
as suggested by Eric Blake.

The final patch is for a "new" warning from gcc-9 on aarch64 hosts.
Our build box has been upgraded from bionic, so the warning is new
to me, anyway.


r~


Philippe Mathieu-Daudé (1):
  fpu/softfloat: Silence 'bitwise negation of boolean expression'
    warning

Richard Henderson (3):
  configure: Clean up warning flag lists
  configure: Disable -Wtautological-type-limit-compare
  configure: Add -Wno-psabi

Wei Wang (1):
  migration: fix xbzrle encoding rate calculation

 configure       | 42 ++++++++++++++++++++++++++++++++----------
 fpu/softfloat.c | 33 ++++++++++++++++++++++++---------
 migration/ram.c |  4 +---
 3 files changed, 57 insertions(+), 22 deletions(-)

-- 
2.25.1

[PATCH v2 1/5] fpu/softfloat: Silence 'bitwise negation of boolean expression' warning

[Richard Henderson]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

When building with clang version 10.0.0-4ubuntu1, we get:

    CC      lm32-softmmu/fpu/softfloat.o
  fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
      absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
          absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  ...

  fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
          zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fix by rewriting the fishy bitwise AND of two bools as an int.

Suggested-by: Eric Blake <eblake@redhat.com>
Buglink: https://bugs.launchpad.net/bugs/1881004
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200528155420.9802-1-philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6c8f2d597a..5e9746c287 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -3362,7 +3362,9 @@ static int32_t roundAndPackInt32(bool zSign, uint64_t absZ,
     }
     roundBits = absZ & 0x7F;
     absZ = ( absZ + roundIncrement )>>7;
-    absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+    if (!(roundBits ^ 0x40) && roundNearestEven) {
+        absZ &= ~1;
+    }
     z = absZ;
     if ( zSign ) z = - z;
     if ( ( absZ>>32 ) || ( z && ( ( z < 0 ) ^ zSign ) ) ) {
@@ -3420,7 +3422,9 @@ static int64_t roundAndPackInt64(bool zSign, uint64_t absZ0, uint64_t absZ1,
     if ( increment ) {
         ++absZ0;
         if ( absZ0 == 0 ) goto overflow;
-        absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
+        if (!(absZ1 << 1) && roundNearestEven) {
+            absZ0 &= ~1;
+        }
     }
     z = absZ0;
     if ( zSign ) z = - z;
@@ -3480,7 +3484,9 @@ static int64_t roundAndPackUint64(bool zSign, uint64_t absZ0,
             float_raise(float_flag_invalid, status);
             return UINT64_MAX;
         }
-        absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
+        if (!(absZ1 << 1) && roundNearestEven) {
+            absZ0 &= ~1;
+        }
     }
 
     if (zSign && absZ0) {
@@ -3603,7 +3609,9 @@ static float32 roundAndPackFloat32(bool zSign, int zExp, uint32_t zSig,
         status->float_exception_flags |= float_flag_inexact;
     }
     zSig = ( zSig + roundIncrement )>>7;
-    zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
+    if (!(roundBits ^ 0x40) && roundNearestEven) {
+        zSig &= ~1;
+    }
     if ( zSig == 0 ) zExp = 0;
     return packFloat32( zSign, zExp, zSig );
 
@@ -3757,7 +3765,9 @@ static float64 roundAndPackFloat64(bool zSign, int zExp, uint64_t zSig,
         status->float_exception_flags |= float_flag_inexact;
     }
     zSig = ( zSig + roundIncrement )>>10;
-    zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
+    if (!(roundBits ^ 0x200) && roundNearestEven) {
+        zSig &= ~1;
+    }
     if ( zSig == 0 ) zExp = 0;
     return packFloat64( zSign, zExp, zSig );
 
@@ -3983,8 +3993,9 @@ floatx80 roundAndPackFloatx80(int8_t roundingPrecision, bool zSign,
             }
             if ( increment ) {
                 ++zSig0;
-                zSig0 &=
-                    ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
+                if (!(zSig1 << 1) && roundNearestEven) {
+                    zSig0 &= ~1;
+                }
                 if ( (int64_t) zSig0 < 0 ) zExp = 1;
             }
             return packFloatx80( zSign, zExp, zSig0 );
@@ -4000,7 +4011,9 @@ floatx80 roundAndPackFloatx80(int8_t roundingPrecision, bool zSign,
             zSig0 = UINT64_C(0x8000000000000000);
         }
         else {
-            zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
+            if (!(zSig1 << 1) && roundNearestEven) {
+                zSig0 &= ~1;
+            }
         }
     }
     else {
@@ -4270,7 +4283,9 @@ static float128 roundAndPackFloat128(bool zSign, int32_t zExp,
     }
     if ( increment ) {
         add128( zSig0, zSig1, 0, 1, &zSig0, &zSig1 );
-        zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
+        if ((zSig2 + zSig2 == 0) && roundNearestEven) {
+            zSig1 &= ~1;
+        }
     }
     else {
         if ( ( zSig0 | zSig1 ) == 0 ) zExp = 0;
-- 
2.25.1

[PATCH v2 2/5] migration: fix xbzrle encoding rate calculation

[Richard Henderson]

From: Wei Wang <wei.w.wang@intel.com>

It's reported an error of implicit conversion from "unsigned long" to
"double" when compiling with Clang 10. Simply make the encoding rate 0
when the encoded_size is 0.

Fixes: e460a4b1a4
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 migration/ram.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 41cc530d9d..069b6e30bc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -913,10 +913,8 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
         unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
                          TARGET_PAGE_SIZE;
         encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
-        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+        if (xbzrle_counters.pages == rs->xbzrle_pages_prev || !encoded_size) {
             xbzrle_counters.encoding_rate = 0;
-        } else if (!encoded_size) {
-            xbzrle_counters.encoding_rate = UINT64_MAX;
         } else {
             xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
         }
-- 
2.25.1

[PATCH v2 3/5] configure: Clean up warning flag lists

[Richard Henderson]

Use a helper function to tidy the assembly of gcc_flags.
Separate flags that disable warnings from those that enable,
and sort the disable warnings to the end.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 597e909b53..464202e375 100755
--- a/configure
+++ b/configure
@@ -97,6 +97,11 @@ do_cxx() {
     do_compiler "$cxx" "$@"
 }
 
+# Append $2 to the variable named $1, with space separation
+add_to() {
+    eval $1=\${$1:+\"\$$1 \"}\$2
+}
+
 update_cxxflags() {
     # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those
     # options which some versions of GCC's C++ compiler complain about
@@ -2007,16 +2012,31 @@ if ! compile_prog "" "" ; then
     error_exit "You need at least GCC v4.8 or Clang v3.4 (or XCode Clang v5.1)"
 fi
 
-gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
-gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
-gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
-gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags"
-gcc_flags="-Wno-initializer-overrides -Wexpansion-to-defined $gcc_flags"
-gcc_flags="-Wno-string-plus-int -Wno-typedef-redefinition $gcc_flags"
-# Note that we do not add -Werror to gcc_flags here, because that would
-# enable it for all configure tests. If a configure test failed due
-# to -Werror this would just silently disable some features,
-# so it's too error prone.
+# Accumulate -Wfoo and -Wno-bar separately.
+# We will list all of the enable flags first, and the disable flags second.
+# Note that we do not add -Werror, because that would enable it for all
+# configure tests. If a configure test failed due to -Werror this would
+# just silently disable some features, so it's too error prone.
+
+add_to warn_flags -Wold-style-declaration
+add_to warn_flags -Wold-style-definition
+add_to warn_flags -Wtype-limits
+add_to warn_flags -Wformat-security
+add_to warn_flags -Wformat-y2k
+add_to warn_flags -Winit-self
+add_to warn_flags -Wignored-qualifiers
+add_to warn_flags -Wempty-body
+add_to warn_flags -Wnested-externs
+add_to warn_flags -Wendif-labels
+add_to warn_flags -Wno-initializer-overrides
+add_to warn_flags -Wexpansion-to-defined
+
+add_to nowarn_flags -Wno-missing-include-dirs
+add_to nowarn_flags -Wno-shift-negative-value
+add_to nowarn_flags -Wno-string-plus-int
+add_to nowarn_flags -Wno-typedef-redefinition
+
+gcc_flags="$warn_flags $nowarn_flags"
 
 cc_has_warning_flag() {
     write_c_skeleton;
-- 
2.25.1

Re: [PATCH v2 3/5] configure: Clean up warning flag lists

[Eric Blake]

On 6/10/20 3:39 PM, Richard Henderson wrote:
> Use a helper function to tidy the assembly of gcc_flags.
> Separate flags that disable warnings from those that enable,
> and sort the disable warnings to the end.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   configure | 40 ++++++++++++++++++++++++++++++----------
>   1 file changed, 30 insertions(+), 10 deletions(-)
> 

> +# Accumulate -Wfoo and -Wno-bar separately.
> +# We will list all of the enable flags first, and the disable flags second.
> +# Note that we do not add -Werror, because that would enable it for all
> +# configure tests. If a configure test failed due to -Werror this would
> +# just silently disable some features, so it's too error prone.
> +
> +add_to warn_flags -Wold-style-declaration

Hmm - should we add:
warn_flags= nowarn_flags=
prior to this line, to ensure that something inherited from the 
environment doesn't mess us up.

> +add_to warn_flags -Wold-style-definition
> +add_to warn_flags -Wtype-limits
> +add_to warn_flags -Wformat-security
> +add_to warn_flags -Wformat-y2k
> +add_to warn_flags -Winit-self
> +add_to warn_flags -Wignored-qualifiers
> +add_to warn_flags -Wempty-body
> +add_to warn_flags -Wnested-externs
> +add_to warn_flags -Wendif-labels
> +add_to warn_flags -Wno-initializer-overrides

wrong list

> +add_to warn_flags -Wexpansion-to-defined
> +
> +add_to nowarn_flags -Wno-missing-include-dirs
> +add_to nowarn_flags -Wno-shift-negative-value
> +add_to nowarn_flags -Wno-string-plus-int
> +add_to nowarn_flags -Wno-typedef-redefinition
> +
> +gcc_flags="$warn_flags $nowarn_flags"

Otherwise looks sane.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 3/5] configure: Clean up warning flag lists

[Richard Henderson]

On 6/11/20 7:25 AM, Eric Blake wrote:
>> +add_to warn_flags -Wold-style-declaration
> 
> Hmm - should we add:
> warn_flags= nowarn_flags=
> prior to this line, to ensure that something inherited from the environment
> doesn't mess us up.
> 
>> +add_to warn_flags -Wold-style-definition
>> +add_to warn_flags -Wtype-limits
>> +add_to warn_flags -Wformat-security
>> +add_to warn_flags -Wformat-y2k
>> +add_to warn_flags -Winit-self
>> +add_to warn_flags -Wignored-qualifiers
>> +add_to warn_flags -Wempty-body
>> +add_to warn_flags -Wnested-externs
>> +add_to warn_flags -Wendif-labels
>> +add_to warn_flags -Wno-initializer-overrides
> 
> wrong list

Thanks, fixed both.


r~

[PATCH v2 4/5] configure: Disable -Wtautological-type-limit-compare

[Richard Henderson]

Clang 10 enables this by default with -Wtype-limit.

All of the instances flagged by this Werror so far have been
cases in which we really do want the compiler to optimize away
the test completely.  Disabling the warning will avoid having
to add ifdefs to work around this.

Cc: Eric Blake <eblake@redhat.com>
Fixes: https://bugs.launchpad.net/qemu/+bug/1878628
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v2: Use the new add_to function.
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 464202e375..8b33447048 100755
--- a/configure
+++ b/configure
@@ -2035,6 +2035,7 @@ add_to nowarn_flags -Wno-missing-include-dirs
 add_to nowarn_flags -Wno-shift-negative-value
 add_to nowarn_flags -Wno-string-plus-int
 add_to nowarn_flags -Wno-typedef-redefinition
+add_to nowarn_flags -Wno-tautological-type-limit-compare
 
 gcc_flags="$warn_flags $nowarn_flags"
 
-- 
2.25.1

[PATCH v2 5/5] configure: Add -Wno-psabi

[Richard Henderson]

On aarch64, gcc 9.3 is generating

qemu/exec.c: In function ‘address_space_translate_iommu’:
qemu/exec.c:431:28: note: parameter passing for argument of type \
  ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1

and many other reptitions.  This structure, and the functions
amongst which it is passed, are not part of a QEMU public API.
Therefore we do not care how the compiler passes the argument,
so long as the compiler is self-consistent.

Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
TODO: The only portion of QEMU which does have a public api,
and so must have a stable abi, is "qemu/plugin.h".  We could
test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
I can't seem to make that work -- Alex?
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 8b33447048..76d32e0f7b 100755
--- a/configure
+++ b/configure
@@ -2036,6 +2036,7 @@ add_to nowarn_flags -Wno-shift-negative-value
 add_to nowarn_flags -Wno-string-plus-int
 add_to nowarn_flags -Wno-typedef-redefinition
 add_to nowarn_flags -Wno-tautological-type-limit-compare
+add_to nowarn_flags -Wno-psabi
 
 gcc_flags="$warn_flags $nowarn_flags"
 
-- 
2.25.1

Re: [PATCH v2 5/5] configure: Add -Wno-psabi

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On aarch64, gcc 9.3 is generating
>
> qemu/exec.c: In function ‘address_space_translate_iommu’:
> qemu/exec.c:431:28: note: parameter passing for argument of type \
>   ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1
>
> and many other reptitions.  This structure, and the functions
> amongst which it is passed, are not part of a QEMU public API.
> Therefore we do not care how the compiler passes the argument,
> so long as the compiler is self-consistent.
>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> TODO: The only portion of QEMU which does have a public api,
> and so must have a stable abi, is "qemu/plugin.h".  We could
> test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
> I can't seem to make that work -- Alex?

modified   plugins/Makefile.objs
@@ -5,6 +5,7 @@
 obj-y += loader.o
 obj-y += core.o
 obj-y += api.o
+api.o-cflags := -Wpsabi
 
 # Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
 # when the final binary includes the plugin object.

Seems to work for me.

-- 
Alex Bennée

Re: [PATCH v2 5/5] configure: Add -Wno-psabi

[Richard Henderson]

On 6/11/20 9:44 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On aarch64, gcc 9.3 is generating
>>
>> qemu/exec.c: In function ‘address_space_translate_iommu’:
>> qemu/exec.c:431:28: note: parameter passing for argument of type \
>>   ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1
>>
>> and many other reptitions.  This structure, and the functions
>> amongst which it is passed, are not part of a QEMU public API.
>> Therefore we do not care how the compiler passes the argument,
>> so long as the compiler is self-consistent.
>>
>> Cc: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> TODO: The only portion of QEMU which does have a public api,
>> and so must have a stable abi, is "qemu/plugin.h".  We could
>> test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
>> I can't seem to make that work -- Alex?
> 
> modified   plugins/Makefile.objs
> @@ -5,6 +5,7 @@
>  obj-y += loader.o
>  obj-y += core.o
>  obj-y += api.o
> +api.o-cflags := -Wpsabi
>  
>  # Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
>  # when the final binary includes the plugin object.
> 
> Seems to work for me.

Wrong directory -- that's the part that goes into qemu, which also uses other
qemu internal headers.  As opposed to the tests/, which only use the one
"qemu/plugins.h" header (plus libc).


r~

Re: [PATCH v2 5/5] configure: Add -Wno-psabi

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 6/11/20 9:44 AM, Alex Bennée wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> On aarch64, gcc 9.3 is generating
>>>
>>> qemu/exec.c: In function ‘address_space_translate_iommu’:
>>> qemu/exec.c:431:28: note: parameter passing for argument of type \
>>>   ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1
>>>
>>> and many other reptitions.  This structure, and the functions
>>> amongst which it is passed, are not part of a QEMU public API.
>>> Therefore we do not care how the compiler passes the argument,
>>> so long as the compiler is self-consistent.
>>>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> TODO: The only portion of QEMU which does have a public api,
>>> and so must have a stable abi, is "qemu/plugin.h".  We could
>>> test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
>>> I can't seem to make that work -- Alex?
>> 
>> modified   plugins/Makefile.objs
>> @@ -5,6 +5,7 @@
>>  obj-y += loader.o
>>  obj-y += core.o
>>  obj-y += api.o
>> +api.o-cflags := -Wpsabi
>>  
>>  # Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
>>  # when the final binary includes the plugin object.
>> 
>> Seems to work for me.
>
> Wrong directory -- that's the part that goes into qemu, which also uses other
> qemu internal headers.  As opposed to the tests/, which only use the one
> "qemu/plugins.h" header (plus libc).

It's a sub-make so I just did:

modified   tests/plugin/Makefile
@@ -18,7 +18,7 @@ NAMES += hwprofile
 
 SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
 
-QEMU_CFLAGS += -fPIC
+QEMU_CFLAGS += -fPIC -Wpsabi
 QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu
 
-- 
Alex Bennée

Re: [PATCH v2 5/5] configure: Add -Wno-psabi

[Richard Henderson]

On 6/11/20 10:17 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 6/11/20 9:44 AM, Alex Bennée wrote:
>>>
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> On aarch64, gcc 9.3 is generating
>>>>
>>>> qemu/exec.c: In function ‘address_space_translate_iommu’:
>>>> qemu/exec.c:431:28: note: parameter passing for argument of type \
>>>>   ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1
>>>>
>>>> and many other reptitions.  This structure, and the functions
>>>> amongst which it is passed, are not part of a QEMU public API.
>>>> Therefore we do not care how the compiler passes the argument,
>>>> so long as the compiler is self-consistent.
>>>>
>>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>> TODO: The only portion of QEMU which does have a public api,
>>>> and so must have a stable abi, is "qemu/plugin.h".  We could
>>>> test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
>>>> I can't seem to make that work -- Alex?
>>>
>>> modified   plugins/Makefile.objs
>>> @@ -5,6 +5,7 @@
>>>  obj-y += loader.o
>>>  obj-y += core.o
>>>  obj-y += api.o
>>> +api.o-cflags := -Wpsabi
>>>  
>>>  # Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
>>>  # when the final binary includes the plugin object.
>>>
>>> Seems to work for me.
>>
>> Wrong directory -- that's the part that goes into qemu, which also uses other
>> qemu internal headers.  As opposed to the tests/, which only use the one
>> "qemu/plugins.h" header (plus libc).
> 
> It's a sub-make so I just did:
> 
> modified   tests/plugin/Makefile
> @@ -18,7 +18,7 @@ NAMES += hwprofile
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> -QEMU_CFLAGS += -fPIC
> +QEMU_CFLAGS += -fPIC -Wpsabi
>  QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu

Did you look at the actual flags passed to the actual cc via V=1?
Neither of these flags is arriving.

I sent you mail about this yesterday...


r~

Re: [PATCH v2 5/5] configure: Add -Wno-psabi

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 6/11/20 10:17 AM, Alex Bennée wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> On 6/11/20 9:44 AM, Alex Bennée wrote:
>>>>
>>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>>
>>>>> On aarch64, gcc 9.3 is generating
>>>>>
>>>>> qemu/exec.c: In function ‘address_space_translate_iommu’:
>>>>> qemu/exec.c:431:28: note: parameter passing for argument of type \
>>>>>   ‘MemTxAttrs’ {aka ‘struct MemTxAttrs’} changed in GCC 9.1
>>>>>
>>>>> and many other reptitions.  This structure, and the functions
>>>>> amongst which it is passed, are not part of a QEMU public API.
>>>>> Therefore we do not care how the compiler passes the argument,
>>>>> so long as the compiler is self-consistent.
>>>>>
>>>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> ---
>>>>> TODO: The only portion of QEMU which does have a public api,
>>>>> and so must have a stable abi, is "qemu/plugin.h".  We could
>>>>> test this by forcing -Wpsabi or -Werror=psabi in tests/plugin.
>>>>> I can't seem to make that work -- Alex?
>>>>
>>>> modified   plugins/Makefile.objs
>>>> @@ -5,6 +5,7 @@
>>>>  obj-y += loader.o
>>>>  obj-y += core.o
>>>>  obj-y += api.o
>>>> +api.o-cflags := -Wpsabi
>>>>  
>>>>  # Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
>>>>  # when the final binary includes the plugin object.
>>>>
>>>> Seems to work for me.
>>>
>>> Wrong directory -- that's the part that goes into qemu, which also uses other
>>> qemu internal headers.  As opposed to the tests/, which only use the one
>>> "qemu/plugins.h" header (plus libc).
>> 
>> It's a sub-make so I just did:
>> 
>> modified   tests/plugin/Makefile
>> @@ -18,7 +18,7 @@ NAMES += hwprofile
>>  
>>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>>  
>> -QEMU_CFLAGS += -fPIC
>> +QEMU_CFLAGS += -fPIC -Wpsabi
>>  QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu
>
> Did you look at the actual flags passed to the actual cc via V=1?
> Neither of these flags is arriving.

I did:

cc -iquote /home/alex/lsrc/qemu.git/builds/all.plugin/. -iquote . -iquote /home/alex/lsrc/qemu.git/tcg/i386 -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem /home/alex/lsrc/qemu.git/builds/all.plugin/linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/accel/tcg -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/disas/libvixl -I/usr/include/pixman-1 -I/home/alex/lsrc/qemu.git/dtc/libfdt -Werror  -pthread -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Og -ggdb3 -fvar-tracking-assignments -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong  -I/usr/include/p11-kit-1  -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/capstone  -fPIC -Wpsabi -I/home/alex/lsrc/qemu.git/include/qemu -MMD -MP -MT hwprofile.o -MF ./hwprofile.d -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g   -c -o hwprofile.o /home/alex/lsrc/qemu.git/tests/plugin/hwprofile.c

It's nested between the -I's with -fPIC.

>
> I sent you mail about this yesterday...
>
>
> r~


-- 
Alex Bennée