configure: Expand test which disable -Wmissing-braces

[PATCH] configure: Expand test which disable -Wmissing-braces

Posted by Anthony PERARD via 1 year, 4 months ago

From: Anthony PERARD <anthony.perard@citrix.com>

With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build
fine, but clang still suggest braces around the zero initializer in a
few places, where there is a subobject. Expand test to include a sub
struct which doesn't build on clang 6.0.0-1ubuntu2, and give:
    config-temp/qemu-conf.c:7:8: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    } x = {0};
           ^
           {}

These are the error reported by clang on QEMU's code (v7.2.0):
hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };

hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };

tests/qtest/virtio-net-test.c:322:34: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    QOSGraphTestOptions opts = { 0 };

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

While Ubuntu Bionic isn't supposed to be supported anymore, clang v6
is still the minimum required as tested by ./configure.
---
 configure | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configure b/configure
index 9f0bc57546..3cd9b8bad4 100755
--- a/configure
+++ b/configure
@@ -1290,7 +1290,11 @@ fi
 # Disable -Wmissing-braces on older compilers that warn even for
 # the "universal" C zero initializer {0}.
 cat > $TMPC << EOF
+struct s {
+  void *a;
+};
 struct {
+  struct s s;
   int a[2];
 } x = {0};
 EOF
-- 
Anthony PERARD

Re: [PATCH] configure: Expand test which disable -Wmissing-braces

Posted by Daniel P. Berrangé 1 year, 3 months ago

On Fri, Jan 06, 2023 at 03:21:10PM +0100, Anthony PERARD via wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build
> fine, but clang still suggest braces around the zero initializer in a
> few places, where there is a subobject. Expand test to include a sub
> struct which doesn't build on clang 6.0.0-1ubuntu2, and give:
>     config-temp/qemu-conf.c:7:8: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>     } x = {0};
>            ^
>            {}
> 
> These are the error reported by clang on QEMU's code (v7.2.0):
> hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>     dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
> 
> hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>     dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
> 
> tests/qtest/virtio-net-test.c:322:34: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>     QOSGraphTestOptions opts = { 0 };
> 
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> While Ubuntu Bionic isn't supposed to be supported anymore, clang v6
> is still the minimum required as tested by ./configure.

The configure checks don't always keep track with our supported OS
versions, because we often don't update the min versions until we
find a technical issue that motivates it. IOW, the supported OS
versions are considered to be the overriding policy regardless of
what the code tool/library versions checks currently implement.

So if the compile problem only exists on OS versions that are outside
our support matrix, then we should be bumping the minimum version
check in configure, which could possibly enable us to get rid of the
entire check for broken -Wmissing-braces.

> ---
>  configure | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configure b/configure
> index 9f0bc57546..3cd9b8bad4 100755
> --- a/configure
> +++ b/configure
> @@ -1290,7 +1290,11 @@ fi
>  # Disable -Wmissing-braces on older compilers that warn even for
>  # the "universal" C zero initializer {0}.
>  cat > $TMPC << EOF
> +struct s {
> +  void *a;
> +};
>  struct {
> +  struct s s;
>    int a[2];
>  } x = {0};
>  EOF
> -- 
> Anthony PERARD
> 
> 

With 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: [PATCH] configure: Expand test which disable -Wmissing-braces

Posted by Thomas Huth 1 year, 3 months ago

On 06/01/2023 15.21, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> With "clang 6.0.0-1ubuntu2" on Ubuntu Bionic, the test with build
> fine, but clang still suggest braces around the zero initializer in a
> few places, where there is a subobject. Expand test to include a sub
> struct which doesn't build on clang 6.0.0-1ubuntu2, and give:
>      config-temp/qemu-conf.c:7:8: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>      } x = {0};
>             ^
>             {}
> 
> These are the error reported by clang on QEMU's code (v7.2.0):
> hw/pci-bridge/cxl_downstream.c:101:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>      dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
> 
> hw/pci-bridge/cxl_root_port.c:62:51: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>      dvsec = (uint8_t *)&(CXLDVSECPortExtensions){ 0 };
> 
> tests/qtest/virtio-net-test.c:322:34: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
>      QOSGraphTestOptions opts = { 0 };
> 
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> While Ubuntu Bionic isn't supposed to be supported anymore, clang v6
> is still the minimum required as tested by ./configure.
> ---
>   configure | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/configure b/configure
> index 9f0bc57546..3cd9b8bad4 100755
> --- a/configure
> +++ b/configure
> @@ -1290,7 +1290,11 @@ fi
>   # Disable -Wmissing-braces on older compilers that warn even for
>   # the "universal" C zero initializer {0}.
>   cat > $TMPC << EOF
> +struct s {
> +  void *a;
> +};
>   struct {
> +  struct s s;
>     int a[2];
>   } x = {0};
>   EOF

Not sure whether this is really a good fix...

Nobody is really still testing such old compiler versions, so it would be 
better to adjust our minimum compiler version, I think.

According to repology.org, our supported distros ship these versions of 
Clang these days:

               Fedora 36: 14.0.5
       CentOS 8 (RHEL-8): 12.0.1
               Debian 11: 13.0.1
      OpenSUSE Leap 15.4: 13.0.1
        Ubuntu LTS 20.04: 12.0.0
           FreeBSD Ports: 15.0.7
           NetBSD pkgsrc: 15.0.7
                Homebrew: 15.0.7
             MSYS2 mingw: 15.0.7
             Haiku ports: 12.0.1

The Xcode that we are testing in the gitlab-CI seems to be "Apple clang 
version 13" which should correspond to LLVM 12.0.0 according to 
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)_2 
.

So from that numbers, it seems we could increase the minimal Clang version 
to 12.0.0 nowadays.

However, looking at our CI jobs, the Debian runner still seems to use Clang 
11 instead:

  https://gitlab.com/qemu-project/qemu/-/jobs/3637272746

And our tests/docker/dockerfiles/ubuntu2004.docker file still seems to use 
Clang version 10.0.0 ...

So without much additional effort, I think it should be fine to bump our 
minimal required version of Clang to 10.0.0 nowadays, thus I'd suggest that 
you send a patch for the configure script to do that instead. (Xcode version 
should then be bumped from v10 to v12 instead)

Then you can simply remove that old test for missing-braces from the 
configure script completely.

  Thomas