>-----Original Message----- >From: Viktor Prutyanov [mailto:viktor.prutya...@phystech.edu] >Sent: Friday, March 6, 2020 7:48 PM >To: Chenqun (kuhn) <kuhn.chen...@huawei.com> >Cc: qemu-triv...@nongnu.org; pbonz...@redhat.com; qemu- >de...@nongnu.org; Zhanghailiang <zhang.zhanghaili...@huawei.com> >Subject: Re: [PATCH] contrib/elf2dmp: prevent uninitialized warning > >On Fri, 6 Mar 2020 02:18:07 +0000 >"Chenqun (kuhn)" <kuhn.chen...@huawei.com> wrote: > >> >-----Original Message----- >> >From: Viktor Prutyanov [mailto:viktor.prutya...@phystech.edu] >> >Sent: Friday, March 6, 2020 2:59 AM >> >To: Chenqun (kuhn) <kuhn.chen...@huawei.com> >> >Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; Zhanghailiang >> ><zhang.zhanghaili...@huawei.com>; qemu-triv...@nongnu.org >> >Subject: Re: [PATCH] contrib/elf2dmp: prevent uninitialized warning >> > >> >On Fri, 7 Feb 2020 12:16:01 +0800 >> ><kuhn.chen...@huawei.com> wrote: >> > >> >> From: Chen Qun <kuhn.chen...@huawei.com> >> >> >> >> Fix compilation warnings: >> >> contrib/elf2dmp/main.c:66:17: warning: ‘KdpDataBlockEncoded’ may be >> >> used uninitialized in this function [-Wmaybe-uninitialized] >> >> block = __builtin_bswap64(block ^ kdbe) ^ kwa; >> >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> contrib/elf2dmp/main.c:78:24: note: ‘KdpDataBlockEncoded’ was >> >> declared here uint64_t kwn, kwa, KdpDataBlockEncoded; >> >> ^~~~~~~~~~~~~~~~~~~ >> >> >> >> Reported-by: Euler Robot <euler.ro...@huawei.com> >> >> Signed-off-by: Chen Qun <kuhn.chen...@huawei.com> >> >> --- >> >> contrib/elf2dmp/main.c | 25 ++++++++++++------------- >> >> 1 file changed, 12 insertions(+), 13 deletions(-) >> >> >> >> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c index >> >> 9a2dbc2902..203b9e6d04 100644 >> >> --- a/contrib/elf2dmp/main.c >> >> +++ b/contrib/elf2dmp/main.c >> >> @@ -76,6 +76,7 @@ static KDDEBUGGER_DATA64 *get_kdbg(uint64_t >> >> KernBase, struct pdb_reader *pdb, DBGKD_DEBUG_DATA_HEADER64 >> >kdbg_hdr; >> >> bool decode = false; >> >> uint64_t kwn, kwa, KdpDataBlockEncoded; >> >> + uint64_t KiWaitNever, KiWaitAlways; >> >> >> >> if (va_space_rw(vs, >> >> KdDebuggerDataBlock + offsetof(KDDEBUGGER_DATA64, >> >> Header), @@ -84,21 +85,19 @@ static KDDEBUGGER_DATA64 >> >> *get_kdbg(uint64_t KernBase, struct pdb_reader *pdb, return NULL; >> >> } >> >> >> >> - if (memcmp(&kdbg_hdr.OwnerTag, OwnerTag, sizeof(OwnerTag))) { >> >> - uint64_t KiWaitNever, KiWaitAlways; >> >> - >> >> - decode = true; >> >> + if (!SYM_RESOLVE(KernBase, pdb, KiWaitNever) || >> >> + !SYM_RESOLVE(KernBase, pdb, KiWaitAlways) || >> >> + !SYM_RESOLVE(KernBase, pdb, KdpDataBlockEncoded)) { >> >> + return NULL; >> >> + } >> >> >> >> - if (!SYM_RESOLVE(KernBase, pdb, KiWaitNever) || >> >> - !SYM_RESOLVE(KernBase, pdb, KiWaitAlways) || >> >> - !SYM_RESOLVE(KernBase, pdb, KdpDataBlockEncoded)) >> >> { >> >> - return NULL; >> >> - } >> >> + if (va_space_rw(vs, KiWaitNever, &kwn, sizeof(kwn), 0) || >> >> + va_space_rw(vs, KiWaitAlways, &kwa, sizeof(kwa), 0)) { >> >> + return NULL; >> >> + } >> >> >> >> - if (va_space_rw(vs, KiWaitNever, &kwn, sizeof(kwn), 0) || >> >> - va_space_rw(vs, KiWaitAlways, &kwa, sizeof(kwa), >> >> 0)) { >> >> - return NULL; >> >> - } >> >> + if (memcmp(&kdbg_hdr.OwnerTag, OwnerTag, sizeof(OwnerTag))) { >> >> + decode = true; >> >> >> >> printf("[KiWaitNever] = 0x%016"PRIx64"\n", kwn); >> >> printf("[KiWaitAlways] = 0x%016"PRIx64"\n", kwa); >> > >> >Hi! >> > >> >I suppose the problem is in your compiler, because kdbg_decode() is >> >only used when KdpDataBlockEncoded is already initialized by >> >SYM_RESOLVE(). >> > >> Hi Viktor, >> >> I know it's actually initialized when 'decode = true;', >> otherwise ' return kdbg;' no need to initialize. >> But usually the compiler cannot understand it, because it seems >> that the initialization is only in the if() statement. > >As for me, my GCC 9.2.1 doesn't show any warning while building elf2dmp. > Maybe you are right, my GCC version lower( 7.3.0).
> >> If we put the initialization outside the if() statement, it might >> looks better without affecting the functionality ? > >For now, your original patch affects the functionality. The utility tries to >resolve symbols as little as possible during conversion, because we don't >know exactly how Windows kernel works. This is the reason why KDBG >header should be checked before resolving 3 symbols. > OK , let's drop this patch. Thanks. >> >> Thanks. >> >-- >> >Viktor Prutyanov > > > >-- >Viktor Prutyanov