Re: [Qemu-devel] [PATCH] [SPARC] Gdbstub: Fix back-trace on SPARC32

[Blue Swirl <blauwirbel@gmail.com>]

On Thu, Sep 1, 2011 at 2:17 PM, Fabien Chouteau <chouteau@adacore.com> wrote:
> Gdb expects all registers windows to be flushed in ram, which is not the case
> in Qemu. Therefore the back-trace generation doesn't work. This patch adds a
> function to handle reads/writes in stack frames as if windows were flushed.
>
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
>  gdbstub.c             |   10 ++++--
>  target-sparc/cpu.h    |    7 ++++
>  target-sparc/helper.c |   85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 99 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 3b87c27..85d5ad7 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -41,6 +41,9 @@
>  #include "qemu_socket.h"
>  #include "kvm.h"
>
> +#ifndef TARGET_CPU_MEMORY_RW_DEBUG
> +#define TARGET_CPU_MEMORY_RW_DEBUG cpu_memory_rw_debug

These days, inline functions are preferred over macros.

> +#endif
>
>  enum {
>     GDB_SIGNAL_0 = 0,
> @@ -2013,7 +2016,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>         if (*p == ',')
>             p++;
>         len = strtoull(p, NULL, 16);
> -        if (cpu_memory_rw_debug(s->g_cpu, addr, mem_buf, len, 0) != 0) {
> +        if (TARGET_CPU_MEMORY_RW_DEBUG(s->g_cpu, addr, mem_buf, len, 0) != 0) {

cpu_memory_rw_debug() could remain unwrapped with a generic function
like cpu_gdb_sync_memory() which gdbstub should explicitly call.

Maybe the lazy condition codes etc. could be handled in similar way,
cpu_gdb_sync_registers().

>             put_packet (s, "E14");
>         } else {
>             memtohex(buf, mem_buf, len);
> @@ -2028,10 +2031,11 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>         if (*p == ':')
>             p++;
>         hextomem(mem_buf, p, len);
> -        if (cpu_memory_rw_debug(s->g_cpu, addr, mem_buf, len, 1) != 0)
> +        if (TARGET_CPU_MEMORY_RW_DEBUG(s->g_cpu, addr, mem_buf, len, 1) != 0) {
>             put_packet(s, "E14");
> -        else
> +        } else {
>             put_packet(s, "OK");
> +        }
>         break;
>     case 'p':
>         /* Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 8654f26..3f76eaf 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -495,6 +495,13 @@ int cpu_sparc_handle_mmu_fault(CPUSPARCState *env1, target_ulong address, int rw
>  target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev);
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env);
>
> +#if !defined(TARGET_SPARC64)
> +int sparc_cpu_memory_rw_debug(CPUState *env, target_ulong addr,
> +                              uint8_t *buf, int len, int is_write);
> +#define TARGET_CPU_MEMORY_RW_DEBUG sparc_cpu_memory_rw_debug
> +#endif
> +
> +
>  /* translate.c */
>  void gen_intermediate_code_init(CPUSPARCState *env);
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index 1fe1f07..2cf4e8b 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -358,6 +358,91 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env)
>     }
>  }
>
> +
> +/* Gdb expects all registers windows to be flushed in ram. This function handles
> + * reads/writes in stack frames as if windows were flushed. We assume that the
> + * sparc ABI is followed.
> + */

We can't assume that, it depends on what we are executing (BIOS, OS,
even application). On Sparc64 there are two ABIs (32 bit and 64 bit
with offset of -2047), though calling flushw instruction could handle
that. If the flush happens to trigger a fault, we're in big trouble.

Overall, I think this is too hackish. Maybe this is a bug in GDB
instead, information from backtrace is not reliable if ABI is not
known.

> +int sparc_cpu_memory_rw_debug(CPUState *env, target_ulong addr,
> +                              uint8_t *buf, int len, int is_write)
> +{
> +    int i;
> +    int len1;
> +    int cwp = env->cwp;
> +
> +    for (i = 0; i < env->nwindows; i++) {
> +        int off;
> +        target_ulong fp = env->regbase[cwp * 16 + 22];
> +
> +        /* Assume fp == 0 means end of frame.  */
> +        if (fp == 0) {
> +            break;
> +        }
> +
> +        cwp = cpu_cwp_inc(env, cwp + 1);
> +
> +        /* Invalid window ? */
> +        if (env->wim & (1 << cwp)) {
> +            break;
> +        }
> +
> +        /* According to the ABI, the stack is growing downward.  */
> +        if (addr + len < fp) {
> +            break;
> +        }
> +
> +        /* Not in this frame.  */
> +        if (addr > fp + 64) {
> +            continue;
> +        }
> +
> +        /* Handle access before this window.  */
> +        if (addr < fp) {
> +            len1 = fp - addr;
> +            if (cpu_memory_rw_debug(env, addr, buf, len1, is_write) != 0) {
> +                return -1;
> +            }
> +            addr += len1;
> +            len -= len1;
> +            buf += len1;
> +        }
> +
> +        /* Access byte per byte to registers. Not very efficient but speed is
> +         * not critical.
> +         */
> +        off = addr - fp;
> +        len1 = 64 - off;
> +
> +        if (len1 > len) {
> +            len1 = len;
> +        }
> +
> +        for (; len1; len1--) {
> +            int reg = cwp * 16 + 8 + (off >> 2);
> +            union {
> +                uint32_t v;
> +                uint8_t c[4];
> +            } u;
> +            u.v = cpu_to_be32(env->regbase[reg]);
> +            if (is_write) {
> +                u.c[off & 3] = *buf++;
> +                env->regbase[reg] = be32_to_cpu(u.v);
> +            } else {
> +                *buf++ = u.c[off & 3];
> +            }
> +            addr++;
> +            len--;
> +            off++;
> +        }
> +
> +        if (len == 0) {
> +            return 0;
> +        }
> +    }
> +    return cpu_memory_rw_debug(env, addr, buf, len, is_write);
> +}
> +
> +
>  #else /* !TARGET_SPARC64 */
>
>  // 41 bit physical address space
> --
> 1.7.4.1
>
>