* [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26
@ 2017-08-07 11:36 Greg Kurz
2017-08-07 11:55 ` Paolo Bonzini
2017-08-07 12:16 ` Cornelia Huck
0 siblings, 2 replies; 4+ messages in thread
From: Greg Kurz @ 2017-08-07 11:36 UTC (permalink / raw)
To: qemu-devel
Cc: Cornelia Huck, Paolo Bonzini, Eric Blake, Philippe Mathieu-Daudé
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.
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); \
})
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26
2017-08-07 11:36 [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26 Greg Kurz
@ 2017-08-07 11:55 ` Paolo Bonzini
2017-08-07 14:06 ` Philippe Mathieu-Daudé
2017-08-07 12:16 ` Cornelia Huck
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2017-08-07 11:55 UTC (permalink / raw)
To: Greg Kurz, qemu-devel
Cc: Cornelia Huck, Eric Blake, Philippe Mathieu-Daudé
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); \
> })
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26
2017-08-07 11:36 [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26 Greg Kurz
2017-08-07 11:55 ` Paolo Bonzini
@ 2017-08-07 12:16 ` Cornelia Huck
1 sibling, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2017-08-07 12:16 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-devel, Paolo Bonzini, Philippe Mathieu-Daudé
On Mon, 07 Aug 2017 13:36:44 +0200
Greg Kurz <groug@kaod.org> 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.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - use memcpy()
> ---
> include/sysemu/kvm.h | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
Acked-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26
2017-08-07 11:55 ` Paolo Bonzini
@ 2017-08-07 14:06 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-07 14:06 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Greg Kurz, qemu-devel@nongnu.org Developers, Cornelia Huck, Eric Blake
On Mon, Aug 7, 2017 at 8:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 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.
Indeed :)
>
> Paolo
>
>> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.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); \
>> })
>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-07 14:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 11:36 [Qemu-devel] [PATCH v2] kvm: workaround build break on gcc-7.1.1 / fedora26 Greg Kurz
2017-08-07 11:55 ` Paolo Bonzini
2017-08-07 14:06 ` Philippe Mathieu-Daudé
2017-08-07 12:16 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.