RE: [Patch 1/2] PowerPC, Fix-test-gdb.base-store.exp

[Carl Love <cel@us.ibm.com>]

Keith:

Thanks for the review.  I have fixed the typos as noted below.

                 Carl 
----------

On Fri, 2023-10-13 at 13:34 -0700, Keith Seitz wrote:
> Carl Love wrote:
> 
> > This patch fixes two regression test failures in
> > gdb.base/store.exp.  It
> > also fixes two regression failures in gdb.python/py-thread-
> > exited.exp.
> 
> I did not notice any failures on HEAD in this test? But I also don't
> see
> any new regressions in any test with your patch.  Just FAIL -> PASS.
> 
> > Patch has been tested on Power 8 LE/BE, Power 9 LE/BE, Power 10
> > with no
> > new regressions.
> 
> I've read through this patch (and tested it), and I only have a few
> very
> trivial fixes to request.  I don't think there's any reason to repost
> to
> fix a couple of typos, so just await a proper maintainers approval
> and
> commit with that approval.
> 
> I don't know this code sufficiently well to give a proper approval,
> but
> I did not notice anything egregiously wrong.
> 
> Tested-by: Keith Seitz <keiths@redhat.com>
> 
> Thanks for the patch!
> Keith
> 
> ---
>  gdb/infrun.c         | 13 ++++++++++
>  gdb/ppc-linux-tdep.c | 56
> ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+)
> 
> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
> index 784dafa59db..7fb90799dff 100644
> --- a/gdb/ppc-linux-tdep.c
> +++ b/gdb/ppc-linux-tdep.c
> @@ -83,6 +83,7 @@
>  #include "features/rs6000/powerpc-isa207-vsx64l.c"
>  #include "features/rs6000/powerpc-isa207-htm-vsx64l.c"
>  #include "features/rs6000/powerpc-e500l.c"
> +#include "dwarf2/frame.h"
>  
>  /* Shared library operations for PowerPC-Linux.  */
>  static struct target_so_ops powerpc_so_ops;
> @@ -2088,6 +2089,52 @@ ppc_linux_displaced_step_prepare  (gdbarch
> *arch, thread_info *thread,
>    return per_inferior->disp_step_buf->prepare (thread,
> displaced_pc);
>  }
>  
> +/* Convert a Dwarf 2 register number to a GDB register number for
> Linux.  */
> +static int
> +rs6000_linux_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int num)
> +{
> +  ppc_gdbarch_tdep *tdep = gdbarch_tdep<ppc_gdbarch_tdep>(gdbarch);
> +
> +  if (0 <= num && num <= 31)
> +    return tdep->ppc_gp0_regnum + num;
> +  else if (32 <= num && num <= 63)
> +    /* FIXME: jimb/2004-05-05: What should we do when the debug info
> +       specifies registers the architecture doesn't have?  Our
> +       callers don't check the value we return.  */
> +    return tdep->ppc_fp0_regnum + (num - 32);
> +  else if (77 <= num && num < 77 + 32)
> +    return tdep->ppc_vr0_regnum + (num - 77);
> +  else
> +    switch (num)
> +      {
> +      case 65:
> +	return tdep->ppc_lr_regnum;
> +      case 66:
> +	return tdep->ppc_ctr_regnum;
> +      case 76:
> +	return tdep->ppc_xer_regnum;
> +      case 109:
> +	return tdep->ppc_vrsave_regnum;
> +      case 110:
> +	return tdep->ppc_vrsave_regnum - 1; /* vscr */
> +      }
> +
> +  /* Unknown DWARF register number.  */
> +  return -1;
> +}
> +
> 
> The spacing here is inconsistent. In the function above, there is no
> newline between the comment and the definition.  Here there are two
> newlines:

Added a blank line between comment and
rs6000_linux_dwarf2_reg_to_regnum function definition.

> 
> +/* Translate a .eh_frame register to DWARF register, or adjust a
> +   .debug_frame register.  */
> +
> +

Removed the extra line.

> +static int
> +rs6000_linux_adjust_frame_regnum (struct gdbarch *gdbarch, int num,
> +				  int eh_frame_p)
> +{
> +  /* Linux uses the same numbering for .debug_frame numbering as
> .eh_frame.  */
> +  return num;
> +}
> +
>  static void
>  ppc_linux_init_abi (struct gdbarch_info info,
>  		    struct gdbarch *gdbarch)
> @@ -2135,6 +2182,15 @@ ppc_linux_init_abi (struct gdbarch_info info,
>    set_gdbarch_stap_is_single_operand (gdbarch,
> ppc_stap_is_single_operand);
>    set_gdbarch_stap_parse_special_token (gdbarch,
>  					ppc_stap_parse_special_token);
> +  /* Linux DWARF register mapping is different from the othe
> OS's.  */
> 
> Note the typo, "othe[r]". The correct plural form is "OSes".

Ah, thanks for catching the missing r and the spelling error.  Fixed.

> 
> +  set_gdbarch_dwarf2_reg_to_regnum (gdbarch,
> +				    rs6000_linux_dwarf2_reg_to_regnum);
> +  /* Note on Linux the mapping for the DWARF registers and the stab
> registers
> +     use the same numbers.  Install
> rs6000_linux_dwarf2_reg_to_regnum for the
> +     stab register mappings as well.  */
> +  set_gdbarch_stab_reg_to_regnum (gdbarch,
> +				    rs6000_linux_dwarf2_reg_to_regnum);
> +  dwarf2_frame_set_adjust_regnum (gdbarch,
> rs6000_linux_adjust_frame_regnum);
>  
>    if (tdep->wordsize == 4)
>      {