Message ID | 20180810222502.19663-1-laurent@vivier.eu |
---|---|
State | New |
Headers | show |
Series | sh4: fix use_icount with linux-user | expand |
On 08/11/2018 12:25 AM, Laurent Vivier wrote: > This patch revert changes from 4834871bc9 that are > not only state renaming. > > This fixes java in a linux-user chroot: > $ java --version > qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed. > qemu: uncaught target signal 6 (Aborted) - core dumped > Aborted (core dumped) > > The problem seems to be in gen_conditional_jump() in the > GUSA_EXCLUSIVE part: it should reset base.is_jmp to DISAS_NEXT after the > gen_goto_tb() as it is done in gen_delayed_conditional_jump() after the > gen_jump(). Fantastic, this fixes it! Can we get this into 3.0.0 before release? Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
On 08/10/2018 03:25 PM, Laurent Vivier wrote: > +++ b/target/sh4/translate.c > @@ -253,7 +253,6 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) > tcg_gen_lookup_and_goto_ptr(); > } > } > - ctx->base.is_jmp = DISAS_NORETURN; > } > Looking at the other places gen_goto_tb is used, this doesn't look right. Based on the description, I expected a modification in gen_conditional_jump, much like the one you remove here: > @@ -324,7 +323,6 @@ static void gen_delayed_conditional_jump(DisasContext * ctx) > gen_jump(ctx); > > gen_set_label(l1); > - ctx->base.is_jmp = DISAS_NEXT; > return; > } > r~
On 08/10/2018 03:47 PM, John Paul Adrian Glaubitz wrote:
> Fantastic, this fixes it! Can we get this into 3.0.0 before release?
Probably not, as -rc4 was supposed to be the last pre-release on Tuesday. But
perhaps a revised version can make 3.0.1 (I see problems with this fix).
r~
diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 1b9a201d6d..5010b0d349 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -253,7 +253,6 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) tcg_gen_lookup_and_goto_ptr(); } } - ctx->base.is_jmp = DISAS_NORETURN; } static void gen_jump(DisasContext * ctx) @@ -324,7 +323,6 @@ static void gen_delayed_conditional_jump(DisasContext * ctx) gen_jump(ctx); gen_set_label(l1); - ctx->base.is_jmp = DISAS_NEXT; return; } @@ -1877,6 +1875,7 @@ static void decode_opc(DisasContext * ctx) ctx->envflags &= ~GUSA_MASK; tcg_gen_movi_i32(cpu_flags, ctx->envflags); + ctx->base.is_jmp = DISAS_NORETURN; if (old_flags & DELAY_SLOT_CONDITIONAL) { gen_delayed_conditional_jump(ctx); } else {
This patch revert changes from 4834871bc9 that are not only state renaming. This fixes java in a linux-user chroot: $ java --version qemu-sh4: .../accel/tcg/cpu-exec.c:634: cpu_loop_exec_tb: Assertion `use_icount' failed. qemu: uncaught target signal 6 (Aborted) - core dumped Aborted (core dumped) The problem seems to be in gen_conditional_jump() in the GUSA_EXCLUSIVE part: it should reset base.is_jmp to DISAS_NEXT after the gen_goto_tb() as it is done in gen_delayed_conditional_jump() after the gen_jump(). Bug: https://bugs.launchpad.net/qemu/+bug/1768246 Fixes: 4834871bc95b67343248100e2a75ae0d287bc08b ("target/sh4: Convert to DisasJumpType") Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- target/sh4/translate.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)