Message ID | 20210423094343.5850-3-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/clang: build fixes | expand |
On Fri, Apr 23, 2021 at 11:43:43AM +0200, Roger Pau Monne wrote:
> Clang reports the following build error without CONFIG_PV:
s/CONFIG_PV/CONFIG_PV32/ on the line above, sorry.
Thanks, Roger.
On 23.04.2021 11:43, Roger Pau Monne wrote: > Clang reports the following build error without CONFIG_PV: > > hypercall.c:253:10: error: variable 'op' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > if ( !is_pv_32bit_vcpu(curr) ) > ^~~~~~~~~~~~~~~~~~~~~~~ > hypercall.c:282:21: note: uninitialized use occurs here > return unlikely(op == __HYPERVISOR_iret) > ^~ > /root/src/xen/xen/include/xen/compiler.h:21:43: note: expanded from macro 'unlikely' > #define unlikely(x) __builtin_expect(!!(x),0) > ^ > hypercall.c:253:5: note: remove the 'if' if its condition is always true > if ( !is_pv_32bit_vcpu(curr) ) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > hypercall.c:251:21: note: initialize the variable 'op' to silence this warning > unsigned long op; > ^ > = 0 > > Rearrange the code in arch_do_multicall_call so that the if guards the > 32bit branch and when CONFIG_PV32 is not set there's no conditional at > all. > > Fixes: 527922008bc ('x86: slim down hypercall handling when !PV32') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> I find it odd for the compiler to warn like this, but well ... Acked-by: Jan Beulich <jbeulich@suse.com> > Should the is_pv_32bit_vcpu be wrapped in an unlikely hint? Not sure. Andrew did some similar rearrangement elsewhere for other reasons, without adding unlikely(). Personally I think we'd better add them, but then preferably add consistently as possible. Jan
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c index e30c59b6286..d573f74aa1e 100644 --- a/xen/arch/x86/pv/hypercall.c +++ b/xen/arch/x86/pv/hypercall.c @@ -250,34 +250,34 @@ enum mc_disposition arch_do_multicall_call(struct mc_state *state) struct vcpu *curr = current; unsigned long op; - if ( !is_pv_32bit_vcpu(curr) ) +#ifdef CONFIG_PV32 + if ( is_pv_32bit_vcpu(curr) ) { - struct multicall_entry *call = &state->call; + struct compat_multicall_entry *call = &state->compat_call; op = call->op; if ( (op < ARRAY_SIZE(pv_hypercall_table)) && - pv_hypercall_table[op].native ) - call->result = pv_hypercall_table[op].native( + pv_hypercall_table[op].compat ) + call->result = pv_hypercall_table[op].compat( call->args[0], call->args[1], call->args[2], call->args[3], call->args[4], call->args[5]); else call->result = -ENOSYS; } -#ifdef CONFIG_PV32 else +#endif { - struct compat_multicall_entry *call = &state->compat_call; + struct multicall_entry *call = &state->call; op = call->op; if ( (op < ARRAY_SIZE(pv_hypercall_table)) && - pv_hypercall_table[op].compat ) - call->result = pv_hypercall_table[op].compat( + pv_hypercall_table[op].native ) + call->result = pv_hypercall_table[op].native( call->args[0], call->args[1], call->args[2], call->args[3], call->args[4], call->args[5]); else call->result = -ENOSYS; } -#endif return unlikely(op == __HYPERVISOR_iret) ? mc_exit
Clang reports the following build error without CONFIG_PV: hypercall.c:253:10: error: variable 'op' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if ( !is_pv_32bit_vcpu(curr) ) ^~~~~~~~~~~~~~~~~~~~~~~ hypercall.c:282:21: note: uninitialized use occurs here return unlikely(op == __HYPERVISOR_iret) ^~ /root/src/xen/xen/include/xen/compiler.h:21:43: note: expanded from macro 'unlikely' #define unlikely(x) __builtin_expect(!!(x),0) ^ hypercall.c:253:5: note: remove the 'if' if its condition is always true if ( !is_pv_32bit_vcpu(curr) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ hypercall.c:251:21: note: initialize the variable 'op' to silence this warning unsigned long op; ^ = 0 Rearrange the code in arch_do_multicall_call so that the if guards the 32bit branch and when CONFIG_PV32 is not set there's no conditional at all. Fixes: 527922008bc ('x86: slim down hypercall handling when !PV32') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Should the is_pv_32bit_vcpu be wrapped in an unlikely hint? --- xen/arch/x86/pv/hypercall.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)