Re: powerpc-linux biarch corefile support

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Tue, 23 Jan 2007 03:13:08 +1030
> From: Alan Modra <amodra@bigpond.net.au>
> 
> 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.