Re: [Qemu-devel] [PATCH] Regression for m68k causing Single-Step via GDB/RSP to not single step

[Laurent Vivier]

Le 26/05/2019 à 09:50, Lucien Murray-Pitts a écrit :
> A regression that was introduced, with the refactor to TranslatorOps,
> drops two lines that update the PC when single-stepping is being performed.
> ( short commit 11ab74b )
> 
> This patch resolves that issue.

Fixes: 11ab74b01e0a ("target/m68k: Convert to TranslatorOps")

> Signed-off-by: Lucien Murray-Pitts <address@hidden>
> ---
>  target/m68k/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index f0534a4ba0..2922ea79c3 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -6130,6 +6130,8 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, 
> CPUState *cpu)
>          return;
>      }
>      if (dc->base.singlestep_enabled) {
> +        update_cc_op(dc);
> +        tcg_gen_movi_i32(QREG_PC, dc->pc);
>          gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
>          return;
>      }
> 

I've tested this fix single-stepping on a kernel, these two lines are 
not enough to fix the problem. In fact four lines have been dropped and 
we must re-add them all:

iff --git a/target/m68k/translate.c b/target/m68k/translate.c
index d0f6d1f5cc..6c78001501 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6200,6 +6200,10 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, 
CPUState *cpu)
         return;
     }
     if (dc->base.singlestep_enabled) {
+        if (dc->base.is_jmp != DISAS_JUMP) {
+            update_cc_op(dc);
+            tcg_gen_movi_i32(QREG_PC, dc->pc);
+        }
         gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
         return;
     }

The problem happens when we single-step over an "rts" instruction, 
instead of returning to the caller the PC points to the following 
instruction (PC + 2):

0x00002e26 in arch_cpu_idle ()
1: x/i $pc  0x2e26 <arch_cpu_idle+4>:   rts
(gdb) si
0x00002e28 in machine_restart ()
1: x/i $pc  0x2e28 <machine_restart>:   moveal 0x438ae4 <mach_reset>,%a0

The good instruction stream is:

0x00002e26 in arch_cpu_idle ()
1: x/i $pc  0x2e26 <arch_cpu_idle+4>:   rts
(gdb) si
0x0002be6a in do_idle ()
1: x/i $pc  0x2be6a <do_idle+114>:      movew %sr,%d0

Thanks,
Laurent