Re: [PATCH] x86: please Clang in arch_set_info_guest()

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> 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@xxxxxxxxxx>

Thanks.

Jan