Bug 1565766
| Summary: | qemu failing to build with clang; __atomic_fetch_or_4 | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Dr. David Alan Gilbert <dgilbert> |
| Component: | qemu | Assignee: | Fedora Virtualization Maintainers <virt-maint> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 28 | CC: | airlied, amit, berrange, cfergeau, davejohansen, dwmw2, fziglio, itamar, pbonzini, ravnzon, rjones, sbergman, siddharth.kde, tstellar, virt-maint |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2018-07-30 18:25:02 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
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. |
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)