Re: [PATCH for-7.0] target/arm: Don't use DISAS_NORETURN in STXP !HAVE_CMPXCHG128 codegen

[Richard Henderson <richard.henderson@linaro.org>]

On 3/31/22 09:08, Peter Maydell wrote:
> 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 848126d11e93ff
> ("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>
> ---
>   target/arm/translate-a64.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~