This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] mn10300 prologue analyzer fixes


This patch actually contains two distinct fixes, but one of them is
a one-liner...

The bulk of the patch is concerned with the pattern match that
culminates in finding a series of fmov instructions.  As explained in
the comments, the code attempted to defer the increment of `addr'
until it was certain that the pattern match succeeded.  Unfortunately,
it fails to do the right thing in at least two places.  The first is
just below that comment which used to say "We're committed now."
The second is in the `for' loop which processes the fmov instructions. 
The initialization portion of this loop advances `addr' prior to a
successful read of an fmov instruction.  If / when we do either of these
increments, but later fail to find an fmov instruction, the remainder
of the prologue scan will continue with `addr' advanced too far.

The one-liner portion of the patch concerns the adjustment of
`stack_extra_size' in the last hunk of the patch.   As I recall,
the size being extracted is a negative quantity, which necessitates
changing the sign.  This bug was discovered when attempting to
backtrace through a frame-pointer-less function.

Okay?

	* 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.134
diff -u -p -r1.134 mn10300-tdep.c
--- mn10300-tdep.c	8 Sep 2005 22:48:56 -0000	1.134
+++ mn10300-tdep.c	7 Dec 2005 22:07:09 -0000
@@ -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;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]