This is the mail archive of the gdb-patches@sources.redhat.com 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]

[PATCH RFA] arm-tdep.c: prologue scanner adjustments


I've come across a case where the current ARM prologue scanner is
deficient.  Here's the prologue in question...

0x400ef260 <__sigsuspend>:      stmdb   sp!, {r4, r5, r10, lr}
0x400ef264 <__sigsuspend+4>:    mov     r4, r0
0x400ef268 <__sigsuspend+8>:
    ldr r10, [pc, #144] ; 0x400ef300 <__sigsuspend+160>
0x400ef26c <__sigsuspend+12>:
    ldr r3, [pc, #144]  ; 0x400ef304 <__sigsuspend+164>
0x400ef270 <__sigsuspend+16>:   add     r10, pc, r10
0x400ef274 <__sigsuspend+20>:   ldr     r5, [r10, r3]
0x400ef278 <__sigsuspend+24>:   ldr     r2, [r5]
0x400ef27c <__sigsuspend+28>:   cmp     r2, #0  ; 0x0

It turns out that the ARM prologue scanner has two problems with
the above prologue.

First, it does not start with "mov ip, sp".  I've read the current
ABI documents and, while this may have been mandatory at one time,
I can't find any language which requires that this be the first
instruction in the prologue.  I've revised the prologue scanner to
make it optional.

Second, it appears to me that the "no frame pointer" case has never
been tested.  The code which sets the frame offset will incorrectly
compute negative value which leads to some pretty weird behavior when
attempting to do a backtrace.

I'm attaching two versions of the patch.  The first will be of interest
to those of you examining this patch either for correctness or to just
see the salient changes.  It uses the -w switch which causes the whitespace
caused by the (right) shifting of many lines to be ignored.  The second
version of the patch should be used by anyone wanting to actually apply
the patch to the sources.

In the testing I've done, this patch fixes a number of FAILs in
linux-dp.exp.  I haven't seen any regressions.

Okay to commit?

	* arm-tdep.c (arm_scan_prologue): Don't require "mov ip, sp"
	to be the first instruction in the prologue.  Also, revise
	the way the frame offset is computed for frameless functions.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.17
diff -u -w -p -r1.17 arm-tdep.c
--- arm-tdep.c	2001/11/14 08:18:32	1.17
+++ arm-tdep.c	2001/11/27 06:18:25
@@ -797,15 +797,19 @@ arm_scan_prologue (struct frame_info *fi
      traceback.
 
      In the APCS, the prologue should start with  "mov ip, sp" so
-     if we don't see this as the first insn, we will stop.  */
+     if we don't see this as the first insn, we will stop.  [Note:
+     This doesn't seem to be true any longer, so it's now an optional
+     part of the prologue.  - Kevin Buettner, 2001-11-20]  */
 
   sp_offset = fp_offset = 0;
 
   if (read_memory_unsigned_integer (prologue_start, 4)
       == 0xe1a0c00d)		/* mov ip, sp */
-    {
-      for (current_pc = prologue_start + 4; current_pc < prologue_end;
-	   current_pc += 4)
+    current_pc = prologue_start + 4;
+  else
+    current_pc = prologue_start;
+
+  for (; current_pc < prologue_end; current_pc += 4)
 	{
 	  unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
 
@@ -882,13 +886,15 @@ arm_scan_prologue (struct frame_info *fi
 	       so we just skip what we don't recognize. */
 	    continue;
 	}
-    }
 
   /* The frame size is just the negative of the offset (from the original SP)
      of the last thing thing we pushed on the stack.  The frame offset is
      [new FP] - [new SP].  */
   fi->framesize = -sp_offset;
+  if (fi->framereg == FP_REGNUM)
   fi->frameoffset = fp_offset - sp_offset;
+  else
+    fi->frameoffset = 0;
 
   save_prologue_cache (fi);
 }

---- Second version which shows changes due to shifted lines... ----

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.17
diff -u -p -r1.17 arm-tdep.c
--- arm-tdep.c	2001/11/14 08:18:32	1.17
+++ arm-tdep.c	2001/11/27 06:16:07
@@ -797,98 +797,104 @@ arm_scan_prologue (struct frame_info *fi
      traceback.
 
      In the APCS, the prologue should start with  "mov ip, sp" so
-     if we don't see this as the first insn, we will stop.  */
+     if we don't see this as the first insn, we will stop.  [Note:
+     This doesn't seem to be true any longer, so it's now an optional
+     part of the prologue.  - Kevin Buettner, 2001-11-20]  */
 
   sp_offset = fp_offset = 0;
 
   if (read_memory_unsigned_integer (prologue_start, 4)
       == 0xe1a0c00d)		/* mov ip, sp */
+    current_pc = prologue_start + 4;
+  else
+    current_pc = prologue_start;
+
+  for (; current_pc < prologue_end; current_pc += 4)
     {
-      for (current_pc = prologue_start + 4; current_pc < prologue_end;
-	   current_pc += 4)
+      unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
+
+      if ((insn & 0xffff0000) == 0xe92d0000)
+	/* stmfd sp!, {..., fp, ip, lr, pc}
+	   or
+	   stmfd sp!, {a1, a2, a3, a4}  */
 	{
-	  unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
+	  int mask = insn & 0xffff;
 
-	  if ((insn & 0xffff0000) == 0xe92d0000)
-	    /* stmfd sp!, {..., fp, ip, lr, pc}
-	       or
-	       stmfd sp!, {a1, a2, a3, a4}  */
-	    {
-	      int mask = insn & 0xffff;
+	  /* Calculate offsets of saved registers. */
+	  for (regno = PC_REGNUM; regno >= 0; regno--)
+	    if (mask & (1 << regno))
+	      {
+		sp_offset -= 4;
+		fi->fsr.regs[regno] = sp_offset;
+	      }
+	}
+      else if ((insn & 0xfffff000) == 0xe24cb000)	/* sub fp, ip #n */
+	{
+	  unsigned imm = insn & 0xff;	/* immediate value */
+	  unsigned rot = (insn & 0xf00) >> 7;	/* rotate amount */
+	  imm = (imm >> rot) | (imm << (32 - rot));
+	  fp_offset = -imm;
+	  fi->framereg = FP_REGNUM;
+	}
+      else if ((insn & 0xfffff000) == 0xe24dd000)	/* sub sp, sp #n */
+	{
+	  unsigned imm = insn & 0xff;	/* immediate value */
+	  unsigned rot = (insn & 0xf00) >> 7;	/* rotate amount */
+	  imm = (imm >> rot) | (imm << (32 - rot));
+	  sp_offset -= imm;
+	}
+      else if ((insn & 0xffff7fff) == 0xed6d0103)	/* stfe f?, [sp, -#c]! */
+	{
+	  sp_offset -= 12;
+	  regno = F0_REGNUM + ((insn >> 12) & 0x07);
+	  fi->fsr.regs[regno] = sp_offset;
+	}
+      else if ((insn & 0xffbf0fff) == 0xec2d0200)	/* sfmfd f0, 4, [sp!] */
+	{
+	  int n_saved_fp_regs;
+	  unsigned int fp_start_reg, fp_bound_reg;
 
-	      /* Calculate offsets of saved registers. */
-	      for (regno = PC_REGNUM; regno >= 0; regno--)
-		if (mask & (1 << regno))
-		  {
-		    sp_offset -= 4;
-		    fi->fsr.regs[regno] = sp_offset;
-		  }
-	    }
-	  else if ((insn & 0xfffff000) == 0xe24cb000)	/* sub fp, ip #n */
+	  if ((insn & 0x800) == 0x800)	/* N0 is set */
 	    {
-	      unsigned imm = insn & 0xff;	/* immediate value */
-	      unsigned rot = (insn & 0xf00) >> 7;	/* rotate amount */
-	      imm = (imm >> rot) | (imm << (32 - rot));
-	      fp_offset = -imm;
-	      fi->framereg = FP_REGNUM;
+	      if ((insn & 0x40000) == 0x40000)	/* N1 is set */
+		n_saved_fp_regs = 3;
+	      else
+		n_saved_fp_regs = 1;
 	    }
-	  else if ((insn & 0xfffff000) == 0xe24dd000)	/* sub sp, sp #n */
+	  else
 	    {
-	      unsigned imm = insn & 0xff;	/* immediate value */
-	      unsigned rot = (insn & 0xf00) >> 7;	/* rotate amount */
-	      imm = (imm >> rot) | (imm << (32 - rot));
-	      sp_offset -= imm;
+	      if ((insn & 0x40000) == 0x40000)	/* N1 is set */
+		n_saved_fp_regs = 2;
+	      else
+		n_saved_fp_regs = 4;
 	    }
-	  else if ((insn & 0xffff7fff) == 0xed6d0103)	/* stfe f?, [sp, -#c]! */
+
+	  fp_start_reg = F0_REGNUM + ((insn >> 12) & 0x7);
+	  fp_bound_reg = fp_start_reg + n_saved_fp_regs;
+	  for (; fp_start_reg < fp_bound_reg; fp_start_reg++)
 	    {
 	      sp_offset -= 12;
-	      regno = F0_REGNUM + ((insn >> 12) & 0x07);
-	      fi->fsr.regs[regno] = sp_offset;
+	      fi->fsr.regs[fp_start_reg++] = sp_offset;
 	    }
-	  else if ((insn & 0xffbf0fff) == 0xec2d0200)	/* sfmfd f0, 4, [sp!] */
-	    {
-	      int n_saved_fp_regs;
-	      unsigned int fp_start_reg, fp_bound_reg;
-
-	      if ((insn & 0x800) == 0x800)	/* N0 is set */
-		{
-		  if ((insn & 0x40000) == 0x40000)	/* N1 is set */
-		    n_saved_fp_regs = 3;
-		  else
-		    n_saved_fp_regs = 1;
-		}
-	      else
-		{
-		  if ((insn & 0x40000) == 0x40000)	/* N1 is set */
-		    n_saved_fp_regs = 2;
-		  else
-		    n_saved_fp_regs = 4;
-		}
-
-	      fp_start_reg = F0_REGNUM + ((insn >> 12) & 0x7);
-	      fp_bound_reg = fp_start_reg + n_saved_fp_regs;
-	      for (; fp_start_reg < fp_bound_reg; fp_start_reg++)
-		{
-		  sp_offset -= 12;
-		  fi->fsr.regs[fp_start_reg++] = sp_offset;
-		}
-	    }
-	  else if ((insn & 0xf0000000) != 0xe0000000)
-	    break;	/* Condition not true, exit early */
-	  else if ((insn & 0xfe200000) == 0xe8200000) /* ldm? */
-	    break;	/* Don't scan past a block load */
-	  else
-	    /* The optimizer might shove anything into the prologue,
-	       so we just skip what we don't recognize. */
-	    continue;
 	}
+      else if ((insn & 0xf0000000) != 0xe0000000)
+	break;	/* Condition not true, exit early */
+      else if ((insn & 0xfe200000) == 0xe8200000) /* ldm? */
+	break;	/* Don't scan past a block load */
+      else
+	/* The optimizer might shove anything into the prologue,
+	   so we just skip what we don't recognize. */
+	continue;
     }
 
   /* The frame size is just the negative of the offset (from the original SP)
      of the last thing thing we pushed on the stack.  The frame offset is
      [new FP] - [new SP].  */
   fi->framesize = -sp_offset;
-  fi->frameoffset = fp_offset - sp_offset;
+  if (fi->framereg == FP_REGNUM)
+    fi->frameoffset = fp_offset - sp_offset;
+  else
+    fi->frameoffset = 0;
 
   save_prologue_cache (fi);
 }


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