Re: [PATCH] Introduce gdbarch_num_cooked_regs

[Simon Marchi <simon.marchi@polymtl.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