Clang 10 reports
domain.c:1328:10: error: variable 'cr3_mfn' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
if ( !compat )
^~~~~~~
domain.c:1334:34: note: uninitialized use occurs here
cr3_page = get_page_from_mfn(cr3_mfn, d);
^~~~~~~
domain.c:1328:5: note: remove the 'if' if its condition is always true
if ( !compat )
^~~~~~~~~~~~~~
domain.c:1042:18: note: initialize the variable 'cr3_mfn' to silence this warning
mfn_t cr3_mfn;
^
= 0
domain.c:1189:14: error: variable 'fail' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
if ( !compat )
^~~~~~~
domain.c:1211:9: note: uninitialized use occurs here
fail |= v->arch.pv.gdt_ents != c(gdt_ents);
^~~~
domain.c:1189:9: note: remove the 'if' if its condition is always true
if ( !compat )
^~~~~~~~~~~~~~
domain.c:1187:18: note: initialize the variable 'fail' to silence this warning
bool fail;
^
= false
despite this being a build with -O2 in effect, and despite "compat"
being constant "false" when CONFIG_COMPAT (and hence CONFIG_PV32) is not
defined, as it gets set at the top of the function from the result of
is_pv_32bit_domain().
Re-arrange the two "offending" if()s such that when COMPAT=n the
respective variables will be seen as unconditionally initialized. The
original aim was to have the !compat cases first, though.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I wonder how many more there are to come.
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1186,7 +1186,17 @@ int arch_set_info_guest(
unsigned long pfn = pagetable_get_pfn(v->arch.guest_table);
bool fail;
- if ( !compat )
+#ifdef CONFIG_COMPAT
+ if ( compat )
+ {
+ l4_pgentry_t *l4tab = map_domain_page(_mfn(pfn));
+
+ pfn = l4e_get_pfn(*l4tab);
+ unmap_domain_page(l4tab);
+ fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
+ }
+ else
+#endif
{
fail = xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[3];
if ( pagetable_is_null(v->arch.guest_table_user) )
@@ -1197,16 +1207,6 @@ int arch_set_info_guest(
fail |= xen_pfn_to_cr3(pfn) != c.nat->ctrlreg[1];
}
}
-#ifdef CONFIG_COMPAT
- else
- {
- l4_pgentry_t *l4tab = map_domain_page(_mfn(pfn));
-
- pfn = l4e_get_pfn(*l4tab);
- unmap_domain_page(l4tab);
- fail = compat_pfn_to_cr3(pfn) != c.cmp->ctrlreg[3];
- }
-#endif
fail |= v->arch.pv.gdt_ents != c(gdt_ents);
for ( i = 0; !fail && i < nr_gdt_frames; ++i )
@@ -1325,12 +1325,12 @@ int arch_set_info_guest(
set_bit(_VPF_in_reset, &v->pause_flags);
- if ( !compat )
- cr3_mfn = _mfn(xen_cr3_to_pfn(c.nat->ctrlreg[3]));
#ifdef CONFIG_COMPAT
- else
+ if ( compat )
cr3_mfn = _mfn(compat_cr3_to_pfn(c.cmp->ctrlreg[3]));
+ else
#endif
+ cr3_mfn = _mfn(xen_cr3_to_pfn(c.nat->ctrlreg[3]));
cr3_page = get_page_from_mfn(cr3_mfn, d);
if ( !cr3_page )
On 09/06/2021 14:14, Jan Beulich wrote: > Clang 10 reports > > domain.c:1328:10: error: variable 'cr3_mfn' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > if ( !compat ) > ^~~~~~~ > domain.c:1334:34: note: uninitialized use occurs here > cr3_page = get_page_from_mfn(cr3_mfn, d); > ^~~~~~~ > domain.c:1328:5: note: remove the 'if' if its condition is always true > if ( !compat ) > ^~~~~~~~~~~~~~ > domain.c:1042:18: note: initialize the variable 'cr3_mfn' to silence this warning > mfn_t cr3_mfn; > ^ > = 0 > domain.c:1189:14: error: variable 'fail' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > if ( !compat ) > ^~~~~~~ > domain.c:1211:9: note: uninitialized use occurs here > fail |= v->arch.pv.gdt_ents != c(gdt_ents); > ^~~~ > domain.c:1189:9: note: remove the 'if' if its condition is always true > if ( !compat ) > ^~~~~~~~~~~~~~ > domain.c:1187:18: note: initialize the variable 'fail' to silence this warning > bool fail; > ^ > = false > > despite this being a build with -O2 in effect, and despite "compat" > being constant "false" when CONFIG_COMPAT (and hence CONFIG_PV32) is not > defined, as it gets set at the top of the function from the result of > is_pv_32bit_domain(). > > Re-arrange the two "offending" if()s such that when COMPAT=n the > respective variables will be seen as unconditionally initialized. The > original aim was to have the !compat cases first, though. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > I wonder how many more there are to come. https://gitlab.com/xen-project/patchew/xen/-/pipelines/317744453 Everything seems ok now. The failure is a known arm32 randconfig issue which still hasn't been fixed, and is unrelated to this. Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 09.06.2021 17:45, Andrew Cooper wrote: > On 09/06/2021 14:14, Jan Beulich wrote: >> Clang 10 reports >> >> domain.c:1328:10: error: variable 'cr3_mfn' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] >> if ( !compat ) >> ^~~~~~~ >> domain.c:1334:34: note: uninitialized use occurs here >> cr3_page = get_page_from_mfn(cr3_mfn, d); >> ^~~~~~~ >> domain.c:1328:5: note: remove the 'if' if its condition is always true >> if ( !compat ) >> ^~~~~~~~~~~~~~ >> domain.c:1042:18: note: initialize the variable 'cr3_mfn' to silence this warning >> mfn_t cr3_mfn; >> ^ >> = 0 >> domain.c:1189:14: error: variable 'fail' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] >> if ( !compat ) >> ^~~~~~~ >> domain.c:1211:9: note: uninitialized use occurs here >> fail |= v->arch.pv.gdt_ents != c(gdt_ents); >> ^~~~ >> domain.c:1189:9: note: remove the 'if' if its condition is always true >> if ( !compat ) >> ^~~~~~~~~~~~~~ >> domain.c:1187:18: note: initialize the variable 'fail' to silence this warning >> bool fail; >> ^ >> = false >> >> despite this being a build with -O2 in effect, and despite "compat" >> being constant "false" when CONFIG_COMPAT (and hence CONFIG_PV32) is not >> defined, as it gets set at the top of the function from the result of >> is_pv_32bit_domain(). >> >> Re-arrange the two "offending" if()s such that when COMPAT=n the >> respective variables will be seen as unconditionally initialized. The >> original aim was to have the !compat cases first, though. >> >> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> I wonder how many more there are to come. > > https://gitlab.com/xen-project/patchew/xen/-/pipelines/317744453 > > Everything seems ok now. The failure is a known arm32 randconfig issue > which still hasn't been fixed, and is unrelated to this. Well, the question was primarily for current code and the presently used Clang version (which you say looks okay now), but also for arbitrary code changes which may trigger the same issue for any other similar constructs, plus also for future Clang versions, which may become even pickier. And not to forget .config variations. > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks. Jan
© 2016 - 2024 Red Hat, Inc.