Re: [Qemu-devel] [PATCH] kvm: workaround build break on gcc-7.1.1 / fedora26

[Cornelia Huck]

On Thu, 3 Aug 2017 09:10:29 -0500
Eric Blake <address@hidden> wrote:

> On 08/03/2017 08:46 AM, Philippe Mathieu-Daudé wrote:
> > Hi Greg,
> > 
> > On 08/02/2017 11:47 AM, Greg Kurz wrote:  
> >> Building QEMU on fedora26 with the latest gcc package fails:
> >>
> >>    CC      ppc64-softmmu/target/ppc/kvm.o
> >> In file included from include/sysemu/hw_accel.h:16:0,
> >>                   from target/ppc/kvm.c:31:
> >> target/ppc/kvm.c: In function ‘kvmppc_booke_watchdog_enable’:
> >> include/sysemu/kvm.h:449:35: error: ‘args_tmp[i]’ may be used
> >> uninitialized
> >>   in this function [-Werror=maybe-uninitialized]
> >>               cap.args[i] = args_tmp[i];                               \
> >>                                     ^
> >> target/ppc/kvm.c: In function ‘kvmppc_set_papr’:
> >> include/sysemu/kvm.h:449:35: error: ‘args_tmp[i]’ may be used
> >> uninitialized
> >>   in this function [-Werror=maybe-uninitialized]
> >> cc1: all warnings being treated as errors  
> > 
> > I'm trying to reproduce this in our docker images (all x86_64 based) but
> > can't reproduce.  
> 
> That's because x86_64 hosts only call kvm_vm_enable_cap() with non-empty
> varargs.  

There's

target/i386/kvm.c:            kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_SYNIC, 0)) 
{

> But we have:
> 
> accel/kvm/kvm-all.c: ret = kvm_vm_enable_cap(s, KVM_CAP_S390_IRQCHIP, 0);
> 
> which is only compiled on s390 hosts (or, at least that's my guess,
> based on the cap name)

I don't see how the compiler can optimize this away, as the check for
this cap is an ioctl...

>  - and THAT code is passing empty varargs, which
> explains args_tmp[] being a 0-length array, and getting the compiler to
> complain about i < 0 always being false.

[I don't have any s390x system with gcc7 yet, or I'd test this.]

> 
> So my question on IRC was whether we can stack the decks, and force a
> non-empty args_tmp = { 0, __VA_ARGS__} coupled by skipping the first
> iteration in the for loop.  Or, since cap.args[] is already being
> zero-initialized, args_tmp = { __VA_ARGS__, 0 } means the last iteration
> of the for loop is a no-op (assigning 0 to something that is already 0)
> - although that may be harder to correctly account for both empty and
> non-empty __VA_ARGS__.

This seems a bit ugly. And I still don't understand why this only seems
to hit on ppc...