| Message ID | 158941062048.240484.9693581559252337111.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Various clean-up and fixes for radix64 | expand |
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 <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > 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; >
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;
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 <groug@kaod.org> --- target/ppc/mmu-radix64.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)