On Thu, 2026-07-30 at 21:18 +0300, A-Shehab wrote: > From: Max Chou <[email protected]> > > Move gen_update_pc call before conditional logic to ensure consistent > PC state regardless of execution path. > > Previously, the host instructions generated to update the cpu_pc were > only executed in the failure path when shadow stack validation > failed. > This created inconsistent PC synchronization. > > This inconsistency caused issues in CF_PCREL mode where subsequent > instructions calculated wrong relative offsets from stale pc_save > values, and could lead to incorrect exception return addresses. > > This fix ensures PC is always synchronized before any helper that > might raise an exception, maintaining consistent translator state > across all execution paths. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4118 > Signed-off-by: Max Chou <[email protected]> > [ahshehab: rebased on current master; file moved to > target/riscv/tcg/insn_trans/ and the ssp load is now 64-bit wide] > Tested-by: A-Shehab <[email protected]> > Signed-off-by: A-Shehab <[email protected]>
Thanks! Applied to riscv-to-apply.next Alistair > --- > This is a repost of Max Chou's patch from 2025-11-05 [1], which did > not > receive any review. Rebased onto current master: the file moved to > target/riscv/tcg/insn_trans/ and the ssp load is now a 64-bit load, > so > the original patch no longer applies. > > I opened a GitLab issue (#4118) with a minimal, self-contained bare- > metal > reproducer for this bug. It runs an sspopchk that matches the shadow > stack > (the common case) followed by an auipc in the same translation block; > on > current master the auipc returns an address 4 bytes too low (exit 42) > and > with this patch it is correct (exit 0). The reproducer is included in > the > issue. > > [1] > https://lore.kernel.org/qemu-devel/[email protected]/ > > target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc > b/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc > index a813232887..d47a9f9c7d 100644 > --- a/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc > +++ b/target/riscv/tcg/insn_trans/trans_rvzicfiss.c.inc > @@ -32,6 +32,7 @@ static bool trans_sspopchk(DisasContext *ctx, > arg_sspopchk *a) > TCGLabel *skip = gen_new_label(); > uint32_t tmp = (get_xl(ctx) == MXL_RV64) ? 8 : 4; > TCGv data = tcg_temp_new(); > + gen_update_pc(ctx, 0); > TCGv_i64 wide_addr = tcg_temp_new_i64(); > tcg_gen_ld_i64(wide_addr, tcg_env, offsetof(CPURISCVState, > ssp)); > tcg_gen_trunc_i64_tl(addr, wide_addr); > @@ -42,7 +43,6 @@ static bool trans_sspopchk(DisasContext *ctx, > arg_sspopchk *a) > tcg_gen_brcond_tl(TCG_COND_EQ, data, rs1, skip); > tcg_gen_st8_i32(tcg_constant_i32(RISCV_EXCP_SW_CHECK_BCFI_TVAL), > tcg_env, offsetof(CPURISCVState, > sw_check_code)); > - gen_update_pc(ctx, 0); > gen_helper_raise_exception(tcg_env, > tcg_constant_i32(RISCV_EXCP_SW_CHECK)); > gen_set_label(skip);
