Re: [PATCH] xen: Use -Wuninitialized and -Winit-self

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 28 Dec 2023, Andrew Cooper wrote:
> The use of uninitialised data is undefined behaviour.  At -O2 with trivial
> examples, both Clang and GCC delete the variable, and in the case of a
> function return, the caller gets whatever was stale in %rax prior to the call.
> 
> Clang includes -Wuninitialized within -Wall, but GCC only includes it in
> -Wextra, which is not used by Xen at this time.
> 
> Furthermore, the specific pattern of assigning a variable to itself in its
> declaration is only diagnosed by GCC with -Winit-self.  Clang does diagnoise
> simple forms of this pattern with a plain -Wuninitialized, but it fails to
> diagnose the instances in Xen that GCC manages to find.
> 
> GCC, with -Wuninitialized and -Winit-self notices:
> 
>   arch/x86/time.c: In function ‘read_pt_and_tsc’:
>   arch/x86/time.c:297:14: error: ‘best’ is used uninitialized in this 
> function [-Werror=uninitialized]
>     297 |     uint32_t best = best;
>         |              ^~~~
>   arch/x86/time.c: In function ‘read_pt_and_tmcct’:
>   arch/x86/time.c:1022:14: error: ‘best’ is used uninitialized in this 
> function [-Werror=uninitialized]
>    1022 |     uint64_t best = best;
>         |              ^~~~
> 
> and both have logic paths where best can be returned while uninitalised.  In
> both cases, initialise to ~0 like the associated *_min variable which also
> gates updating best.
> 
> Fixes: 23658e823238 ("x86/time: further improve TSC / CPU freq calibration 
> accuracy")
> Fixes: 3f3906b462d5 ("x86/APIC: calibrate against platform timer when 
> possible")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>