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

[Cédric Le Goater]

On 5/14/20 12:57 AM, 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.
> 
> Some recent versions of gcc (eg. gcc-9.3.1-2.fc30) may still think
> that lpid or pid is used uninitialized though, so this also adds
> default cases in the switch statements to make it clear this cannot
> happen.
> 
> Signed-off-by: Greg Kurz <address@hidden>

Reviewed-by: Cédric Le Goater <address@hidden>

> ---
>  target/ppc/mmu-radix64.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index c76879f65b78..07f956c9864f 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -50,6 +50,8 @@ static bool ppc_radix64_get_fully_qualified_addr(const 
> CPUPPCState *env,
>              *lpid = 0;
>              *pid = 0;
>              break;
> +        default:
> +            g_assert_not_reached();
>          }
>      } else {  /* !MSR[HV] -> Guest */
>          switch (eaddr & R_EADDR_QUADRANT) {
> @@ -64,6 +66,8 @@ static bool ppc_radix64_get_fully_qualified_addr(const 
> CPUPPCState *env,
>              *lpid = env->spr[SPR_LPIDR];
>              *pid = 0; /* pid set to 0 -> addresses guest operating system */
>              break;
> +        default:
> +            g_assert_not_reached();
>          }
>      }
>  
> @@ -433,7 +437,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;
>