On 28/07/2023 17.13, Peter Maydell wrote:
On Fri, 28 Jul 2023 at 15:28, Thomas Huth <th...@redhat.com> wrote:
Clang on Windows does not seem to know the "gcc_struct" attribute
and emits a warning when we try to use it. Add an additional check
here with __has_attribute() to avoid this problem.
Signed-off-by: Thomas Huth <th...@redhat.com>
---
include/qemu/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index a309f90c76..5065b4447c 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -22,7 +22,7 @@
#define QEMU_EXTERN_C extern
#endif
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
+#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) &&
!defined(__clang__)
# define QEMU_PACKED __attribute__((gcc_struct, packed))
#else
# define QEMU_PACKED __attribute__((packed))
I'm not sure about this. The idea of QEMU_PACKED is that
it's supposed to give you the same struct layout
regardless of compiler. With this change it no longer
does that, and there's no compile-time guard against
using something in a packed struct that has a different
layout on Windows clang vs everything else.
If it was OK to use plain attribute(packed) we wouldn't
need the ifdef at all because we could use it on GCC too.
I still haven't quite grasped whether this attribute just affects structures
with bitfields in it, or whether it could also affect other structures
without bitfields.
Looking at https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html and
https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html#index-mms-bitfields , it
sounds like it only affects structs with bitfields, unless you specify an
"aligned(x)" attribute, too?
Anyway, using bitfields in structs for exchanging data with the guest is
just way too error-prone, as you can see in the discussion about that
VTD_IR_TableEntry in my other patch. We should maybe advise against
bitfields in our coding style and point people to registerfields.h instead
for new code? ... so that we use QEMU_PACKED mainly for legacy code. Would
it then be OK for you, Peter, to go on with this approach?
Or do you see another possibility how we could fix that timeout problem in
the 64-bit MSYS2 job? Still switching to clang, but compiling with
--extra-cflags="-Wno-unknown-attributes" maybe?
Thomas
