Skip to content

Commit

Permalink
target/arm: Don't use DISAS_NORETURN in STXP !HAVE_CMPXCHG128 codegen
Browse files Browse the repository at this point in the history
In gen_store_exclusive(), if the host does not have a cmpxchg128
primitive then we generate bad code for STXP for storing two 64-bit
values.  We generate a call to the exit_atomic helper, which never
returns, and set is_jmp to DISAS_NORETURN.  However, this is
forgetting that we have already emitted a brcond that jumps over this
call for the case where we don't hold the exclusive.  The effect is
that we don't generate any code to end the TB for the
exclusive-not-held execution path, which falls into the "exit with
TB_EXIT_REQUESTED" code that gen_tb_end() emits.  This then causes an
assert at runtime when cpu_loop_exec_tb() sees an EXIT_REQUESTED TB
return that wasn't for an interrupt or icount.

In particular, you can hit this case when using the clang sanitizers
and trying to run the xlnx-versal-virt acceptance test in 'make
check-acceptance'.  This bug was masked until commit 848126d
("meson: move int128 checks from configure") because we used to set
CONFIG_CMPXCHG128=1 and avoid the buggy codepath, but after that we
do not.

Fix the bug by not setting is_jmp.  The code after the exit_atomic
call up to the fail_label is dead, but TCG is smart enough to
eliminate it.  We do need to set 'tmp' to some valid value, though
(in the same way the exit_atomic-using code in tcg/tcg-op.c does).

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/953
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220331150858.96348-1-peter.maydell@linaro.org
  • Loading branch information
pm215 committed Apr 1, 2022
1 parent 034e050 commit a5b1e1a
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion target/arm/translate-a64.c
Expand Up @@ -2470,7 +2470,12 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
} else if (tb_cflags(s->base.tb) & CF_PARALLEL) {
if (!HAVE_CMPXCHG128) {
gen_helper_exit_atomic(cpu_env);
s->base.is_jmp = DISAS_NORETURN;
/*
* Produce a result so we have a well-formed opcode
* stream when the following (dead) code uses 'tmp'.
* TCG will remove the dead ops for us.
*/
tcg_gen_movi_i64(tmp, 0);
} else if (s->be_data == MO_LE) {
gen_helper_paired_cmpxchg64_le_parallel(tmp, cpu_env,
cpu_exclusive_addr,
Expand Down

0 comments on commit a5b1e1a

Please sign in to comment.