mips-tdep.c: Fix retrieval of the virtual frame pointer

mips-tdep.c: Fix retrieval of the virtual frame pointer

[Maciej W. Rozycki]

Hello,

 A number of test cases in the gdb.trace/ subset fail with an error like 
this:

(gdb) actions
Enter actions for tracepoint 1, one per line.
End with a line saying just "end".
> collect parm[0], parm[1], parm[2], parm[3]
/n/bank/raid/macro/src7-mdi/combined/gdb/arch-utils.c:200: internal-error: No virtual frame pointer available
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) FAIL: gdb.trace/packetlen.exp: setup collect actions
FAIL: gdb.trace/packetlen.exp: survive the long packet send (GDB internal error)

The reason seems to be legacy_virtual_frame_pointer() assuming the raw and 
cooked numbers for the frame pointer register are the same.  This is not 
the case for MIPS.

 Tested using the mipsisa32-sde-elf target, with the mips-sim-sde32/-EB 
and mips-sim-sde32/-EL boards, fixing 19 regressions.

2007-09-28  Chris Dearman  <chris@mips.com>
            Maciej W. Rozycki  <macro@mips.com>

	* mips-tdep.c (mips_virtual_frame_pointer): New function.
	(mips_gdbarch_init): Set virtual_frame_pointer function.

 OK to apply?

  Maciej

12593.diff
Index: binutils-quilt/src/gdb/mips-tdep.c
===================================================================
--- binutils-quilt.orig/src/gdb/mips-tdep.c	2007-09-28 16:07:41.000000000 +0100
+++ binutils-quilt/src/gdb/mips-tdep.c	2007-09-28 18:26:52.000000000 +0100
@@ -5006,6 +5006,25 @@
   return value_of_register (*reg_p, frame);
 }
 
+/* The default legacy_virtual_frame_pointer function checks register
+   numbers against gdbarch_num_regs which doesn't work for us...  */
+
+static void
+mips_virtual_frame_pointer (CORE_ADDR pc,
+			    int *frame_regnum, LONGEST *frame_offset)
+{
+  if (gdbarch_deprecated_fp_regnum (current_gdbarch) >= 0)
+    *frame_regnum = gdbarch_deprecated_fp_regnum (current_gdbarch);
+  else if (MIPS_SP_REGNUM >= 0)
+    *frame_regnum = MIPS_SP_REGNUM;
+  else
+    /* Should this be an internal error?  I guess so, it is reflecting
+       an architectural limitation in the current design.  */
+    internal_error (__FILE__, __LINE__,
+		    _("No virtual frame pointer available"));
+  *frame_offset = 0;
+}
+
 static struct gdbarch *
 mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
@@ -5353,6 +5372,8 @@
   set_gdbarch_elf_make_msymbol_special (gdbarch,
 					mips_elf_make_msymbol_special);
 
+  set_gdbarch_virtual_frame_pointer (gdbarch, mips_virtual_frame_pointer);
+
   /* Fill in the OS dependant register numbers and names.  */
   {
     const char **reg_names;

Re: mips-tdep.c: Fix retrieval of the virtual frame pointer

[Daniel Jacobowitz]

On Fri, Sep 28, 2007 at 06:46:19PM +0100, Maciej W. Rozycki wrote:
> Hello,
> 
>  A number of test cases in the gdb.trace/ subset fail with an error like 
> this:

Have I mentioned that I have queued patches I haven't gotten to yet?
:-)  I spent the past two weeks fighting with an obstinate qemu, and
one of the patches I was testing was basically this one.

Mine was:

+/* Dummy virtual frame pointer method.  This is no more or less accurate
+   than most other architectures; we just need to be explicit about it,
+   because the pseudo-register gdbarch_sp_regnum will otherwise lead to
+   an assertion failure.  */
+
+static void
+mips_virtual_frame_pointer (CORE_ADDR pc, int *reg, LONGEST *offset)
+{
+  *reg = MIPS_SP_REGNUM;
+  *offset = 0;
+}

I think this is more accurate than yours, since these:

> +  if (gdbarch_deprecated_fp_regnum (current_gdbarch) >= 0)

> +  else if (MIPS_SP_REGNUM >= 0)

are both constants for MIPS.

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Fix retrieval of the virtual frame pointer

[Maciej W. Rozycki]

On Fri, 28 Sep 2007, Daniel Jacobowitz wrote:

> Have I mentioned that I have queued patches I haven't gotten to yet?
> :-)  I spent the past two weeks fighting with an obstinate qemu, and
> one of the patches I was testing was basically this one.

 I see...

> Mine was:
> 
> +/* Dummy virtual frame pointer method.  This is no more or less accurate
> +   than most other architectures; we just need to be explicit about it,
> +   because the pseudo-register gdbarch_sp_regnum will otherwise lead to
> +   an assertion failure.  */
> +
> +static void
> +mips_virtual_frame_pointer (CORE_ADDR pc, int *reg, LONGEST *offset)
> +{
> +  *reg = MIPS_SP_REGNUM;
> +  *offset = 0;
> +}
> 
> I think this is more accurate than yours, since these:

 You mean simpler?  No argument.

> > +  if (gdbarch_deprecated_fp_regnum (current_gdbarch) >= 0)
> 
> > +  else if (MIPS_SP_REGNUM >= 0)
> 
> are both constants for MIPS.

 Well, this may change in theory, but I know of no plans to do that, so 
shall I replace Chris's patch with yours and test it and you will commit 
it shortly?

  Maciej

Re: mips-tdep.c: Fix retrieval of the virtual frame pointer

[Daniel Jacobowitz]

On Fri, Sep 28, 2007 at 07:13:47PM +0100, Maciej W. Rozycki wrote:
>  Well, this may change in theory, but I know of no plans to do that, so 
> shall I replace Chris's patch with yours and test it and you will commit 
> it shortly?

I've just successfully finished testing it; a related patch is
halfway through a test cycle now.  I'll plan to commit it Monday,
OK?

-- 
Daniel Jacobowitz
CodeSourcery

Re: mips-tdep.c: Fix retrieval of the virtual frame pointer

[Maciej W. Rozycki]

On Fri, 28 Sep 2007, Daniel Jacobowitz wrote:

> I've just successfully finished testing it; a related patch is
> halfway through a test cycle now.  I'll plan to commit it Monday,
> OK?

 Fine with me, thanks.

  Maciej