MIPS: Ignore invalid regs during info registers all

  The "info registers all" command causes mips_print_registers_info () to be
called for all register numbers, including invalid ones such as unused DSP
register numbers. This triggers an error () call which prevents further
register values being printed. Just silently return without printing
anything or erroring, so that all valid registers can be printed.

For example, before this patch:
  (gdb) info registers all
  zero: 0x0
  ...
  fir: 0x30f30320
  Not a valid register for the current processor type
  (gdb)

After this patch:
  (gdb) info registers all
  zero: 0x0
  ...
  fir: 0x30f30320
  restart: 0x0
  (gdb)

gdb/ChangeLog:

	* mips-tdep.c (mips_print_registers_info): Replace error for
	single invalid register with silent return.
---
 gdb/ChangeLog   | 5 +++++
 gdb/mips-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
  

On 09/30/2014 03:43 PM, James Hogan wrote:
> The "info registers all" command causes mips_print_registers_info () to be
> called for all register numbers, including invalid ones such as unused DSP
> register numbers. This triggers an error () call which prevents further
> register values being printed. Just silently return without printing
> anything or erroring, so that all valid registers can be printed.

What happens when the user does "info registers that-unused-register" ?

Thanks,
Pedro Alves

> 
> For example, before this patch:
>   (gdb) info registers all
>   zero: 0x0
>   ...
>   fir: 0x30f30320
>   Not a valid register for the current processor type
>   (gdb)
> 
> After this patch:
>   (gdb) info registers all
>   zero: 0x0
>   ...
>   fir: 0x30f30320
>   restart: 0x0
>   (gdb)
> 
> gdb/ChangeLog:
> 
> 	* mips-tdep.c (mips_print_registers_info): Replace error for
> 	single invalid register with silent return.
> ---
>  gdb/ChangeLog   | 5 +++++
>  gdb/mips-tdep.c | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index f3282144c303..5da8415c3ca0 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-09-30  James Hogan  <james.hogan@imgtec.com>
> +
> +	* mips-tdep.c (mips_print_registers_info): Replace error for
> +	single invalid register with silent return.
> +
>  2014-09-30  Andreas Arnez  <arnez@linux.vnet.ibm.com>
>  
>  	* gdbarch.sh (regset_from_core_section): Remove gdbarch method.
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 188580f2ebdc..3c4665457552 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -6332,7 +6332,7 @@ mips_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file,
>      {
>        gdb_assert (regnum >= gdbarch_num_regs (gdbarch));
>        if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
> -	error (_("Not a valid register for the current processor type"));
> +	return;
>  
>        mips_print_register (file, frame, regnum);
>        fprintf_filtered (file, "\n");
>
  

On 30/09/14 16:00, Pedro Alves wrote:
> On 09/30/2014 03:43 PM, James Hogan wrote:
>> The "info registers all" command causes mips_print_registers_info () to be
>> called for all register numbers, including invalid ones such as unused DSP
>> register numbers. This triggers an error () call which prevents further
>> register values being printed. Just silently return without printing
>> anything or erroring, so that all valid registers can be printed.
> 
> What happens when the user does "info registers that-unused-register" ?

I don't think that's possible, because the check is:
if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')

So any such register already has no name by which to refer to it.

Cheers
James
  

On 09/30/2014 04:05 PM, James Hogan wrote:
> On 30/09/14 16:00, Pedro Alves wrote:
>> On 09/30/2014 03:43 PM, James Hogan wrote:
>>> The "info registers all" command causes mips_print_registers_info () to be
>>> called for all register numbers, including invalid ones such as unused DSP
>>> register numbers. This triggers an error () call which prevents further
>>> register values being printed. Just silently return without printing
>>> anything or erroring, so that all valid registers can be printed.
>>
>> What happens when the user does "info registers that-unused-register" ?
> 
> I don't think that's possible, because the check is:
> if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
> 
> So any such register already has no name by which to refer to it.

Indeed.  :-)  I'll leave it to Maciej to approve.

I see that sh64-tdep.c:sh64_media_print_registers_info has the
same problem.

A bit silly that we force each arch backend to do this.

I guess the loop in registers_info could/should already skip
empty-named registers, like default_print_registers_info does.

Thanks,
Pedro Alves
  

On Tue, 30 Sep 2014, Pedro Alves wrote:

> >>> The "info registers all" command causes mips_print_registers_info () to be
> >>> called for all register numbers, including invalid ones such as unused DSP
> >>> register numbers. This triggers an error () call which prevents further
> >>> register values being printed. Just silently return without printing
> >>> anything or erroring, so that all valid registers can be printed.
> >>
> >> What happens when the user does "info registers that-unused-register" ?
> > 
> > I don't think that's possible, because the check is:
> > if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
> > 
> > So any such register already has no name by which to refer to it.
> 
> Indeed.  :-)  I'll leave it to Maciej to approve.

 I didn't know `info registers' (and `info all-registers' presumably as 
well) supported further arguments; I'll experiment with the patch a bit 
and see what comes out.

 James, did you push your change through regression testing?  If so, then 
how?  Please always state precisely how changes you submit have been 
validated.

  Maciej
  

Hi Maciej,

On 03/10/14 17:32, Maciej W. Rozycki wrote:
> On Tue, 30 Sep 2014, Pedro Alves wrote:
> 
>>>>> The "info registers all" command causes mips_print_registers_info () to be
>>>>> called for all register numbers, including invalid ones such as unused DSP
>>>>> register numbers. This triggers an error () call which prevents further
>>>>> register values being printed. Just silently return without printing
>>>>> anything or erroring, so that all valid registers can be printed.
>>>>
>>>> What happens when the user does "info registers that-unused-register" ?
>>>
>>> I don't think that's possible, because the check is:
>>> if (*(gdbarch_register_name (gdbarch, regnum)) == '\0')
>>>
>>> So any such register already has no name by which to refer to it.
>>
>> Indeed.  :-)  I'll leave it to Maciej to approve.
> 
>  I didn't know `info registers' (and `info all-registers' presumably as 
> well) supported further arguments; I'll experiment with the patch a bit 
> and see what comes out.
> 
>  James, did you push your change through regression testing?  If so, then 
> how?  Please always state precisely how changes you submit have been 
> validated.

No I didn't. I'll get myself set up for running the test suite.

Cheers
James