Re: [RFA] mn10300 prologue analyzer fixes

[Kevin Buettner <kevinb at redhat dot com>]

On Tue, 28 Feb 2006 10:49:22 -0800
Michael Snyder <msnyder@redhat.com> wrote:

> Looks good, except that the indentation seems staggered in a
> couple of block comments.  Might just be a tabs artifact...

Yes, it was indeed a tabs artifact.  I found two lines which were
using spaces instead of tabs.  I fixed those lines to match the
whitespace convention used in the rest of the comment and now things
look better.

I've just committed the patch below:

2006-02-28  Kevin Buettner  <kevinb@redhat.com>

	* mn10300-tdep.c (mn10300_analyze_prologue):  Implement backtrack
	out of pattern match by saving relevant state.  Fix stack size
	adjustment bug.

Index: mn10300-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mn10300-tdep.c,v
retrieving revision 1.135
retrieving revision 1.136
diff -u -p -r1.135 -r1.136
--- mn10300-tdep.c	17 Dec 2005 22:34:01 -0000	1.135
+++ mn10300-tdep.c	28 Feb 2006 22:28:21 -0000	1.136
@@ -620,6 +620,17 @@ mn10300_analyze_prologue (struct frame_i
 	[mov sp,a3]        [mov sp,a3]
 	[add -SIZE2,sp]    [add -SIZE2,sp]                                 */
 
+      /* Remember the address at which we started in the event that we
+	 don't ultimately find an fmov instruction.  Once we're certain
+	 that we matched one of the above patterns, we'll set
+	 ``restore_addr'' to the appropriate value.  Note: At one time
+	 in the past, this code attempted to not adjust ``addr'' until
+	 there was a fair degree of certainty that the pattern would be
+	 matched.  However, that code did not wait until an fmov instruction
+	 was actually encountered.  As a consequence, ``addr'' would
+	 sometimes be advanced even when no fmov instructions were found.  */
+      CORE_ADDR restore_addr = addr;
+
       /* First, look for add -SIZE,sp (i.e. add imm8,sp  (0xf8feXX)
                                          or add imm16,sp (0xfafeXXXX)
                                          or add imm32,sp (0xfcfeXXXXXXXX)) */
@@ -651,10 +662,10 @@ mn10300_analyze_prologue (struct frame_i
 		     This is a one byte instruction:  mov sp,aN = 0011 11XX
 		     where XX is the register number.
 
-		     Skip this instruction by incrementing addr. (We're
-		     committed now.) The "fmov" instructions will have the
-		     form "fmov fs#,(aN+)" in this case, but that will not
-		     necessitate a change in the "fmov" parsing logic below. */
+		     Skip this instruction by incrementing addr.  The "fmov"
+		     instructions will have the form "fmov fs#,(aN+)" in this
+		     case, but that will not necessitate a change in the
+		     "fmov" parsing logic below. */
 
 		  addr++;
 
@@ -698,6 +709,14 @@ mn10300_analyze_prologue (struct frame_i
 		      if (buf[0] != 0xf9 && buf[0] != 0xfb)
 			break;
 
+		      /* An fmov instruction has just been seen.  We can
+		         now really commit to the pattern match.  Set the
+			 address to restore at the end of this speculative
+			 bit of code to the actually address that we've
+			 been incrementing (or not) throughout the
+			 speculation.  */
+		      restore_addr = addr;
+
 		      /* Get the floating point register number from the 
 			 2nd and 3rd bytes of the "fmov" instruction:
 			 Machine Code: 0000 00X0 YYYY 0000 =>
@@ -719,6 +738,7 @@ mn10300_analyze_prologue (struct frame_i
 		{
 		  /* No "fmov" was found. Reread the two bytes at the original
 		     "addr" to reset the state. */
+		  addr = restore_addr;
 		  if (!safe_frame_unwind_memory (fi, addr, buf, 2))
 		    goto finish_prologue;
 		}
@@ -727,8 +747,16 @@ mn10300_analyze_prologue (struct frame_i
 	     instruction. Handle this below. */
 	}
       /* else no "add -SIZE,sp" was found indicating no floating point
-	 registers are saved in this prologue. Do not increment addr. Pretend
-	 this bit of code never happened. */
+	 registers are saved in this prologue.  */
+
+      /* In the pattern match code contained within this block, `restore_addr'
+	 is set to the starting address at the very beginning and then
+	 iteratively to the next address to start scanning at once the
+	 pattern match has succeeded.  Thus `restore_addr' will contain
+	 the address to rewind to if the pattern match failed.  If the
+	 match succeeded, `restore_addr' and `addr' will already have the
+	 same value.  */
+      addr = restore_addr;
     }
 
   /* Now see if we set up a frame pointer via "mov sp,a3" */
@@ -777,7 +805,7 @@ mn10300_analyze_prologue (struct frame_i
 	goto finish_prologue;
 
       /* Note the size of the stack.  */
-      stack_extra_size += extract_signed_integer (buf, imm_size);
+      stack_extra_size -= extract_signed_integer (buf, imm_size);
 
       /* We just consumed 2 + imm_size bytes.  */
       addr += 2 + imm_size;