Re: powerpc-linux biarch corefile support

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

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