[RFA] Call FRAME_INIT_SAVED_REGS instead of mips_find_saved_regs

[RFA] Call FRAME_INIT_SAVED_REGS instead of mips_find_saved_regs

[Kevin Buettner]

The patch below replaces calls to mips_find_saved_regs with calls to
FRAME_INIT_SAVED_REGS.

It should be noted that FRAME_INIT_SAVED_REGS invokes
mips_find_saved_regs.  FRAME_INIT_SAVED_REGS also makes sure that
frame->saved_regs[SP_REGNUM] is set.  (I don't know why the author of
mips_find_saved_regs didn't do it there.)

The fact that frame->saved_regs[SP_REGNUM] is setup in
FRAME_INIT_SAVED_REGS has important implications for my rewrite of
mips_get_saved_register().  In the rewrite, I've changed
mips_get_saved_register() to call frame_register_unwind() instead of
the soon-to-be-defunct find_saved_register().  Now
find_saved_register() looped through frames calling
FRAME_INIT_SAVED_REGS() unconditionally.  This meant that
frame->saved_regs[SP_REGNUM] would always be set up, regardless of
the means by which frame->saved_regs had been initialized.

frame_register_unwind() and company doesn't unconditionally call
FRAME_INIT_SAVED_REGS().  Instead, it assumes if the saved_regs field
has been allocated (i.e, it's non-zero), then nothing more needs to be
done.  I think this is a reasonable assumption.  In order to make this
assumption hold for mips, we need to visit all of the other code paths
which might potentially initialize saved_regs and make sure that
they also fill in the correct value for SP_REGNUM.  If this is not
done, then the SP value ends up being incorrect for many frames. 
There is still one more code path which needs to be fixed in this
regard, but I'm submitting a patch for that separately.  (Stay tuned.)

In the course of multiarching FRAME_INIT_SAVED_REGS, I did consider
adding the necessary bits of code for setting
frame->saved_regs[SP_REGNUM] in mips_find_saved_regs, but decided it
would be more obvious if I did a straightforward translation of the
(now replaced) macro.  The elimination of mips_find_saved_regs() can
wait for another day.  After this patch though, there'll only be one
caller, so it'll be easier to merge mips_find_saved_regs into
mips_frame_init_saved_regs().

Okay to commit?

	* mips-tdep.c (read_next_frame_reg): Call FRAME_INIT_SAVED_REGS
	instead of mips_find_saved_regs.
	* (mips_pop_frame): Likewise.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.93
diff -u -p -r1.93 mips-tdep.c
--- mips-tdep.c	8 Aug 2002 23:32:52 -0000	1.93
+++ mips-tdep.c	8 Aug 2002 23:42:57 -0000
@@ -1466,7 +1466,7 @@ read_next_frame_reg (struct frame_info *
       else
 	{
 	  if (fi->saved_regs == NULL)
-	    mips_find_saved_regs (fi);
+	    FRAME_INIT_SAVED_REGS (fi);
 	  if (fi->saved_regs[regno])
 	    return read_memory_integer (ADDR_BITS_REMOVE (fi->saved_regs[regno]), MIPS_SAVED_REGSIZE);
 	}
@@ -2890,7 +2890,7 @@ mips_pop_frame (void)
 
   write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
   if (frame->saved_regs == NULL)
-    mips_find_saved_regs (frame);
+    FRAME_INIT_SAVED_REGS (frame);
   for (regnum = 0; regnum < NUM_REGS; regnum++)
     {
       if (regnum != SP_REGNUM && regnum != PC_REGNUM

Re: [RFA] Call FRAME_INIT_SAVED_REGS instead of mips_find_saved_regs

[Andrew Cagney]

> In the course of multiarching FRAME_INIT_SAVED_REGS, I did consider
> adding the necessary bits of code for setting
> frame->saved_regs[SP_REGNUM] in mips_find_saved_regs, but decided it
> would be more obvious if I did a straightforward translation of the
> (now replaced) macro.  The elimination of mips_find_saved_regs() can
> wait for another day.  After this patch though, there'll only be one
> caller, so it'll be easier to merge mips_find_saved_regs into
> mips_frame_init_saved_regs().

Can you please add a comment to mips_find_saved_regs() indicating this.

Otherwize approved.

Andrew

Re: [RFA] Call FRAME_INIT_SAVED_REGS instead of mips_find_saved_regs

[Kevin Buettner]

On Aug 9, 12:19am, Andrew Cagney wrote:
> Subject: Re: [RFA] Call FRAME_INIT_SAVED_REGS instead of mips_find_saved_r
> 
> > In the course of multiarching FRAME_INIT_SAVED_REGS, I did consider
> > adding the necessary bits of code for setting
> > frame->saved_regs[SP_REGNUM] in mips_find_saved_regs, but decided it
> > would be more obvious if I did a straightforward translation of the
> > (now replaced) macro.  The elimination of mips_find_saved_regs() can
> > wait for another day.  After this patch though, there'll only be one
> > caller, so it'll be easier to merge mips_find_saved_regs into
> > mips_frame_init_saved_regs().
> 
> Can you please add a comment to mips_find_saved_regs() indicating this.
> 
> Otherwize approved.

Committed.  For the record, here's what went in:
(Hmm... gotta fix the comment delimiter.  I'll do that in a moment.)

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.94
diff -u -p -r1.94 mips-tdep.c
--- mips-tdep.c	9 Aug 2002 01:42:41 -0000	1.94
+++ mips-tdep.c	9 Aug 2002 20:28:30 -0000
@@ -1256,7 +1256,16 @@ mips_next_pc (CORE_ADDR pc)
 }
 
 /* Guaranteed to set fci->saved_regs to some values (it never leaves it
-   NULL).  */
+   NULL).
+
+   Note: kevinb/2002-08-09: The only caller of this function is (and
+   should remain) mips_frame_init_saved_regs().  In fact,
+   aside from calling mips_find_saved_regs(), mips_frame_init_saved_regs()
+   does nothing more than set frame->saved_regs[SP_REGNUM].  These two
+   functions should really be combined and now that there is only one
+   caller, it should be straightforward.  (Watch out for multiple returns
+   though.)
+*/
 
 static void
 mips_find_saved_regs (struct frame_info *fci)
@@ -1465,7 +1474,7 @@ read_next_frame_reg (struct frame_info *
       else
 	{
 	  if (fi->saved_regs == NULL)
-	    mips_find_saved_regs (fi);
+	    FRAME_INIT_SAVED_REGS (fi);
 	  if (fi->saved_regs[regno])
 	    return read_memory_integer (ADDR_BITS_REMOVE (fi->saved_regs[regno]), MIPS_SAVED_REGSIZE);
 	}
@@ -2890,7 +2905,7 @@ mips_pop_frame (void)
 
   write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
   if (frame->saved_regs == NULL)
-    mips_find_saved_regs (frame);
+    FRAME_INIT_SAVED_REGS (frame);
   for (regnum = 0; regnum < NUM_REGS; regnum++)
     {
       if (regnum != SP_REGNUM && regnum != PC_REGNUM