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

[xen staging] xen: Use -Wuninitialized and -Winit-self



commit 90a1bc9e825fa087331c63aa12bb64683bd75020
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Fri May 10 23:56:52 2024 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Sat May 11 00:13:43 2024 +0100

    xen: Use -Wuninitialized and -Winit-self
    
    Assigning a variable to itself is an anti-pattern.  It introduces definite 
UB
    in an attempt to silence a warning about possible UB.
    
    As it's definite undefined behaviour, it also mis-compiles in simple cases,
    using whatever stale value happened to be in the allocated register.
    
    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 diagnose
    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;
            |              ^~~~
    
    Fix these up to start with a value of ~0, which is also more robust in the
    case that something goes wrong.
    
    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>
---
 xen/Makefile        | 3 ++-
 xen/arch/x86/time.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 71f0cb5071..da8855f3c3 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -394,9 +394,10 @@ CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections 
-fdata-sections
 
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wwrite-strings -Wno-pointer-arith
-CFLAGS += -Wdeclaration-after-statement
+CFLAGS += -Wdeclaration-after-statement -Wuninitialized
 $(call cc-option-add,CFLAGS,CC,-Wvla)
 $(call cc-option-add,CFLAGS,CC,-Wflex-array-member-not-at-end)
+$(call cc-option-add,CFLAGS,CC,-Winit-self)
 CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
 
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 6f136f4b14..78ea095e3e 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -293,7 +293,7 @@ static uint32_t __init read_pt_and_tsc(uint64_t *tsc,
                                        const struct platform_timesource *pts)
 {
     uint64_t tsc_prev = *tsc = rdtsc_ordered(), tsc_min = ~0;
-    uint32_t best = best;
+    uint32_t best = ~0;
     unsigned int i;
 
     for ( i = 0; ; ++i )
@@ -1018,7 +1018,7 @@ static u64 __init init_platform_timer(void)
 static uint64_t __init read_pt_and_tmcct(uint32_t *tmcct)
 {
     uint32_t tmcct_prev = *tmcct = apic_tmcct_read(), tmcct_min = ~0;
-    uint64_t best = best;
+    uint64_t best = ~0;
     unsigned int i;
 
     for ( i = 0; ; ++i )
--
generated by git-patchbot for /home/xen/git/xen.git#staging



 


Rackspace

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