[RFC] mips-tdep.c: Ignore use of sw after sd in prologue scanner

[RFC] mips-tdep.c: Ignore use of sw after sd in prologue scanner

[Kevin Buettner]

While running the GDB testsuite on a mips64 target, we were encountering
a large number of failures in gdb.base/restore.exp.

The simulator was dying with the message

    UNPREDICTABLE: PC = some-address

It turned out that GDB was incorrectly restoring some of the registers
when using GDB's "return" command.  The prologue scanner was the root
cause of the problem.  Consider the following example:

0x80020310 <callee1+4>: sd      ra,16(sp)
0x80020314 <callee1+8>: sd      s8,8(sp)
0x80020318 <callee1+12>:        move    s8,sp
0x8002031c <callee1+16>:        move    v0,a0
0x80020320 <callee1+20>:        sll     v0,v0,0x0
0x80020324 <callee1+24>:        sw      v0,0(s8)

While examining that prologue, the scanner was recording the offsets for
ra and s8 as it should, but it was also recording an offset for v0.  Note
that when saving ra and s8, the compiler uses sd instructions since this
is a 64-bit architecture, but when saving v0 (which is an argument to the
function), it uses an sw instruction.

The value of 0x7eec was being passed to callee1.  When the return was
forced, this value was restored to the high 32 bits of v0.  When, later on, the
simulator executed a move instruction involving v0, it checked to make
sure that the high 32 bits were in a reasonable state (all 0s or all 1s).
If not (and it wasn't) it would abort execution with that "UNPREDICTABLE"
message.

The patch below fixes the problem by setting a flag when an "sd"
instruction is seen.  When this flag is set, patterns involving "sw"
which save a register to the stack are ignored.

Comments?

	* mips-tdep.c (mips32_scan_prologue): After seeing an "sd"
	instruction, don't allow patterns involving the "sw"
	instruction to be used for recording the offset at which a
	register has been saved on the stack.

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.469
diff -u -p -r1.469 mips-tdep.c
--- mips-tdep.c	20 Feb 2008 14:34:43 -0000	1.469
+++ mips-tdep.c	6 Mar 2008 21:59:01 -0000
@@ -1929,6 +1929,7 @@ mips32_scan_prologue (CORE_ADDR start_pc
 
   CORE_ADDR end_prologue_addr = 0;
   int seen_sp_adjust = 0;
+  int seen_sd = 0;
   int load_immediate_bytes = 0;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
 
@@ -1973,7 +1974,7 @@ restart:
 	    break;
           seen_sp_adjust = 1;
 	}
-      else if ((high_word & 0xFFE0) == 0xafa0)	/* sw reg,offset($sp) */
+      else if (((high_word & 0xFFE0) == 0xafa0) && !seen_sd)	/* sw reg,offset($sp) */
 	{
 	  set_reg_offset (this_cache, reg, sp + low_word);
 	}
@@ -1981,6 +1982,8 @@ restart:
 	{
 	  /* Irix 6.2 N32 ABI uses sd instructions for saving $gp and $ra.  */
 	  set_reg_offset (this_cache, reg, sp + low_word);
+	  /* If sd is used, don't consider later sw instructions.  */
+	  seen_sd = 1;
 	}
       else if (high_word == 0x27be)	/* addiu $30,$sp,size */
 	{
@@ -2041,7 +2044,7 @@ restart:
 	        }
 	    }
 	}
-      else if ((high_word & 0xFFE0) == 0xafc0)	/* sw reg,offset($30) */
+      else if ((high_word & 0xFFE0) == 0xafc0 && !seen_sd)	/* sw reg,offset($30) */
 	{
 	  set_reg_offset (this_cache, reg, frame_addr + low_word);
 	}

Re: [RFC] mips-tdep.c: Ignore use of sw after sd in prologue scanner

[Daniel Jacobowitz]

On Thu, Mar 06, 2008 at 03:12:49PM -0700, Kevin Buettner wrote:
> Comments?

Seems reasonable enough.  Making it ABI-regsize-dependent would make
sense too.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] mips-tdep.c: Ignore use of sw after sd in prologue scanner

[Kevin Buettner]

On Thu, 6 Mar 2008 17:40:40 -0500
Daniel Jacobowitz <drow@false.org> wrote:

> Seems reasonable enough.  Making it ABI-regsize-dependent would make
> sense too.

Thanks for looking it over.

I decided that I liked the "ABI-regsize-dependent" approach better than
my original patch.  Here is what I've committed:

	* mips-tdep.c (mips32_scan_prologue): Use the ABI register size
	to decide whether to match instruction patterns using "sw" and "sd".

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.470
diff -u -p -r1.470 mips-tdep.c
--- mips-tdep.c	13 Mar 2008 12:22:13 -0000	1.470
+++ mips-tdep.c	14 Mar 2008 23:55:32 -0000
@@ -1931,6 +1931,7 @@ mips32_scan_prologue (CORE_ADDR start_pc
   int seen_sp_adjust = 0;
   int load_immediate_bytes = 0;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  int regsize_is_64_bits = (mips_abi_regsize (gdbarch) == 8);
 
   /* Can be called when there's no process, and hence when there's no
      NEXT_FRAME.  */
@@ -1973,11 +1974,13 @@ restart:
 	    break;
           seen_sp_adjust = 1;
 	}
-      else if ((high_word & 0xFFE0) == 0xafa0)	/* sw reg,offset($sp) */
+      else if (((high_word & 0xFFE0) == 0xafa0) /* sw reg,offset($sp) */
+               && !regsize_is_64_bits)
 	{
 	  set_reg_offset (this_cache, reg, sp + low_word);
 	}
-      else if ((high_word & 0xFFE0) == 0xffa0)	/* sd reg,offset($sp) */
+      else if (((high_word & 0xFFE0) == 0xffa0)	/* sd reg,offset($sp) */
+               && regsize_is_64_bits)
 	{
 	  /* Irix 6.2 N32 ABI uses sd instructions for saving $gp and $ra.  */
 	  set_reg_offset (this_cache, reg, sp + low_word);
@@ -2041,7 +2044,8 @@ restart:
 	        }
 	    }
 	}
-      else if ((high_word & 0xFFE0) == 0xafc0)	/* sw reg,offset($30) */
+      else if ((high_word & 0xFFE0) == 0xafc0 	/* sw reg,offset($30) */
+               && !regsize_is_64_bits)
 	{
 	  set_reg_offset (this_cache, reg, frame_addr + low_word);
 	}

Re: [RFC] mips-tdep.c: Ignore use of sw after sd in prologue scanner

[Daniel Jacobowitz]

On Fri, Mar 14, 2008 at 05:03:25PM -0700, Kevin Buettner wrote:
> On Thu, 6 Mar 2008 17:40:40 -0500
> Daniel Jacobowitz <drow@false.org> wrote:
> 
> > Seems reasonable enough.  Making it ABI-regsize-dependent would make
> > sense too.
> 
> Thanks for looking it over.
> 
> I decided that I liked the "ABI-regsize-dependent" approach better than
> my original patch.  Here is what I've committed:
> 
> 	* mips-tdep.c (mips32_scan_prologue): Use the ABI register size
> 	to decide whether to match instruction patterns using "sw" and "sd".

This version looks good to me too.

-- 
Daniel Jacobowitz
CodeSourcery