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

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

[Thomas Huth]

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?

 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index e93836aaae..4d50a07b00 100755
--- a/configure
+++ b/configure
@@ -5939,7 +5939,8 @@ int main(void)
   return 0;
 }
 EOF
-if compile_prog "" "" ; then
+if compile_prog "-Wno-implicit-function-declaration -Wno-strict-prototypes" "";
+then
   atomic64=yes
 fi
 
-- 
2.18.1

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200716131101.18462-1-thuth@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TEST    iotest-qcow2: 027
  TEST    check-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL
ERROR test-char - Bail out! ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 029
  TEST    check-qtest-x86_64: tests/qtest/hd-geo-test
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0ee63a7b9a8e423da9897fe0d132e392', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-xsqm14as/src/docker-src.2020-07-16-09.51.27.7272:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=0ee63a7b9a8e423da9897fe0d132e392
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-xsqm14as/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    15m58.033s
user    0m8.543s


The full log is available at
http://patchew.org/logs/20200716131101.18462-1-thuth@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Christian Schoenebeck]

On Donnerstag, 16. Juli 2020 15:11:01 CEST 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]

Well, __atomic_*_8() functions do exist on macOS, but it does not look like 
they are supposed to be 'officially' used.

You can compile sources with these functions, and yes they are linking fine 
despite the warning, but IMO not a good idea to use them, as AFAICS they are 
not defined by any public header file.

> 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.

Probably it is better to switch to their official C11 counterpart functions 
for this test, like e.g. __atomic_load() instead of __atomic_load_8(), etc.
That's what the actual qemu code base is using actually anyway.

Best regards,
Christian Schoenebeck

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

[Thomas Huth]

On 16/07/2020 16.15, Christian Schoenebeck wrote:
> On Donnerstag, 16. Juli 2020 15:11:01 CEST 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]
> 
> Well, __atomic_*_8() functions do exist on macOS, but it does not look like 
> they are supposed to be 'officially' used.
> 
> You can compile sources with these functions, and yes they are linking fine 
> despite the warning, but IMO not a good idea to use them, as AFAICS they are 
> not defined by any public header file.
> 
>> 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.
> 
> Probably it is better to switch to their official C11 counterpart functions 
> for this test, like e.g. __atomic_load() instead of __atomic_load_8(), etc.
> That's what the actual qemu code base is using actually anyway.

Thanks, that sounds like a good idea! I'll have a try when I've got some
spare minutes...

 Thomas

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

[Daniel P. Berrangé]

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 ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Thomas Huth]

On 16/07/2020 16.25, 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've dumped the config.log from a macOS run here:

 https://cirrus-ci.com/task/4569461585870848?command=main#L1295

Looks like -std=gnu99 is used for the test already.

 Thomas

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