Bill Wendling <mo...@google.com> writes:
> On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <mo...@google.com> wrote:
>> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nat...@kernel.org> wrote:
>> > On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
>> > > Christophe Leroy <christophe.le...@csgroup.eu> writes:
>> > > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
...
>> > > >> All warnings (new ones prefixed by >>):
>> > > >>
>> > > >> In file included from arch/powerpc/kernel/asm-offsets.c:71:
>> > > >> In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>> > > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is
>> > > >>>> uninitialized when used here [-Wuninitialized]
>> > > >> *inst = ppc_inst(val);
>> > > >> ^~~
>> > > >> arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro
>> > > >> 'ppc_inst'
>> > > >> #define ppc_inst(x) (x)
>> > > >> ^
>> > > >> arch/powerpc/include/asm/inst.h:155:18: note: initialize the
>> > > >> variable 'val' to silence this warning
>> > > >> unsigned int val, suffix;
>> > > >> ^
>> > > >> = 0
>> > > >
>> > > > I can't understand what's wrong here.
...
>> > > >
>> > > > I see no possibility, no alternative path where val wouldn't be set.
>> > > > The
>> > > > asm clearly has *addr as an output param so it is always set.
>> > >
>> > > I guess clang can't convince itself of that?
...
>> >
>> > It certainly looks like there is something wrong with how clang is
>> > tracking the initialization of the variable because it looks to me like
>> > val is only used in the fallthrough path, which happens after it is
>> > initialized via lwz. Perhaps something is wrong with the logic of
>> > https://reviews.llvm.org/D71314? I've added Bill to CC (LLVM issues are
>> > being migrated from Bugzilla to GitHub Issues right now so I cannot file
>> > this upstream at the moment).
>> >
>> If I remove the casts of "val" the warning doesn't appear. I suspect
>> that when I wrote that patch I forgot to remove those when checking.
>> #include "Captain_Picard_facepalm.h"
>>
>> I'll look into it.
>>
> Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
I guess for now I'll just squash this in as a workaround?
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 631436f3f5c3..5b591c51fec9 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t
*inst, u32 *src)
if (unlikely(!is_kernel_addr((unsigned long)src)))
return -ERANGE;
+#ifdef CONFIG_CC_IS_CLANG
+ val = suffix = 0;
+#endif
__get_kernel_nofault(&val, src, u32, Efault);
if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
__get_kernel_nofault(&suffix, src + 1, u32, Efault);
cheers
