[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 2/4] hw/display/qxl: Suppress clang-7 warning about m
From: |
Gerd Hoffmann |
Subject: |
[Qemu-devel] [PULL 2/4] hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation |
Date: |
Fri, 5 Oct 2018 13:11:55 +0200 |
From: Peter Maydell <address@hidden>
If QEMU is compiled with clang-7 it results in the warning:
hw/display/qxl.c:1884:19: error: misaligned or large atomic operation
may incur significant performance penalty [-Werror,-Watomic-alignment]
old_pending = atomic_fetch_or(&d->ram->int_pending, le_events);
^
This is because the Spice headers forgot to define the QXLRam struct
with the '__aligned__(4)' attribute. clang 7 and newer will thus
warn that the access here to int_pending might not be 4-aligned
(because the QXLRam object d->ram points at might start at a
misaligned address). In fact we set up d->ram in init_qxl_ram() so
it always starts at a 4K boundary, so we know the atomic access here
is OK.
Newer Spice versions (with Spice commit
beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1) will fix the bug;
for older Spice versions, work around it by telling the compiler
explicitly that the alignment is OK using __builtin_assume_aligned().
Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>
Message-id: address@hidden
Signed-off-by: Gerd Hoffmann <address@hidden>
---
include/qemu/compiler.h | 9 +++++++++
hw/display/qxl.c | 26 +++++++++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 5843812710..bf47e7bee4 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -122,6 +122,15 @@
#ifndef __has_feature
#define __has_feature(x) 0 /* compatibility with non-clang compilers */
#endif
+
+#ifndef __has_builtin
+#define __has_builtin(x) 0 /* compatibility with non-clang compilers */
+#endif
+
+#if __has_builtin(__builtin_assume_aligned) || QEMU_GNUC_PREREQ(4, 7)
+#define HAS_ASSUME_ALIGNED
+#endif
+
/* Implement C11 _Generic via GCC builtins. Example:
*
* QEMU_GENERIC(x, (float, sinf), (long double, sinl), sin) (x)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 747986478f..e628cf1286 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1893,7 +1893,31 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
trace_qxl_send_events_vm_stopped(d->id, events);
return;
}
- old_pending = atomic_fetch_or(&d->ram->int_pending, le_events);
+ /*
+ * Older versions of Spice forgot to define the QXLRam struct
+ * with the '__aligned__(4)' attribute. clang 7 and newer will
+ * thus warn that atomic_fetch_or(&d->ram->int_pending, ...)
+ * might be a misaligned atomic access, and will generate an
+ * out-of-line call for it, which results in a link error since
+ * we don't currently link against libatomic.
+ *
+ * In fact we set up d->ram in init_qxl_ram() so it always starts
+ * at a 4K boundary, so we know that &d->ram->int_pending is
+ * naturally aligned for a uint32_t. Newer Spice versions
+ * (with Spice commit beda5ec7a6848be20c0cac2a9a8ef2a41e8069c1)
+ * will fix the bug directly. To deal with older versions,
+ * we tell the compiler to assume the address really is aligned.
+ * Any compiler which cares about the misalignment will have
+ * __builtin_assume_aligned.
+ */
+#ifdef HAS_ASSUME_ALIGNED
+#define ALIGNED_UINT32_PTR(P) ((uint32_t *)__builtin_assume_aligned(P, 4))
+#else
+#define ALIGNED_UINT32_PTR(P) ((uint32_t *)P)
+#endif
+
+ old_pending = atomic_fetch_or(ALIGNED_UINT32_PTR(&d->ram->int_pending),
+ le_events);
if ((old_pending & le_events) == le_events) {
return;
}
--
2.9.3