[PATCH] MIPS: Fix pc_regnum and sp_regnum initialisation

[PATCH] MIPS: Fix pc_regnum and sp_regnum initialisation

[Maciej W. Rozycki]

Hi,

 Register numbers used for pc_regnum and sp_regnum are in the cooked range 
and therefore depend on the total number of registers.  At some point the 
MIPS/Linux OS ABI initialiser has been changed such that it may change the 
number of available registers to add the artificial "restart" register.  
Therefore cooked register numbers stored in gdbarch may only be 
initialised once the OS ABI initialiser has run.

 The change below fixes that problem by moving the responsible piece of 
code down in mips_gdbarch_init.  For some reason this problem does not 
trigger in regression testing (it's been spotted in manual testing at one 
point I believe, perhaps with some follow-up patches applied), but the 
change is nevertheless almost obvious.

 No changes in mips-sde-elf (with a simulator) nor mips-linux-gnu (both 
native and remote) testing.  OK to apply?

2011-12-06  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* mips-tdep.c (mips_gdbarch_init): Only set pc_regnum and
	sp_regnum once the gdbarch_init_osabi hook has been called.

  Maciej

gdb-num-regs.diff
Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c	2011-12-05 21:37:06.615624206 +0000
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c	2011-12-05 21:37:06.625666941 +0000
@@ -6443,10 +6443,6 @@ mips_gdbarch_init (struct gdbarch_info i
 
   regnum = GDBARCH_OBSTACK_ZALLOC (gdbarch, struct mips_regnum);
   *regnum = mips_regnum;
-  /* FIXME: cagney/2003-11-15: For MIPS, hasn't gdbarch_pc_regnum been
-     replaced by gdbarch_read_pc?  */
-  set_gdbarch_pc_regnum (gdbarch, regnum->pc + num_regs);
-  set_gdbarch_sp_regnum (gdbarch, MIPS_SP_REGNUM + num_regs);
   set_gdbarch_fp0_regnum (gdbarch, regnum->fp0);
   set_gdbarch_num_regs (gdbarch, num_regs);
   set_gdbarch_num_pseudo_regs (gdbarch, num_regs);
@@ -6675,6 +6671,14 @@ mips_gdbarch_init (struct gdbarch_info i
   info.tdep_info = (void *) tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
+  /* The hook may have adjusted num_regs, fetch the final value and
+     set pc_regnum and sp_regnum now that it has been fixed.  */
+  /* FIXME: cagney/2003-11-15: For MIPS, hasn't gdbarch_pc_regnum been
+     replaced by gdbarch_read_pc?  */
+  num_regs = gdbarch_num_regs (gdbarch);
+  set_gdbarch_pc_regnum (gdbarch, regnum->pc + num_regs);
+  set_gdbarch_sp_regnum (gdbarch, MIPS_SP_REGNUM + num_regs);
+
   /* Unwind the frame.  */
   dwarf2_append_unwinders (gdbarch);
   frame_unwind_append_unwinder (gdbarch, &mips_stub_frame_unwind);

Re: [PATCH] MIPS: Fix pc_regnum and sp_regnum initialisation

[Joel Brobecker]

>  Register numbers used for pc_regnum and sp_regnum are in the cooked range 
> and therefore depend on the total number of registers.  At some point the 
> MIPS/Linux OS ABI initialiser has been changed such that it may change the 
> number of available registers to add the artificial "restart" register.  
> Therefore cooked register numbers stored in gdbarch may only be 
> initialised once the OS ABI initialiser has run.

Ugh...

> 2011-12-06  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gdb/
> 	* mips-tdep.c (mips_gdbarch_init): Only set pc_regnum and
> 	sp_regnum once the gdbarch_init_osabi hook has been called.

Thanks. This looks OK to me.

> +  /* The hook may have adjusted num_regs, fetch the final value and
> +     set pc_regnum and sp_regnum now that it has been fixed.  */
> +  /* FIXME: cagney/2003-11-15: For MIPS, hasn't gdbarch_pc_regnum been
> +     replaced by gdbarch_read_pc?  */

It'd be interesting to answer Andrew's question, but that's for another
rainy day (that would mean tomorrow if you lived in Vancouver :-).

-- 
Joel

Re: [PATCH] MIPS: Fix pc_regnum and sp_regnum initialisation

[Maciej W. Rozycki]

On Thu, 8 Dec 2011, Joel Brobecker wrote:

> > 2011-12-06  Maciej W. Rozycki  <macro@codesourcery.com>
> > 
> > 	gdb/
> > 	* mips-tdep.c (mips_gdbarch_init): Only set pc_regnum and
> > 	sp_regnum once the gdbarch_init_osabi hook has been called.
> 
> Thanks. This looks OK to me.

 This relied on gdb-mips-dsp-linux.diff somewhat as well, applied now too.
Thanks for the review.

> > +  /* The hook may have adjusted num_regs, fetch the final value and
> > +     set pc_regnum and sp_regnum now that it has been fixed.  */
> > +  /* FIXME: cagney/2003-11-15: For MIPS, hasn't gdbarch_pc_regnum been
> > +     replaced by gdbarch_read_pc?  */
> 
> It'd be interesting to answer Andrew's question, but that's for another
> rainy day (that would mean tomorrow if you lived in Vancouver :-).

 Given the special handling of the ISA bit it makes sense to replace 
pc_regnum entirely with read_pc/write_pc, but then I think pc_regnum 
should be set for safety to something that would signal an error if ever 
used, like -1 or suchlike that would be trapped by gdbarch_pc_regnum.  
I'll see if I can have a look into it (Cambridge isn't that rainy though 
-- perhaps one of the driest places in England, as far as precipitation is 
concerned, that is -- we have more than enough water elsewhere: in the 
Fens ;) ).

 And the handling of the ISA bit requires another discussion that I'll 
start shortly -- we're definitely not getting it right.

  Maciej