| Message ID | 20190526075056.33865-1-lucienmp_antispam@yahoo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Regression for m68k causing Single-Step via GDB/RSP to not single step | expand |
Patchew URL: https://patchew.org/QEMU/20190526075056.33865-1-lucienmp_antispam@yahoo.com/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190526075056.33865-1-lucienmp_antispam@yahoo.com Type: series Subject: [Qemu-devel] [PATCH] Regression for m68k causing Single-Step via GDB/RSP to not single step === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu * [new tag] patchew/20190526075056.33865-1-lucienmp_antispam@yahoo.com -> patchew/20190526075056.33865-1-lucienmp_antispam@yahoo.com Switched to a new branch 'test' eab81805cc Regression for m68k causing Single-Step via GDB/RSP to not single step === OUTPUT BEGIN === ERROR: Author email address is mangled by the mailing list #2: Author: Lucien Murray-Pitts via Qemu-devel <qemu-devel@nongnu.org> total: 1 errors, 0 warnings, 8 lines checked Commit eab81805ccc1 (Regression for m68k causing Single-Step via GDB/RSP to not single step) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190526075056.33865-1-lucienmp_antispam@yahoo.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 26/05/2019 09:50, Lucien Murray-Pitts wrote: > 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. > > Signed-off-by: Lucien Murray-Pitts <lucienmp_antispam@yahoo.com> > --- > 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; > } > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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 <lucienmp_antispam@yahoo.com> > --- > 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
On 6/18/19 11:44 AM, Laurent Vivier wrote: > 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 <lucienmp_antispam@yahoo.com> >> --- >> 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; > } Even this isn't quite right, according to the comments in the switch that follows. I think it'd be best written like so. r~ diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 2ae537461f..b61c7ea0f1 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -6124,27 +6124,34 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); - if (dc->base.is_jmp == DISAS_NORETURN) { - return; - } - if (dc->base.singlestep_enabled) { - gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); - return; - } - switch (dc->base.is_jmp) { + case DISAS_NORETURN: + break; case DISAS_TOO_MANY: update_cc_op(dc); - gen_jmp_tb(dc, 0, dc->pc); + if (dc->base.singlestep_enabled) { + tcg_gen_movi_i32(QREG_PC, dc->pc); + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); + } else { + gen_jmp_tb(dc, 0, dc->pc); + } break; case DISAS_JUMP: /* We updated CC_OP and PC in gen_jmp/gen_jmp_im. */ - tcg_gen_lookup_and_goto_ptr(); + if (dc->base.singlestep_enabled) { + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); + } else { + tcg_gen_lookup_and_goto_ptr(); + } break; case DISAS_EXIT: /* We updated CC_OP and PC in gen_exit_tb, but also modified other state that may require returning to the main loop. */ - tcg_gen_exit_tb(NULL, 0); + if (dc->base.singlestep_enabled) { + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); + } else { + tcg_gen_exit_tb(NULL, 0); + } break; default: g_assert_not_reached();
Le 18/06/2019 à 21:39, Richard Henderson a écrit : > On 6/18/19 11:44 AM, Laurent Vivier wrote: >> 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 <lucienmp_antispam@yahoo.com> >>> --- >>> 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; >> } > > Even this isn't quite right, according to the comments in the switch that > follows. I think it'd be best written like so. > > > r~ > > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > index 2ae537461f..b61c7ea0f1 100644 > --- a/target/m68k/translate.c > +++ b/target/m68k/translate.c > @@ -6124,27 +6124,34 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, > CPUState *cpu) > { > DisasContext *dc = container_of(dcbase, DisasContext, base); > > - if (dc->base.is_jmp == DISAS_NORETURN) { > - return; > - } > - if (dc->base.singlestep_enabled) { > - gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); > - return; > - } > - > switch (dc->base.is_jmp) { > + case DISAS_NORETURN: > + break; > case DISAS_TOO_MANY: > update_cc_op(dc); > - gen_jmp_tb(dc, 0, dc->pc); > + if (dc->base.singlestep_enabled) { > + tcg_gen_movi_i32(QREG_PC, dc->pc); > + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); > + } else { > + gen_jmp_tb(dc, 0, dc->pc); > + } > break; > case DISAS_JUMP: > /* We updated CC_OP and PC in gen_jmp/gen_jmp_im. */ > - tcg_gen_lookup_and_goto_ptr(); > + if (dc->base.singlestep_enabled) { > + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); > + } else { > + tcg_gen_lookup_and_goto_ptr(); > + } > break; > case DISAS_EXIT: > /* We updated CC_OP and PC in gen_exit_tb, but also modified > other state that may require returning to the main loop. */ > - tcg_gen_exit_tb(NULL, 0); > + if (dc->base.singlestep_enabled) { > + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); > + } else { > + tcg_gen_exit_tb(NULL, 0); > + } > break; > default: > g_assert_not_reached(); > Yes, it works too. Could you formally send a patch? Thanks, Laurent
Hi Richard/Laurent,
Great catch, I also just stumbled on this problem as well which I didnt see with my other application code.
But I have another problem after applying the changes from your email, "RTE" and breakpoints around a MOV/BusException/RTE behave oddly.
I would like to test with the same software you are using, could you tell me what M68K machine setup, and images you use as well as your debugger please?
Cheers,Luc
On Wednesday, June 19, 2019, 04:59:12 AM GMT+9, Laurent Vivier <laurent@vivier.eu> wrote:
Le 18/06/2019 à 21:39, Richard Henderson a écrit :
> On 6/18/19 11:44 AM, Laurent Vivier wrote:
>> 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 <lucienmp_antispam@yahoo.com>
>>> ---
>>> 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;
>> }
>
> Even this isn't quite right, according to the comments in the switch that
> follows. I think it'd be best written like so.
>
>
> r~
>
>
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 2ae537461f..b61c7ea0f1 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -6124,27 +6124,34 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase,
> CPUState *cpu)
> {
> DisasContext *dc = container_of(dcbase, DisasContext, base);
>
> - if (dc->base.is_jmp == DISAS_NORETURN) {
> - return;
> - }
> - if (dc->base.singlestep_enabled) {
> - gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
> - return;
> - }
> -
> switch (dc->base.is_jmp) {
> + case DISAS_NORETURN:
> + break;
> case DISAS_TOO_MANY:
> update_cc_op(dc);
> - gen_jmp_tb(dc, 0, dc->pc);
> + if (dc->base.singlestep_enabled) {
> + tcg_gen_movi_i32(QREG_PC, dc->pc);
> + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
> + } else {
> + gen_jmp_tb(dc, 0, dc->pc);
> + }
> break;
> case DISAS_JUMP:
> /* We updated CC_OP and PC in gen_jmp/gen_jmp_im. */
> - tcg_gen_lookup_and_goto_ptr();
> + if (dc->base.singlestep_enabled) {
> + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
> + } else {
> + tcg_gen_lookup_and_goto_ptr();
> + }
> break;
> case DISAS_EXIT:
> /* We updated CC_OP and PC in gen_exit_tb, but also modified
> other state that may require returning to the main loop. */
> - tcg_gen_exit_tb(NULL, 0);
> + if (dc->base.singlestep_enabled) {
> + gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG));
> + } else {
> + tcg_gen_exit_tb(NULL, 0);
> + }
> break;
> default:
> g_assert_not_reached();
>
Yes, it works too.
Could you formally send a patch?
Thanks,
Laurent
Le 26/06/2019 à 13:24, Lucien Anti-Spam a écrit : > Hi Richard/Laurent, > > Great catch, I also just stumbled on this problem as well which I didnt > see with my other application code. > > But I have another problem after applying the changes from your email, > "RTE" and breakpoints around a MOV/BusException/RTE behave oddly. > > I would like to test with the same software you are using, could you > tell me what M68K machine setup, and images you use as well as your > debugger please? Hi, I use the Q800 machine from the series: https://patchew.org/QEMU/20190619221933.1981-1-laurent@vivier.eu/ and a kernel I cross-build from the git repo. The debugger is the one from a debian/etch-m68k installation (a real m68k machine or a container with qemu-m68k and configured with binfmt_misc). I use it remotely (with command "target remote localhost:1234" and I start qemu with "-s") . More recent gdb (native or cross built) have a bug: they don't manage correctly the size of FP registers (96bit), they use 64bit FP registers that is only valid with coldfire I pass the kernel and cmdline to qemu with "-kernel" and "-append" parameters. Thanks, Laurent > Cheers, > Luc > > On Wednesday, June 19, 2019, 04:59:12 AM GMT+9, Laurent Vivier > <laurent@vivier.eu> wrote: > > > Le 18/06/2019 à 21:39, Richard Henderson a écrit : >> On 6/18/19 11:44 AM, Laurent Vivier wrote: >>> 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 <lucienmp_antispam@yahoo.com > <mailto:lucienmp_antispam@yahoo.com>> >>>> --- >>>> 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; >>> } >> >> Even this isn't quite right, according to the comments in the switch that >> follows. I think it'd be best written like so. >> >> >> r~ >> >> >> diff --git a/target/m68k/translate.c b/target/m68k/translate.c >> index 2ae537461f..b61c7ea0f1 100644 >> --- a/target/m68k/translate.c >> +++ b/target/m68k/translate.c >> @@ -6124,27 +6124,34 @@ static void m68k_tr_tb_stop(DisasContextBase > *dcbase, >> CPUState *cpu) >> { >> DisasContext *dc = container_of(dcbase, DisasContext, base); >> >> - if (dc->base.is_jmp == DISAS_NORETURN) { >> - return; >> - } >> - if (dc->base.singlestep_enabled) { >> - gen_helper_raise_exception(cpu_env, tcg_const_i32(EXCP_DEBUG)); >> - return; >> - } >> - >> switch (dc->base.is_jmp) { >> + case DISAS_NORETURN: >> + break; >> case DISAS_TOO_MANY: >> update_cc_op(dc); >> - gen_jmp_tb(dc, 0, dc->pc); >> + if (dc->base.singlestep_enabled) { >> + tcg_gen_movi_i32(QREG_PC, dc->pc); >> + gen_helper_raise_exception(cpu_env, > tcg_const_i32(EXCP_DEBUG)); >> + } else { >> + gen_jmp_tb(dc, 0, dc->pc); >> + } >> break; >> case DISAS_JUMP: >> /* We updated CC_OP and PC in gen_jmp/gen_jmp_im. */ >> - tcg_gen_lookup_and_goto_ptr(); >> + if (dc->base.singlestep_enabled) { >> + gen_helper_raise_exception(cpu_env, > tcg_const_i32(EXCP_DEBUG)); >> + } else { >> + tcg_gen_lookup_and_goto_ptr(); >> + } >> break; >> case DISAS_EXIT: >> /* We updated CC_OP and PC in gen_exit_tb, but also modified >> other state that may require returning to the main loop. */ >> - tcg_gen_exit_tb(NULL, 0); >> + if (dc->base.singlestep_enabled) { >> + gen_helper_raise_exception(cpu_env, > tcg_const_i32(EXCP_DEBUG)); >> + } else { >> + tcg_gen_exit_tb(NULL, 0); >> + } >> break; >> default: >> g_assert_not_reached(); >> > > Yes, it works too. > > Could you formally send a patch? > > Thanks, > > Laurent
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; }
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. Signed-off-by: Lucien Murray-Pitts <lucienmp_antispam@yahoo.com> --- target/m68k/translate.c | 2 ++ 1 file changed, 2 insertions(+)