Description of problem: Current qemu doesn't build with clang on f28, it used to on f27, and I _think_ this is a clang change; I've got a smaller reproducer as well. The error is: hw/display/qxl.c:1883: undefined reference to `__atomic_fetch_or_4' that line (which hasn't changed for ~5 years) is: old_pending = atomic_fetch_or(&d->ram->int_pending, le_events); some macros turn that atomic_fetch_or into __atomic_fetch_or and the 'int_pending' is a uint32_t in a QXLRam struct, and the QXLRam struct is marked packed. Version-Release number of selected component (if applicable): clang-libs-6.0.0-5.fc28.x86_64 clang-tools-extra-6.0.0-5.fc28.x86_64 clang-analyzer-6.0.0-5.fc28.noarch clang-6.0.0-5.fc28.x86_64 clang-devel-6.0.0-5.fc28.x86_64 How reproducible: 100% Steps to Reproduce: 1. Grab QEMU source 2. configure with configure --target-list=x86_64-softmmu --cc=clang 3. make -j anumber Actual results: hw/display/qxl.c:1883: undefined reference to `__atomic_fetch_or_4' Expected results: A happy linked qemu; either with a call to an __atomic_fetch_or_4 or a suitable instruction Additional info: a simpler test case ---- #include <stdio.h> #include <stddef.h> #include <stdint.h> struct __attribute__ ((__packed__)) baz { uint32_t x3; }; struct foo { uint32_t x; uint32_t x2; struct baz *b; } fx; static inline uint32_t cpu_to_le32(uint32_t v){ return (v);} uint32_t foor(struct foo *p, uint32_t y) { uint32_t ly = cpu_to_le32(y); return __atomic_fetch_or(&p->b->x3, y, 5); } uint32_t bah(uint32_t *p) { return foor(&fx, 8); } int main() { } ---- clang -O2 t.c /tmp/t-78af46.o: In function `foor': t.c:(.text+0xa): undefined reference to `__atomic_fetch_or_4' /tmp/t-78af46.o: In function `bah': t.c:(.text+0x22): undefined reference to `__atomic_fetch_or_4' clang-6.0: error: linker command failed with exit code 1 (use -v to see invocation)
Hmm this might not be a clang bug after all, at least not on all architectures. If the struct is packed, some architecture may need to use libatomic (which takes a spinlock around atomic access). This is not the case for x86_64, but QEMU probably should not use atomics on packed members.
yeh, linking my test with -latomic makes it link (although that's not necessary with gcc), and I thought x86 could do atomic's on any crazy alignment? (I think in reality the data is actually aligned in this case)
This 'regression' in clang was introduced by this commit: https://reviews.llvm.org/rL314145 see also llvm.org/PR34347.
> I thought x86 could do atomic's on any crazy alignment? Yes, that's why I said on x86_64 it *is* a clang bug. But QEMU probably wants to fail compilation on all architectures when you do this; we already do this for atomics above pointer size, even though x86_32 supports them. BTW the problematic struct here is in spice, so that's where we'd have to fix it. https://pastebin.com/raw/qPRTjDBk is a tentative patch based on this working version of David's testcase: #include <stdio.h> #include <stddef.h> #include <stdint.h> typedef struct __attribute__ ((__packed__)) baz { uint32_t x3; } baz_packed; typedef __attribute__ ((__aligned__(4096))) baz_packed baz; struct foo { uint32_t x; uint32_t x2; baz *b; } fx; static inline uint32_t cpu_to_le32(uint32_t v){ return (v);} uint32_t foor(struct foo *p, uint32_t y) { uint32_t ly = cpu_to_le32(y); return __atomic_fetch_or(&p->b->x3, y, 5); } uint32_t bah(uint32_t *p) { return foor(&fx, 8); } int main() { }
Yes, the QXLRam is always aligned to a page. Note however that alignment affects also the sizeof operator which would potentially break some ABI if we expect data after a QXLRam (like monitor configuration if I remember correctly). Looking at the structure is safe to assume this structure is 8 byte aligned. Note also that for similar reasons also the ring structures define in this header should be 4-byte aligned. Is not hard to define a/some macro(s) to have packed and aligned attributes in gcc/clang, however this header is also used in some Visual Studio project and in VS packed attribute behave usually slightly differently.
Proposed a patch at https://lists.freedesktop.org/archives/spice-devel/2018-June/044384.html. I suppose would need to be backported if accepted.
spice-protocol-0.12.14-2.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-fdadcf6c59
spice-protocol-0.12.14-2.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-fdadcf6c59
spice-protocol-0.12.14-2.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.