[committed] Fix MIPS frame prologue scan problem

["Maciej W. Rozycki" <macro@codesourcery.com>]

On Mon, 26 Nov 2012, Maciej W. Rozycki wrote:

> > > But in mips32_scan_prologue,
> > > the first
> > >   ADDI $s8,$sp,LocalSize
> > > instruction,
> > >   interpreted it in mips32_scan_prologue function
> > > but ended up with a wrong position of my
> > > non-ABI standard frame pointer
> > > because it changed frame pointer register from sp to s8 register,
> > > but kept frame_offset value as set by the
> > >   SUBI $sp, $sp, LocalSize
> > > instruction
> > >   analyzed before.
> > > 
> > >   Thus GDB wrongly ends up with a
> > > frame pointer located a
> > >   value of $s8 register (as from ADDI instruction analysis)
> > > + LocalSize (from SUBI instruction)
> > > 
> > >   This means that of
> > >   $sp is say at address addr
> > >   $s8  is at addr +LocalSize
> > > and the virtual frame pointer
> > >   $fp at  $s8 + LocalSize = addr + 2 * LocalSize
> > > 
> > > 
> > >   This means that it would be better to remove
> > > analysis of the ADDI $s8, $sp, LocalSize
> > > than to leave the current behavior.
> > > 
> > >   I think that we should either use my proposed patch,
> > > or completely remove the analysis of this ADDI $s8, $sp, LocalSize...
> 
>  Given the ambiguities noted above and following the principle of being 
> liberal as to what to accept I agree your proposal makes sense.  I am 
> fairly sure though that in the presence of a hard frame pointer ($fp) it 
> is that value we should rely on as it's there for a reason.

 Thanks for your patience.  I have now looked through it in depth and your 
observation is actually a regression introduced sometime between 4.16 and 
4.17, probably when the function was rewritten when MIPS16 support was 
added sometime in 1997 and the original heuristic_proc_desc code forked 
into mips32_heuristic_proc_desc and mips16_heuristic_proc_desc, which is 
how the respective functions were called back then.

 Before that regression frame_offset was unconditionally forced to zero 
whenever $fp was used to hold the virtual frame pointer -- the only use 
case supported for $fp back then.  Original code read:

    if (has_frame_reg) {
	PROC_FRAME_REG(&temp_proc_desc) = 30;
	PROC_FRAME_OFFSET(&temp_proc_desc) = 0;
    }
    else {
	PROC_FRAME_REG(&temp_proc_desc) = SP_REGNUM;
	PROC_FRAME_OFFSET(&temp_proc_desc) = frame_size;
    }

After the problematic change the PROC_FRAME_OFFSET(&temp_proc_desc) = 0 
assignment was lost.

 I have therefore applied the change below; no change is needed for MIPS16 
or microMIPS analysers as they use different code -- with frame_adjust 
holding the extra frame adjustment in the corresponding case (it may make 
sense to make all the three pieces more uniform, but that's another 
matter).

 This is not a code path that is ever used with modern GCC or it would 
have likely been spotted much earlier than 15 years after the regression 
and it is therefore not really covered by the test suite.  I have run it 
regardless, for the MIPS/Linux target and a couple multilibs, just for the 
peace of mind.

 Again, thanks for your patience and apologies to take so long.

  Maciej

2013-02-24  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* mips-tdep.c (mips32_scan_prologue): Reset frame_offset to zero
	if $fp is used as the virtual frame pointer.

Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c	2013-02-24 00:29:58.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c	2013-02-24 01:34:10.834052078 +0000
@@ -3316,6 +3316,7 @@ mips32_scan_prologue (struct gdbarch *gd
 	      frame_reg = 30;
 	      frame_addr = get_frame_register_signed
 		(this_frame, gdbarch_num_regs (gdbarch) + 30);
+	      frame_offset = 0;
 
 	      alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
 	      if (alloca_adjust > 0)