Re: [RFC PATCH] configure: Fix atomic64 test for --enable-werror on macOS

[Christian Schoenebeck]

On Donnerstag, 16. Juli 2020 16:25:18 CEST Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 03:11:01PM +0200, Thomas Huth wrote:
> > When using --enable-werror for the macOS builders in the Cirrus-CI,
> > the atomic64 test is currently failing, and config.log shows a bunch
> > 
> > of error messages like this:
> >  config-temp/qemu-conf.c:6:7: error: implicit declaration of function
> >  '__atomic_load_8' is invalid in C99
> >  [-Werror,-Wimplicit-function-declaration]>  
> >   y = __atomic_load_8(&x, 0);
> >   
> >       ^
> >  
> >  config-temp/qemu-conf.c:6:7: error: this function declaration is not a
> >  prototype [-Werror,-Wstrict-prototypes]
> > 
> > Suppress the warnings to make it pass.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > 
> >  Not sure whether this is the best way to fix this issue ... thus marked
> >  as RFC.
> >  Even though the compiler warns here, the program links apparently just
> >  fine afterwards and CONFIG_ATOMIC64=y gets set in the config-host.mak
> >  file on macOS, so the 64-bit atomic operations seem to be available...
> >  Any macOS users here who could shed some light on this?
> 
> The error message refers to c99, but QEMU code standard is gnu99.
> 
> It doesn't look like we set std=gnu99 when running configure
> tests though, and I wonder if that is relevant in this case,
> given that the atomic_load* stuff is all compiler built-in.
> eg does  -std=gnu99  have any impact on the warnings ?

I already tried that. It makes no difference for me with clang on macOS 
10.15.5. I also tried higher C standards with & without gnu extensions, same 
thing.

I also tried raising the minimum deployment target, as I saw some macOS system 
libs are hiding these __atomic_*_8() calls depending on the macOS version, did 
not help either though.

Like I mentioned in my other email, I don't see __atomic_*_8() being declared 
in any public header on Mac, and keep in mind if you don't have a proper 
function prototype declaration somewhere (i.e. if you just use the '-Wno-
implicit-function-declaration -Wno-strict-prototypes' hammer), then those 
functions' arguments and return values would *all* simply, silently default to 
type 'int' with C -> potential data truncation and/or ending up in wrong 
registers on ABI level, etc.

So __atomic_*_8() -> __atomic_*() (and including <stdatomic.h>) is probably 
the best choice. Even though __atomic_*() uses a different data type, but its 
just a couple lines changes in this case fortunately, as the actual qemu code 
base is not affected.

Best regards,
Christian Schoenebeck