RE: [PATCH] contrib/elf2dmp: prevent uninitialized warning

[Chenqun (kuhn)]

>-----Original Message-----
>From: Viktor Prutyanov [mailto:address@hidden]
>Sent: Friday, March 6, 2020 2:59 AM
>To: Chenqun (kuhn) <address@hidden>
>Cc: address@hidden; address@hidden; Zhanghailiang
><address@hidden>; address@hidden
>Subject: Re: [PATCH] contrib/elf2dmp: prevent uninitialized warning
>
>On Fri, 7 Feb 2020 12:16:01 +0800
><address@hidden> wrote:
>
>> From: Chen Qun <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Chen Qun <address@hidden>
>> ---
>>  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.
If we put the initialization outside the if() statement, it might looks better 
without affecting the functionality ?

Thanks.
>--
>Viktor Prutyanov