Re: [Qemu-devel] [PATCH] cpu-exec: Don't mask out external interrupts when single-stepping an invalid instruction.

[Peter Maydell]

[cc'ing RTH who may have a better grasp on how the builtin single step is
supposed to work.]

On 11 September 2014 22:02, Martin Galvan
<address@hidden> wrote:
> When using Gdb to remote-debug a program, if we try to single-step an
> invalid instruction,
> Qemu will never return control to the remote Gdb.
> The source of this problem is external interrupts being masked out
> in cpu_exec if cpu->singlestep_enabled has the SSTEP_NOIRQ flag set.
> To solve this I've added an additional flag, SSTEP_EXCEPTION,
> that will be set in the exception_with_syndrome instruction generated
> when trying to translate the invalid instruction and will be cleared
> after checking for cpu->singlestep_enabled in cpu_exec.
>
> Signed-off-by: Martin Galvan <address@hidden>
> ---
>
> The long story: Qemu generates an exception_with_syndrome instruction
> when it realizes the instruction it's trying to translate is invalid.
> That instruction in turn modifies cs->exception_index and calls cpu_loop_exit.
> Normally, the value in cs->exception_index would cause do_interrupt to set
> the PC to point to the corresponding exception handler.
> However, since we're masking out IRQs, the PC will never be set correctly.
> Even worse, since Qemu will have generated an internal exception
> to return control back to the remote Gdb *after* it generated the syndrome 
> one,
> its code will appear after the call to cpu_loop_exit.
> Since the PC won't have changed, we'll try to excecute
> the same translation block as before, thus calling cpu_loop_exit
> again, and so on.
>
> Here's the bug tracker link: https://bugs.launchpad.net/qemu/+bug/1364501
>
>  cpu-exec.c             | 6 +++++-
>  include/qom/cpu.h      | 5 +++++
>  target-arm/op_helper.c | 5 +++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 5fa172c..5d9f231 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -448,9 +448,13 @@ int cpu_exec(CPUArchState *env)
>              for(;;) {
>                  interrupt_request = cpu->interrupt_request;
>                  if (unlikely(interrupt_request)) {
> -                    if (unlikely(cpu->singlestep_enabled & SSTEP_NOIRQ)) {
> +                    if (unlikely((cpu->singlestep_enabled & SSTEP_NOIRQ) &&
> +                            !(cpu->singlestep_enabled & SSTEP_EXCEPTION))) {
>                          /* Mask out external interrupts for this step. */
>                          interrupt_request &= ~CPU_INTERRUPT_SSTEP_MASK;
> +                    } else { /* An exception occured; don't mask out
> this one */
> +                        /* Mask out any future external interrupts */
> +                        cpu->singlestep_enabled &= ~SSTEP_EXCEPTION;
>                      }
>                      if (interrupt_request & CPU_INTERRUPT_DEBUG) {
>                          cpu->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index f2df033..a57800f 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -607,6 +607,11 @@ void qemu_init_vcpu(CPUState *cpu);
>  #define SSTEP_ENABLE    0x1  /* Enable simulated HW single stepping */
>  #define SSTEP_NOIRQ     0x2  /* Do not use IRQ while single stepping */
>  #define SSTEP_NOTIMER   0x4  /* Do not Timers while single stepping */
> +#define SSTEP_EXCEPTION 0x8  /* Don't mask out exception-related
> IRQs. Set only if
> +                              * we have to process an exception while single-
> +                              * stepping (such as when
> single-stepping an invalid
> +                              * instruction).
> +                              */
>
>  /**
>   * cpu_single_step:
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index fe40358..2139ea6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -251,6 +251,11 @@ void HELPER(exception_with_syndrome)(CPUARMState
> *env, uint32_t excp,
>      CPUState *cs = CPU(arm_env_get_cpu(env));
>
>      assert(!excp_is_internal(excp));
> +
> +    if (unlikely(cs->singlestep_enabled & SSTEP_NOIRQ)) {
> +        cs->singlestep_enabled |= SSTEP_EXCEPTION;
> +    }
> +
>      cs->exception_index = excp;
>      env->exception.syndrome = syndrome;
>      cpu_loop_exit(cs);
> --
> 1.9.1
>
> --
>
> Martín Galván
>
> Software Engineer
>
> Taller Technologies Argentina
>
> San Lorenzo 47, 3rd Floor, Office 5
>
> Córdoba, Argentina
>
> Phone: 54 351 4217888 / +54 351 4218211


thanks
-- PMM