| Message ID | 20250102195512.2406928-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/perfc: Cleanup, and wire up for RISCV/PPC | expand |
On 02.01.2025 20:55, Andrew Cooper wrote: > Clang, of at least verion 17 complains: > > arch/x86/pv/hypercall.c:30:10: error: variable 'eax' is used uninitialized > whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > 30 | if ( !compat ) > | ^~~~~~~ > arch/x86/pv/hypercall.c:87:29: note: uninitialized use occurs here > 87 | perfc_incra(hypercalls, eax); > | ^~~ > > This function is forced always_inline to cause compat to be > constant-propagated through, but that is only a heuristic to try and get the > compiler to do what we want, not a gurantee that it does. > > Clang doesn't appear to be able to see that the only case where compat is > true (and therefore the if() is false) is when there's an else clause on the > end which sets eax too. > > Initialise eax to -1, which ought to be optimised out, but if for whatever > reason it happens not to be, then perfc_incra() will fail it's bounds check > and do nothing. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c index 2febade44b73..17581d232e19 100644 --- a/xen/arch/x86/pv/hypercall.c +++ b/xen/arch/x86/pv/hypercall.c @@ -21,7 +21,7 @@ static void always_inline _pv_hypercall(struct cpu_user_regs *regs, bool compat) { struct vcpu *curr = current; - unsigned long eax; + unsigned long eax = -1; /* Clang -Wsometimes-uninitialized */ ASSERT(guest_kernel_mode(curr, regs));
Clang, of at least verion 17 complains: arch/x86/pv/hypercall.c:30:10: error: variable 'eax' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 30 | if ( !compat ) | ^~~~~~~ arch/x86/pv/hypercall.c:87:29: note: uninitialized use occurs here 87 | perfc_incra(hypercalls, eax); | ^~~ This function is forced always_inline to cause compat to be constant-propagated through, but that is only a heuristic to try and get the compiler to do what we want, not a gurantee that it does. Clang doesn't appear to be able to see that the only case where compat is true (and therefore the if() is false) is when there's an else clause on the end which sets eax too. Initialise eax to -1, which ought to be optimised out, but if for whatever reason it happens not to be, then perfc_incra() will fail it's bounds check and do nothing. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/pv/hypercall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: a1746cd4434dd27ca2da8430dfb10edc76264bb3 prerequisite-patch-id: c0c647c3d465fc11e039b5de751da060f2599fff prerequisite-patch-id: 675a622887bb1721684e574fc7755af79463f67b prerequisite-patch-id: 4bc07a7aa6e0f769ed7c89dc56db25091d810760 prerequisite-patch-id: b23c07e16495387ee6cb70edcbcb13f6b42246ac prerequisite-patch-id: fe09857284f3a17ff116de1f0a20d3916e8dda90