On Sun, 30 Nov 2025 at 20:27, Richard Henderson <[email protected]> wrote: > > On 11/28/25 01:31, Peter Maydell wrote: > > On Fri, 28 Nov 2025 at 04:38, Harald van Dijk <[email protected]> > > wrote: > >> > >> trans_BRA does > >> > >> gen_a64_set_pc(s, dst); > >> set_btype_for_br(s, a->rn); > >> > >> gen_a64_set_pc does > >> > >> s->pc_save = -1; > >> > >> set_btype_for_br (if aa64_bti is enabled and the register is not x16 or > >> x17) does > >> > >> gen_pc_plus_diff(s, pc, 0); > >> > >> gen_pc_plus_diff does > >> > >> assert(s->pc_save != -1); > >> > >> Hence, this assert is getting hit. We need to call set_btype_for_br > >> before gen_a64_set_pc, and there is nothing in set_btype_for_br that > >> depends on gen_a64_set_pc having already been called, so this commit > >> simply swaps the calls. > >> > >> Signed-off-by: Harald van Dijk <[email protected]> > >> --- > >> target/arm/tcg/translate-a64.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/target/arm/tcg/translate-a64.c > >> b/target/arm/tcg/translate-a64.c > >> index 08b21d7dbf..cde22a5cca 100644 > >> --- a/target/arm/tcg/translate-a64.c > >> +++ b/target/arm/tcg/translate-a64.c > >> @@ -1916,8 +1916,8 @@ static bool trans_BRA(DisasContext *s, arg_bra *a) > >> return false; > >> } > >> dst = auth_branch_target(s, cpu_reg(s,a->rn), cpu_reg_sp(s, a->rm), > >> !a->m); > >> - gen_a64_set_pc(s, dst); > >> set_btype_for_br(s, a->rn); > >> + gen_a64_set_pc(s, dst); > >> s->base.is_jmp = DISAS_JUMP; > >> return true; > >> } > > > > The commit message on commit 64678fc45d8f6 says > > The set_btype_for_br call must be moved after the gen_a64_set_pc > > call to ensure the current pc can still be computed. > > > > but I think that is incorrect and it meant to say "moved before", > > because the actual code changes it makes to trans_BR() and > > trans_BRAZ() are > > > > @@ -1521,8 +1528,8 @@ static void set_btype_for_blr(DisasContext *s) > > > > static bool trans_BR(DisasContext *s, arg_r *a) > > { > > - gen_a64_set_pc(s, cpu_reg(s, a->rn)); > > set_btype_for_br(s, a->rn); > > + gen_a64_set_pc(s, cpu_reg(s, a->rn)); > > s->base.is_jmp = DISAS_JUMP; > > return true; > > } > > @@ -1581,8 +1588,8 @@ static bool trans_BRAZ(DisasContext *s, arg_braz *a) > > } > > > > dst = auth_branch_target(s, cpu_reg(s, a->rn), tcg_constant_i64(0), > > !a->m); > > - gen_a64_set_pc(s, dst); > > set_btype_for_br(s, a->rn); > > + gen_a64_set_pc(s, dst); > > s->base.is_jmp = DISAS_JUMP; > > return true; > > } > > > > which move the set_btype_for_br() call to before gen_a64_set_pc(). > > > > So I think that we just forgot to also include trans_BRA() in > > that change, and your patch here fixes that. > > > > Richard, does that sound right? > > Yep. > > > > > If so, this should be: > > Cc: [email protected] > > Fixes: 64678fc45d8f6 ("target/arm: Fix BTI versus CF_PCREL") > > > > and you can have > > Reviewed-by: Peter Maydell <[email protected]> > > Reviewed-by: Richard Henderson <[email protected]>
Applied to target-arm.next, thanks. -- PMM
