Re: [PATCH 3/6] target/ppc: Don't initialize some local variables in ppc_radix64_xlate()

[Greg Kurz]

On Mon, 11 May 2020 11:07:06 +0200
Cédric Le Goater <address@hidden> wrote:

> On 5/7/20 7:26 PM, Greg Kurz wrote:
> > It is the job of the ppc_radix64_get_fully_qualified_addr() function
> > which is called at the beginning of ppc_radix64_xlate() to set both
> > lpid *and* pid. It doesn't buy us anything to initialize them first.
> > 
> > Worse, a bug in ppc_radix64_get_fully_qualified_addr(), eg. failing to
> > set either lpid or pid, would be undetectable by static analysis tools
> > like coverity.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> >  target/ppc/mmu-radix64.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> > index c76879f65b78..5e2d912ee346 100644
> > --- a/target/ppc/mmu-radix64.c
> > +++ b/target/ppc/mmu-radix64.c
> > @@ -433,7 +433,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr 
> > eaddr, int rwx,
> >                               bool cause_excp)
> >  {
> >      CPUPPCState *env = &cpu->env;
> > -    uint64_t lpid = 0, pid = 0;
> > +    uint64_t lpid, pid;
> >      ppc_v3_pate_t pate;
> >      int psize, prot;
> >      hwaddr g_raddr;
> > 
> 
> I am seeing this failure with gcc version 9.3.1 20200408 (Red Hat 9.3.1-2) 
> (GCC) 
> 
> target/ppc/mmu-radix64.c: In function ‘ppc_radix64_xlate’:
> target/ppc/mmu-radix64.c:314:12: error: ‘pid’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
>   314 |     offset = pid * sizeof(struct prtb_entry);
>       |     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> target/ppc/mmu-radix64.c:439:20: note: ‘pid’ was declared here
>   439 |     uint64_t lpid, pid;
>       |                    ^~~
> target/ppc/mmu-radix64.c:458:14: error: ‘lpid’ may be used uninitialized in 
> this function [-Werror=maybe-uninitialized]
>   458 |         if (!ppc64_v3_get_pate(cpu, lpid, &pate)) {
>       |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   CC      ppc64-softmmu/target/ppc/fpu_helper.o
> 
> 
> This seems like a compiler optimization issue.
> 

Ah... it seems that gcc is trying to be smart but it doesn't realize
that ppc_radix64_get_fully_qualified_addr() doesn't have any path
that leaves lpid or pid unset... :-\ Adding a default: case in both
switch statements is enough to silent gcc.

I guess it may be easier for David if I post a v2 of the entire series
that addresses all the comments.

Thanks!

> C.