diff mbox series

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

Message ID 20200610203942.887374-6-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series Vs clang-10 and gcc-9 warnings | expand

Commit Message

Richard Henderson June 10, 2020, 8:39 p.m. UTC
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(+)

Comments

Alex Bennée June 11, 2020, 4:44 p.m. UTC | #1
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.
Richard Henderson June 11, 2020, 4:57 p.m. UTC | #2
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~
Alex Bennée June 11, 2020, 5:17 p.m. UTC | #3
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
Richard Henderson June 11, 2020, 5:49 p.m. UTC | #4
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~
Alex Bennée June 12, 2020, 6:42 a.m. UTC | #5
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~
diff mbox series

Patch

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"