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

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/08/2017 13:36, 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
> 
> $ rpm -q gcc
> gcc-7.1.1-3.fc26.ppc64le
> 
> The compiler should obviously optimize this code away when no extra
> agument is passed to kvm_vm_enable_cap() and kvm_vcpu_enable_cap(),
> but it doesn't. This bug should be fixed one day in gcc, but we can
> also change our code pattern so that we don't hit the issue anymore.
> We workaround this, by using memcpy() instead of open-coding the copy.

Nice way to do it, thanks.  I'll queue it for 2.10.

Paolo

> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - use memcpy()
> ---
>  include/sysemu/kvm.h |   14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 91fc07ee9afe..3a458f50e9f4 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -428,11 +428,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
>              .flags = cap_flags,                                      \
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
> -        int i;                                                       \
> -        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
> -                     i < ARRAY_SIZE(cap.args); i++) {                \
> -            cap.args[i] = args_tmp[i];                               \
> -        }                                                            \
> +        size_t n = MIN(ARRAY_SIZE(args_tmp), ARRAY_SIZE(cap.args));  \
> +        memcpy(cap.args, args_tmp, n * sizeof(cap.args[0]));         \
>          kvm_vm_ioctl(s, KVM_ENABLE_CAP, &cap);                       \
>      })
>  
> @@ -443,11 +440,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
>              .flags = cap_flags,                                      \
>          };                                                           \
>          uint64_t args_tmp[] = { __VA_ARGS__ };                       \
> -        int i;                                                       \
> -        for (i = 0; i < (int)ARRAY_SIZE(args_tmp) &&                 \
> -                     i < ARRAY_SIZE(cap.args); i++) {                \
> -            cap.args[i] = args_tmp[i];                               \
> -        }                                                            \
> +        size_t n = MIN(ARRAY_SIZE(args_tmp), ARRAY_SIZE(cap.args));  \
> +        memcpy(cap.args, args_tmp, n * sizeof(cap.args[0]));         \
>          kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap);                   \
>      })
>  
>