[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] x86/pv: fix clang build without CONFIG_PV32



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@xxxxxxxxxx>

I find it odd for the compiler to warn like this, but well ...
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

> 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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.