From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18591 invoked by alias); 22 Jan 2007 17:12:21 -0000 Received: (qmail 18150 invoked by uid 22791); 22 Jan 2007 17:12:17 -0000 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO brahms.sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 22 Jan 2007 17:12:04 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.13.8/8.13.8) with ESMTP id l0MHBs3O015848; Mon, 22 Jan 2007 18:11:54 +0100 (CET) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.13.8/8.13.8/Submit) id l0MHBr7d028668; Mon, 22 Jan 2007 18:11:54 +0100 (CET) Date: Mon, 22 Jan 2007 17:12:00 -0000 Message-Id: <200701221711.l0MHBr7d028668@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: amodra@bigpond.net.au CC: gdb-patches@sourceware.org In-reply-to: <20070122164308.GF8686@bubble.grove.modra.org> (message from Alan Modra on Tue, 23 Jan 2007 03:13:08 +1030) Subject: Re: powerpc-linux biarch corefile support References: <20070122164308.GF8686@bubble.grove.modra.org> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-01/txt/msg00456.txt.bz2 > Date: Tue, 23 Jan 2007 03:13:08 +1030 > From: Alan Modra > > This patch updates the powerpc-linux backend to use the newer struct > regset collect_regset facility. Along with the BFD change in > http://sources.redhat.com/ml/binutils/2006-12/msg00205.html, this > gives better support for debugging 32-bit powerpc programs under a > 64-bit gdb. A powerpc64-linux gdb running the gdb testsuite with -m32 > shows the following improvements without any regressions. No > regressions on powerpc-linux either. > > -FAIL: gdb.base/gcore.exp: where in corefile (pattern 1) > -FAIL: gdb.base/gcore.exp: corefile restored general registers > -FAIL: gdb.base/gcore.exp: capture_command_output failed on print array_func::local_array. > -FAIL: gdb.base/gcore.exp: corefile restored stack array > -FAIL: gdb.base/gcore.exp: corefile restored backtrace > -FAIL: gdb.base/multi-forks.exp: follow parent, print pids (timeout) > -FAIL: gdb.base/readline.exp: print 42 > -FAIL: gdb.threads/gcore-thread.exp: a corefile thread is executing thread2 > -FAIL: gdb.threads/gcore-thread.exp: thread2 is current thread in corefile > > We do lose one feature of the old code, which took some pains to write > the whole register field in a gregset_t when gdb's idea of the > register size was smaller than space in the gregset_t. I figure this > is unimportant since the current gdb code doesn't make use of writing > a single field as far as I can tell. Even if it did, it would > presumably be writing into a buffer that had been initialised at some > point with a full gregset. When writing the full regset, we now clear > the buffer beforehand. > > OK to apply? Unfortunately not: 1. You use C99 structure initialization syntax; GDB still uses C90. 2. The use of memset() in ppc_collect_gregset()/ppc_collect_fpregset() is wrong. These functions should leave the contents of the buffer you're collecting the registers in alone, except for the registers that are actually being collected. The special value -1, means "all registers GDB supports" not "all registers the operating system supports". In some cases the OS will store some additional bits in the data structure used by ptrace(PT_GETREGS, ...) and we want to pass those back unchanged in the matching ptrace(PT_SETREGS, ...). I guess you need this for zero-extending the GPR's to 64-bits, but I think you should do that explicitly for each register, even if REGNUM isn't -1. Also, I'd probably use gpr_size instead of gpr_step, since that expresses more clearly that the registers in the set are really that size. Mark P.S. Could you do me a favour and send unified diffs? That's seems to be what everybody does these days, and I really have lost the ability to read context diffs :(. > * ppc-linux-nat.c (supply_gregset): Use ppc_supply_gregset. > (right_fill_reg): Delete. > (fill_gregset): Use ppc_collect_gregset. > (supply_fpregset): Use ppc_supply_fpregset. > (fill_fpregset): Use ppc_collect_fpregset. > * ppc-linux-tdep.c (PPC_LINUX_PT_*): Don't define. > (right_supply_register, ppc_linux_supply_gregset): Delete. > (ppc32_linux_supply_gregset, ppc64_linux_supply_gregset): Delete. > (ppc_linux_supply_fpregset): Delete. > (ppc32_linux_reg_offsets, ppc64_linux_reg_offsets): New. > (ppc64_32_linux_reg_offsets): New. > (ppc32_linux_gregset, ppc64_linux_gregset): Update to use reg offsets, > ppc_supply_gregset, and ppc_collect_gregset. > (ppc64_32_linux_gregset): New. > (ppc_linux_fpregset): Rename to ppc32_linux_fpregset and update. > (ppc_linux_gregset, ppc_linux_fpregset): New functions. > (ppc_linux_regset_from_core_section): Update. > * ppc-tdep.h (ppc_linux_gregset, ppc_linux_fpregset): Declare. > (ppc_linux_supply_gregset, ppc_linux_supply_fpregset): Delete. > (struct ppc_reg_offsets): Add "gpr_step" field. > * ppcnbsd-tdep.c (_initialize_ppcnbsd_tdep): Init gpr_step. > * ppcobsd-tdep.c (_initialize_ppcobsd_tdep): Likewise. > * rs6000-tdep.c (ppc_supply_gregset): Heed gpr_step. Fix xer reg > offset typo. > (ppc_collect_gregset): Heed gpr_step. Clear entire gregset before > filling if transferring all regs. > (ppc_collect_fpregset): Clear entire fpregset before filling if > transferring all regs.