Re: [PATCH v2] target/arm: Fix assert on BRA.

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 28 Nov 2025 at 04:38, Harald van Dijk <hdijk@accesssoftek.com> 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 <hdijk@accesssoftek.com>
> ---
>  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?

If so, this should be:
Cc: qemu-stable@nongnu.org
Fixes: 64678fc45d8f6 ("target/arm: Fix BTI versus CF_PCREL")

and you can have
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM