MIPS: Go back with the default Linux # of registers to 90

Message ID alpine.DEB.2.00.1604181335550.21846@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki April 18, 2016, 1:17 p.m. UTC
  Set the number of registers for non-XML-described Linux targets to 90, 
reverting a change made here with the addition of DSP register support:

commit 1faeff088bbbd037d7769d214378b4faf805fa2e
Author: Maciej W. Rozycki <macro@linux-mips.org>
Date:   Thu Mar 1 22:19:48 2012 +0000

and fixing a regression introduced for legacy `gdbserver' targets 
causing a "Remote 'g' packet reply is too long" error message where the 
amount of register data received with a `g' packet (90) exceeds the 
maximum number of registers expected (79).

Update the setting for XML-described targets, reflecting the actual 
number of registers which have been assigned numbers, matching the:

      gdb_assert (gdbarch_num_regs (gdbarch) <= MIPS_RESTART_REGNUM);

requirement in `mips_linux_init_abi'.

	gdb/
	* mips-tdep.c (mips_gdbarch_init): For GDB_OSABI_LINUX set 
	`num_regs' to 90 rather than 79.  Where a target description is 
	present adjust the setting appropriately.
---
Orgad,

 Can you please check if this change addresses your problem?

 I'm not currently set up to fully regression-test this change with 
`gdbserver', but I did some manual testing and things look right.  The 
change for `num_regs' in the XML-described case (which is now either 72 or 
79, for non-DSP and DSP targets respectively, rather than always 79) is in 
particular going to be always overridden in `mips_linux_init_abi' with 
`MIPS_RESTART_REGNUM + 1' (80) anyway -- we don't have proper support for 
XML-described bare-metal targets, so only the Linux target matters.  So I 
think this patch is safe and I'm going to install it once I've got a 
confirmation that the legacy case has been addressed.

  Maciej

gdb-mips-gdbarch-init-linux-num-regs.diff
  

Comments

Orgad Shaneh April 18, 2016, 1:23 p.m. UTC | #1
On Mon, Apr 18, 2016 at 4:17 PM, Maciej W. Rozycki <macro@imgtec.com> wrote:
> Set the number of registers for non-XML-described Linux targets to 90,
> reverting a change made here with the addition of DSP register support:
>
> commit 1faeff088bbbd037d7769d214378b4faf805fa2e
> Author: Maciej W. Rozycki <macro@linux-mips.org>
> Date:   Thu Mar 1 22:19:48 2012 +0000
>
> and fixing a regression introduced for legacy `gdbserver' targets
> causing a "Remote 'g' packet reply is too long" error message where the
> amount of register data received with a `g' packet (90) exceeds the
> maximum number of registers expected (79).
>
> Update the setting for XML-described targets, reflecting the actual
> number of registers which have been assigned numbers, matching the:
>
>       gdb_assert (gdbarch_num_regs (gdbarch) <= MIPS_RESTART_REGNUM);
>
> requirement in `mips_linux_init_abi'.
>
>         gdb/
>         * mips-tdep.c (mips_gdbarch_init): For GDB_OSABI_LINUX set
>         `num_regs' to 90 rather than 79.  Where a target description is
>         present adjust the setting appropriately.
> ---
> Orgad,
>
>  Can you please check if this change addresses your problem?
>
>  I'm not currently set up to fully regression-test this change with
> `gdbserver', but I did some manual testing and things look right.  The
> change for `num_regs' in the XML-described case (which is now either 72 or
> 79, for non-DSP and DSP targets respectively, rather than always 79) is in
> particular going to be always overridden in `mips_linux_init_abi' with
> `MIPS_RESTART_REGNUM + 1' (80) anyway -- we don't have proper support for
> XML-described bare-metal targets, so only the Linux target matters.  So I
> think this patch is safe and I'm going to install it once I've got a
> confirmation that the legacy case has been addressed.
>
>   Maciej
>
> gdb-mips-gdbarch-init-linux-num-regs.diff
> Index: binutils/gdb/mips-tdep.c
> ===================================================================
> --- binutils.orig/gdb/mips-tdep.c       2016-04-18 11:53:07.592133428 +0100
> +++ binutils/gdb/mips-tdep.c    2016-04-18 12:30:58.698574753 +0100
> @@ -8192,7 +8192,7 @@ mips_gdbarch_init (struct gdbarch_info i
>        mips_regnum.dspctl = -1;
>        dspacc = 72;
>        dspctl = 78;
> -      num_regs = 79;
> +      num_regs = 90;
>        reg_names = mips_linux_reg_names;
>      }
>    else
> @@ -8311,6 +8311,8 @@ mips_gdbarch_init (struct gdbarch_info i
>           return NULL;
>         }
>
> +      num_regs = mips_regnum.fp_implementation_revision + 1;
> +
>        if (dspacc >= 0)
>         {
>           feature = tdesc_find_feature (info.target_desc,
> @@ -8344,6 +8346,8 @@ mips_gdbarch_init (struct gdbarch_info i
>
>               mips_regnum.dspacc = dspacc;
>               mips_regnum.dspctl = dspctl;
> +
> +             num_regs = mips_regnum.dspctl + 1;
>             }
>         }
>

Hi,

It seems to solve the issue.

Thanks.

- Orgad
  
Maciej W. Rozycki April 22, 2016, 12:24 a.m. UTC | #2
On Mon, 18 Apr 2016, Orgad Shaneh wrote:

> It seems to solve the issue.

 Thanks for verifying.  I have committed this change now.

  Maciej
  

Patch

Index: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c	2016-04-18 11:53:07.592133428 +0100
+++ binutils/gdb/mips-tdep.c	2016-04-18 12:30:58.698574753 +0100
@@ -8192,7 +8192,7 @@  mips_gdbarch_init (struct gdbarch_info i
       mips_regnum.dspctl = -1;
       dspacc = 72;
       dspctl = 78;
-      num_regs = 79;
+      num_regs = 90;
       reg_names = mips_linux_reg_names;
     }
   else
@@ -8311,6 +8311,8 @@  mips_gdbarch_init (struct gdbarch_info i
 	  return NULL;
 	}
 
+      num_regs = mips_regnum.fp_implementation_revision + 1;
+
       if (dspacc >= 0)
 	{
 	  feature = tdesc_find_feature (info.target_desc,
@@ -8344,6 +8346,8 @@  mips_gdbarch_init (struct gdbarch_info i
 
 	      mips_regnum.dspacc = dspacc;
 	      mips_regnum.dspctl = dspctl;
+
+	      num_regs = mips_regnum.dspctl + 1;
 	    }
 	}