[Qemu-devel] [PATCH] sh4: fix use_icount with linux-user

[Qemu-devel] [PATCH] sh4: fix use_icount with linux-user

[Laurent Vivier]

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(-)

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 {
-- 
2.17.1

Re: [Qemu-devel] [PATCH] sh4: fix use_icount with linux-user

[John Paul Adrian Glaubitz]

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>

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] [PATCH] sh4: fix use_icount with linux-user

[Richard Henderson]

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~

Re: [Qemu-devel] [PATCH] sh4: fix use_icount with linux-user

[Richard Henderson]

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~