| Message ID | 20250121184847.2109128-3-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Coverity fixes | expand |
On Wed, Jan 22, 2025 at 4:50 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Coverity found a second DEADCODE issue in rmw_xireg() claiming that we can't > reach 'RISCV_EXCP_NONE' at the 'done' label: > > > 2706 done: > > 2707 if (ret) { > > 2708 return (env->virt_enabled && virt) ? > > 2709 RISCV_EXCP_VIRT_INSTRUCTION_FAULT : RISCV_EXCP_ILLEGAL_INST; > > 2710 } > >>>> CID 1590356: Control flow issues (DEADCODE) > >>>> Execution cannot reach this statement: "return RISCV_EXCP_NONE;". > > 2711 return RISCV_EXCP_NONE; > > Our label is now reduced after fixing another deadcode in the previous > patch but the problem reported here still remains: > > done: > if (ret) { > return RISCV_EXCP_ILLEGAL_INST; > } > return RISCV_EXCP_NONE; > > This happens because 'ret' changes only once at the start of the > function: > > ret = smstateen_acc_ok(env, 0, SMSTATEEN0_SVSLCT); > if (ret != RISCV_EXCP_NONE) { > return ret; > } > > So it's a guarantee that ret will be RISCV_EXCP_NONE (-1) if we ever > reach the label, i.e. "if (ret)" will always be true, and the label can > be even further reduced to: > > done: > return RISCV_EXCP_ILLEGAL_INST; > > To make a better use of the label, remove the 'else' from the > xiselect_aia_range() chain and let it fall-through to the 'done' label > since they are now both returning RISCV_EXCP_ILLEGAL_INST. > > Resolves: Coverity CID 1590356 > Fixes: dc0280723d ("target/riscv: Decouple AIA processing from xiselect and xireg") > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/csr.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index ab209d0cda..0e83c3b045 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -2697,15 +2697,10 @@ static RISCVException rmw_xireg(CPURISCVState *env, int csrno, > } else if (riscv_cpu_cfg(env)->ext_smcsrind || > riscv_cpu_cfg(env)->ext_sscsrind) { > return rmw_xireg_csrind(env, csrno, isel, val, new_val, wr_mask); > - } else { > - return RISCV_EXCP_ILLEGAL_INST; > } > > done: > - if (ret) { > - return RISCV_EXCP_ILLEGAL_INST; > - } > - return RISCV_EXCP_NONE; > + return RISCV_EXCP_ILLEGAL_INST; > } > > static RISCVException rmw_xtopei(CPURISCVState *env, int csrno, > -- > 2.47.1 > >
diff --git a/target/riscv/csr.c b/target/riscv/csr.c index ab209d0cda..0e83c3b045 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -2697,15 +2697,10 @@ static RISCVException rmw_xireg(CPURISCVState *env, int csrno, } else if (riscv_cpu_cfg(env)->ext_smcsrind || riscv_cpu_cfg(env)->ext_sscsrind) { return rmw_xireg_csrind(env, csrno, isel, val, new_val, wr_mask); - } else { - return RISCV_EXCP_ILLEGAL_INST; } done: - if (ret) { - return RISCV_EXCP_ILLEGAL_INST; - } - return RISCV_EXCP_NONE; + return RISCV_EXCP_ILLEGAL_INST; } static RISCVException rmw_xtopei(CPURISCVState *env, int csrno,