Re: [PULL 00/35] MIPS patches for 2021-01-03 - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: "Peter Maydell" <peter.maydell@linaro.org>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Emilio G. Cota" <cota@braap.org>
Cc: "Huacai Chen" <chenhuacai@kernel.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Richard Henderson" <rth@twiddle.net>
Subject: Re: [PULL 00/35] MIPS patches for 2021-01-03
Date: Tue, 5 Jan 2021 16:14:31 +0100	[thread overview]
Message-ID: <f8a0d65d-90e7-f036-544c-eb1cd46ff1f4@amsat.org> (raw)
In-Reply-To: <CAFEAcA_S1yAdY7uRYygC4uTxLQpYf22zrcfr9-JkFfhOj1N44g@mail.gmail.com>

On 1/5/21 2:17 PM, Peter Maydell wrote:
> On Tue, 5 Jan 2021 at 09:36, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> 4/ libatomic on 32-bit hosts (i386, riscv32? arm?)
>>
>> While compiling succeed, linking fails:
>>
>> [850/2216] Linking target tests/test-hbitmap
>> FAILED: tests/test-hbitmap
>> clang  -o tests/test-hbitmap tests/test-hbitmap.p/test-hbitmap.c.o
>> tests/test-hbitmap.p/iothread.c.o -Wl,--as-needed -Wl,--no-undefined
>> -pie -Wl,--whole-archive libblock.fa libcrypto.fa libauthz.fa libqom.fa
>> libio.fa -Wl,--no-whole-archive -Wl,--warn-common -fsanitize=undefined
>> -fsanitize=address -Wl,-z,relro -Wl,-z,now -m32 -ggdb
>> -fstack-protector-strong -Wl,--start-group libqemuutil.a
>> subprojects/libvhost-user/libvhost-user-glib.a
>> subprojects/libvhost-user/libvhost-user.a libblock.fa libcrypto.fa
>> libauthz.fa libqom.fa libio.fa @block.syms -lgio-2.0 -lgobject-2.0
>> -lglib-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -pthread -lutil -lgnutls
>> -lm -lgthread-2.0 -lglib-2.0 /usr/lib/i386-linux-gnu/libglib-2.0.so
>> -liscsi -lgthread-2.0 -lglib-2.0 -laio -lcurl
>> /usr/lib/i386-linux-gnu/libz.so -lrbd -lrados -lnettle -lgnutls
>> -Wl,--end-group
>> libblock.fa(block_io.c.o): In function `stat64_max':
>> include/qemu/stats64.h:58: undefined reference to `__atomic_load_8'
>> include/qemu/stats64.h:60: undefined reference to
>> `__atomic_compare_exchange_8'
>> libblock.fa(block_qapi.c.o): In function `stat64_get':
>> include/qemu/stats64.h:40: undefined reference to `__atomic_load_8'
>> libqemuutil.a(util_qsp.c.o): In function `qatomic_set_u64':
>> include/qemu/atomic.h:478: undefined reference to `__atomic_store_8'
>> libqemuutil.a(util_qsp.c.o): In function `qatomic_read_u64':
>> include/qemu/atomic.h:468: undefined reference to `__atomic_load_8'
>> clang: error: linker command failed with exit code 1 (use -v to see
>> invocation)
> 
> Historically we have not linked against libatomic on purpose,
> on the theory that QEMU should not be trying to use atomic
> operations that the compiler cannot directly open-code as
> native atomic instructions. (This is because we want things
> to work even if the code in another thread that might also be
> doing atomic operations on the data is TCG.) libatomic might
> choose to use a mutex under the hood, if my understanding/memory
> is correct, which obviously TCG won't.
> 
> In particular this means that code that can run on 32-bit hosts
> is not supposed to be doing 64-bit atomic operations. For the
> code in stat64_max/stat64_get, this is guarded by CONFIG_ATOMIC64,
> which configure should only be setting if we can do 64-bit atomics
> without libatomic, so looking at whether that got set and if the
> test is doing the wrong thing would be my first suggestion.

That makes sense.

So on a Ubuntu 18.04 i386 host, "configure --cc=clang-10
--enable-sanitizers' sets atomic64=yes (__ATOMIC_RELAXED
is also defined).

The ./configure check is simple. There is a lot of ifdef'ry
to follow in "qemu/osdep.h" and "qemu/compiler.h" so it is
hard to figure out what changes "qemu/atomic.h" that it doesn't
match with ./configure.

Maybe a issue with the sanitizer code?

Cc'ing Stefan, Emilio & Marc-André too :)