This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: powerpc-linux biarch corefile support
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: amodra at bigpond dot net dot au (Alan Modra)
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 29 Aug 2007 01:03:05 +0200 (CEST)
- Subject: Re: powerpc-linux biarch corefile support
Alan Modra wrote:
> Looking first at the generic rs6000 changes, these are:
> 1) Add a "gpr_size" field to "struct ppc_reg_offsets" so that the
> collect_regset and supply_regset functions will work with both
> 32-bit and 64-bit regsets. We can't simply use tdep->wordsize to
> deduce the size of registers, because in some cases
> (eg. supply_gregset and fill_gregset in ppc-linux-nat.c) the size
> we want is "sizeof (long)" rather than the target wordsize.
> 2) The low level ppc_supply_reg and ppc_collect_reg functions are
> changed as per Ulrich's suggestion, to zero pad a larger register
> field as necessary.
> 3) I reorganized ppc_supply_gregset et al to be a little more
> efficient. It seemed silly to run around a loop to find a gpr/fpr
> offset when transferring one register. This particular change also
> fixes a bug in one of the register offsets, and removes uses of
> current_gdbarch where the gdbarch is available from the regcache.
> 4) ppc_supply_fpregset and ppc_collect_fpregset had an assert that
> the target had a fp unit, and callers generally tested the same.
> I changed the assert to instead return if !ppc_floating_point_unit_p
> and removed the caller tests. It seems silly to duplicate the
> tests, and I believe it is correct for ppc_supply_fpregset and
> ppc_collect_fpregset to do nothing when the target has no fp unit.
> It is certainly consistent with other cases where one or more
> registers are not present in a target.
Thanks for working on this! The patch looks fine to me, however
I wonder about one change:
> -/* Extract the register values found in the WORDSIZED ABI GREGSET,
> - storing their values in REGCACHE. Note that some are left-aligned,
> - while others are right aligned. */
In particular, the original code appears to treat CCR, XER, and CTR
as left-aligned, and the others as right-aligned:
> -void
> -ppc_linux_supply_gregset (struct regcache *regcache,
> - int regnum, const void *gregs, size_t size,
> - int wordsize)
> -{
> - int regi;
> - struct gdbarch *regcache_arch = get_regcache_arch (regcache);
> - struct gdbarch_tdep *regcache_tdep = gdbarch_tdep (regcache_arch);
> - const bfd_byte *buf = gregs;
> -
> - for (regi = 0; regi < ppc_num_gprs; regi++)
> - right_supply_register (regcache, wordsize,
> - regcache_tdep->ppc_gp0_regnum + regi,
> - buf + wordsize * regi);
> -
> - right_supply_register (regcache, wordsize, gdbarch_pc_regnum (regcache_arch),
> - buf + wordsize * PPC_LINUX_PT_NIP);
> - right_supply_register (regcache, wordsize, regcache_tdep->ppc_lr_regnum,
> - buf + wordsize * PPC_LINUX_PT_LNK);
> - regcache_raw_supply (regcache, regcache_tdep->ppc_cr_regnum,
> - buf + wordsize * PPC_LINUX_PT_CCR);
> - regcache_raw_supply (regcache, regcache_tdep->ppc_xer_regnum,
> - buf + wordsize * PPC_LINUX_PT_XER);
> - regcache_raw_supply (regcache, regcache_tdep->ppc_ctr_regnum,
> - buf + wordsize * PPC_LINUX_PT_CTR);
> - if (regcache_tdep->ppc_mq_regnum != -1)
> - right_supply_register (regcache, wordsize, regcache_tdep->ppc_mq_regnum,
> - buf + wordsize * PPC_LINUX_PT_MQ);
> - right_supply_register (regcache, wordsize, regcache_tdep->ppc_ps_regnum,
> - buf + wordsize * PPC_LINUX_PT_MSR);
> -}
Your new code makes no such distinction, they're all treated as
right-aligned.
I wonder whether this change was deliberate? It does appear the
original code is actually wrong; the kernel stores CCR, XER, and
CRT just the same as e.g. NIP and LNK, as far as I can see.
Did you verify that those registers are now handled correctly in
both 64x64 and 32x64 cases (native and core)?
Also, you missed setting the gpr_size field in a couple of cases:
rs6000-aix-tdep.c:rs6000_aix32_reg_offsets
rs6000-aix-tdep.c:rs6000_aix64_reg_offsets
ppcobsd-nat.c:ppcobsd_reg_offsets
And, since you're now using gpr_size in the _fpregset routines as
well, I guess ppcobsd_fpreg_offsets would need to set gpr_size too.
However, I think it would be preferable to not use gpr_size in
those routines in the first place; the size of this register is in
fact always 4.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com