Re: [PATCH] Introduce gdbarch_num_cooked_regs

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2018-10-21 17:26, Kevin Buettner wrote:
> Hi Simon,

Hi Kevin,

> LGTM, aside from a question and one formatting problem...
>
> > diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
> > index 1490ee28668a..b6e8f00a0ba1 100644
> > --- a/gdb/m68hc11-tdep.c
> > +++ b/gdb/m68hc11-tdep.c
> > @@ -854,10 +854,7 @@ m68hc11_frame_unwind_cache (struct frame_info 
> > *this_frame,
> >
> >    /* Adjust all the saved registers so that they contain addresses 
> > and not
> >       offsets.  */
> > -  for (i = 0;
> > -       i < gdbarch_num_regs (gdbarch)
> > -	   + gdbarch_num_pseudo_regs (gdbarch) - 1;
> > -       i++)
> > +  for (i = 0; i < gdbarch_num_cooked_regs (gdbarch); i++)
> >      if (trad_frame_addr_p (info->saved_regs, i))
> >        {
> >          info->saved_regs[i].addr += this_base;
>
> The " - 1" in the original expression was a mistake, right?  (I spent
> a few minutes looking at the mc68hc11's pseudo register layout but 
> can't
> find a reason for subtracting one.)

I don't think I had noticed it, thanks for pointing it out.  I took a 
look too and don't see any reason either.  The saved_regs array is 
allocated with a size equal to the number of cooked registers.  I dug up 
the patch that introduced this code, no comment about it:

https://sourceware.org/ml/gdb-patches/2003-07/msg00483.html

I'll add a comment about it in the commit log.

> > diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> > index 12382cddb357..9c34a070ae14 100644
> > --- a/gdb/tui/tui-regs.c
> > +++ b/gdb/tui/tui-regs.c
> > @@ -206,10 +206,7 @@ tui_show_register_group (struct reggroup *group,
> >
> >    /* See how many registers must be displayed.  */
> >    nr_regs = 0;
> > -  for (regnum = 0;
> > -       regnum < gdbarch_num_regs (gdbarch)
> > -		+ gdbarch_num_pseudo_regs (gdbarch);
> > -       regnum++)
> > +  for (regnum = 0; regnum < gdbarch_num_cooked_regs (gdbarch); 
> > regnum++)
> >      {
> >        const char *name;
> >
> > @@ -253,10 +250,7 @@ tui_show_register_group (struct reggroup *group,
> >
> >        /* Now set the register names and values.  */
> >        pos = 0;
> > -      for (regnum = 0;
> > -	   regnum < gdbarch_num_regs (gdbarch)
> > -		    + gdbarch_num_pseudo_regs (gdbarch);
> > -	   regnum++)
> > +      for (regnum = 0;regnum < gdbarch_num_cooked_regs (gdbarch); 
> > regnum++)
>
> Missing space between ; and "regnum".

Fixed, thanks.

I'm pushing the patch with those fixed.

Simon