From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0VZ5-0003cl-5m for qemu-devel@nongnu.org; Mon, 05 Sep 2011 05:34:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0VZ2-0005fN-BX for qemu-devel@nongnu.org; Mon, 05 Sep 2011 05:34:03 -0400 Received: from mel.act-europe.fr ([194.98.77.210]:51318) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0VZ2-0005cS-4H for qemu-devel@nongnu.org; Mon, 05 Sep 2011 05:34:00 -0400 Message-ID: <4E64976D.2050405@adacore.com> Date: Mon, 05 Sep 2011 11:33:33 +0200 From: Fabien Chouteau MIME-Version: 1.0 References: <1314886646-10714-1-git-send-email-chouteau@adacore.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] [SPARC] Gdbstub: Fix back-trace on SPARC32 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org On 03/09/2011 11:25, Blue Swirl wrote: > On Thu, Sep 1, 2011 at 2:17 PM, Fabien Chouteau 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 >> --- >> 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. > This is to allow target-specific implementation of the function. >> +#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(). > Excuse me, I don't understand here. >> 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). Well, maybe the statement is too strong. The ABI is required to get a valid result. Gdb cannot build back-traces if the ABI is not followed anyway. > On Sparc64 there are two ABIs (32 bit and 64 bit > with offset of -2047), though calling flushw instruction could handle > that. This solution is for SPARC32 only. > If the flush happens to trigger a fault, we're in big trouble. > That's why it's safer/easier to use this "hackish" read/write in the registers. > 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. > It's not a bug in Gdb. To build back-traces you have to read stack frames. To read stack frames, register windows must be flushed. In Qemu we can avoid flushing with this little trick. Regards, -- Fabien Chouteau