Commit 5a358b39 authored by Peter Maydell's avatar Peter Maydell Committed by Gerd Hoffmann
Browse files

hw/display/qxl: Suppress clang-7 warning about misaligned atomic operation 



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: default avatarPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: default avatarRichard Henderson <richard.henderson@linaro.org>
Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180927155538.699-1-peter.maydell@linaro.org
Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
parent 1242429b

hw/display/qxl.c

+25 −1
Original line number Diff line number Diff line
@@ -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;

    }

include/qemu/compiler.h

+9 −0
Original line number Diff line number Diff line
@@ -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)