Re: [PULL 06/26] target/ppc: Improve logging in Radix MMU

["Cédric Le Goater" <clg@kaod.org>]

On 1/11/22 17:51, Peter Maydell wrote:
> On Tue, 4 Jan 2022 at 07:31, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-Id: <20211222071002.1568894-1-clg@kaod.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   target/ppc/mmu-radix64.c | 55 +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 52 insertions(+), 3 deletions(-)
> 
> Hi; Coverity reports that this change introduces a use of
> an uninitialized variable:
> 
>> @@ -306,6 +321,15 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu,
>>       hwaddr pte_addr;
>>       uint64_t pte;
>>
>> +    qemu_log_mask(CPU_LOG_MMU, "%s for %s @0x%"VADDR_PRIx
>> +                  " mmu_idx %u (prot %c%c%c) 0x%"HWADDR_PRIx"\n",
>> +                  __func__, access_str(access_type),
>> +                  eaddr, mmu_idx,
>> +                  *h_prot & PAGE_READ ? 'r' : '-',
>> +                  *h_prot & PAGE_WRITE ? 'w' : '-',
>> +                  *h_prot & PAGE_EXEC ? 'x' : '-',
>> +                  g_raddr);
> 
> At the top of this function we now read the value at h_prot to
> do this logging, but all the callsites pass in an uninitialized
> value:
>> 
>     int h_prot;
>     ...
>             ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, prtbe_addr,
>                                                   pate, &h_raddr, &h_prot, etc);
> 
> 
> Are the callsites wrong (should be initializing h_prot), or is the
> logging wrong (and it's this function itself which is supposed to
> initialize *h_prot as an extra return value) ?>
> This is CID 1468942.

h_prot is initialized after the logging, in ppc_radix64_check_prot().
Logging is wrong.

Thanks,

C.