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]

Re: [rfa] Fix software-watchpoint failures by adding epilogue detection


I wrote:
> Dan Jacobowitz wrote:
> 
> > One of the patches we haven't had a chance to submit took the same
> > approach to solve these failures.  I also did ARM mode, as well as
> > some additional sequences - probably those used by RealView.  I've
> > attached it.
> 
> Thanks!  Unfortunately it doesn't fix the problem with our current
> ARM compiler, because your heuristics doesn't recognize the sequence
> we get as epilogue:
> 
>     83c8:       f107 0710       add.w   r7, r7, #16
>     83cc:       46bd            mov     sp, r7
>     83ce:       bd80            pop     {r7, pc}
> 
> On the other hand, your patch does indeed accept a couple of additonal
> instructions compared to mine.  I'll come up with a merged approach ...

I've compared the Thumb cases of both patches with the following results:

1. Some epilogue sequences accepted by your patch were not accepted by mine:
- My patch only accepted "bx lr" as return, while yours accepts any "bx".
- I had a typo in one of the instruction opcodes.

2. Some epilogue sequences accepted by my patch were not accepted by yours:
- I'm allowing "mov sp, r7" and "vldm" instructions, as well as certain
  additional cases of "ldm.w".
- I'm accepting more diverse sequences due to forward-scanning for multiple
  instructions, and not requiring backward-scanning.

I've now merged the two by basically keeping with the main structure of my
patch for Thumb, and fixing the two issues identified under 1.

For ARM, I'm simply using your patch as-is; I've confirmed that it fixes
all the same testsuite failures when running with -marm as the Thumb patch
fixes with -mthumb.   The more limited scanning doesn't appear to hurt for
ARM as the epilogues tend to be very short anyway.


Retested with no regressions on armv7l-linux-gnueabi both with -marm and
with (implied) -mthumb.

Does this look good to you?

Bye,
Ulrich


2010-09-24  Ulrich Weigand  <uweigand@de.ibm.com>
            Daniel Jacobowitz  <dan@codesourcery.com>

	* arm-tdep.c (thumb_in_function_epilogue_p)
	(arm_in_function_epilogue_p): New.
	(arm_gdbarch_init): Install arm_in_function_epilogue_p as
	gdbarch_in_function_epilogue_p callback.


Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.307
diff -u -p -r1.307 arm-tdep.c
--- gdb/arm-tdep.c	30 Aug 2010 15:26:28 -0000	1.307
+++ gdb/arm-tdep.c	23 Sep 2010 18:43:29 -0000
@@ -1706,6 +1706,150 @@ arm_dwarf2_frame_init_reg (struct gdbarc
     }
 }
 
+/* Return 1 if the stack frame has already been destroyed by the
+   function epilogue.  */
+
+static int
+thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+
+  /* The epilogue is a sequence of instructions along the following lines:
+
+    - [optionally] set SP from the frame pointer
+    - restore registers from SP [may include PC]
+    - a return-type instruction [if PC wasn't already restored]
+
+    If, scanning forward from the current PC, the instructions we find
+    are compatible with this sequence (ending in a return), we consider
+    the current PC to be part of the epilogue.  */
+
+  for (;;)
+    {
+      unsigned int insn;
+      gdb_byte buf[2];
+
+      if (target_read_memory (pc, buf, 2))
+	return 0;
+
+      pc += 2;
+      insn = extract_unsigned_integer (buf, 2, byte_order_for_code);
+
+      if ((insn & 0xff80) == 0x4700)  /* bx <Rm> */
+	return 1;
+      else if (insn == 0x46f7)  /* mov pc, lr */
+	return 1;
+      else if (insn == 0x46bd)  /* mov sp, r7 */
+	;
+      else if ((insn & 0xfe00) == 0xbc00)  /* pop <registers> */
+	{
+	  if (insn & 0x0100)  /* <registers> include PC.  */
+	    return 1;
+	}
+      else if ((insn & 0xe000) == 0xe000)  /* 32-bit Thumb-2 instruction */
+	{
+	  unsigned int insn2;
+
+	  if (target_read_memory (pc, buf, 2))
+	    return 0;
+
+	  pc += 2;
+	  insn2 = extract_unsigned_integer (buf, 2, byte_order_for_code);
+
+	  if ((insn & 0xffdf) == 0xe89d)  /* ldm.w sp{!}, <registers> */
+	    {
+	      if (insn2 & 0x8000)  /* <registers> include PC.  */
+		return 1;
+	    }
+	  else if (insn == 0xf85d  /* ldr.w <Rt>, [sp], #4 */
+		   && (insn2 & 0x0fff) == 0x0b04)
+	    {
+	      if ((insn2 & 0xf000) == 0xf000) /* <Rt> is PC.  */
+		return 1;
+	    }
+	  else if ((insn & 0xffbf) == 0xecbd  /* vldm sp!, <list> */
+		   && (insn2 & 0x0e00) == 0x0a00)
+	    ;
+	  else
+	    return 0;
+	}
+      else
+	return 0;
+    }
+}
+
+/* Return 1 if the stack frame has already been destroyed by the
+   function epilogue.  */
+
+static int
+arm_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+  unsigned int insn;
+  int found_return, found_stack_adjust;
+  CORE_ADDR func_start, func_end;
+
+  if (arm_pc_is_thumb (gdbarch, pc))
+    return thumb_in_function_epilogue_p (gdbarch, pc);
+
+  if (!find_pc_partial_function (pc, NULL, &func_start, &func_end))
+    return 0;
+
+  /* We are in the epilogue if the previous instruction was a stack
+     adjustment and the next instruction is a possible return (bx, mov
+     pc, or pop).  We could have to scan backwards to find the stack
+     adjustment, or forwards to find the return, but this is a decent
+     approximation.  First scan forwards.  */
+
+  found_return = 0;
+  insn = read_memory_unsigned_integer (pc, 4, byte_order_for_code);
+  if (bits (insn, 28, 31) != INST_NV)
+    {
+      if ((insn & 0x0ffffff0) == 0x012fff10)
+	/* BX.  */
+	found_return = 1;
+      else if ((insn & 0x0ffffff0) == 0x01a0f000)
+	/* MOV PC.  */
+	found_return = 1;
+      else if ((insn & 0x0fff0000) == 0x08bd0000
+	  && (insn & 0x0000c000) != 0)
+	/* POP (LDMIA), including PC or LR.  */
+	found_return = 1;
+    }
+
+  if (!found_return)
+    return 0;
+
+  /* Scan backwards.  This is just a heuristic, so do not worry about
+     false positives from mode changes.  */
+
+  if (pc < func_start + 4)
+    return 0;
+
+  insn = read_memory_unsigned_integer (pc - 4, 4, byte_order_for_code);
+  if (bits (insn, 28, 31) != INST_NV)
+    {
+      if ((insn & 0x0df0f000) == 0x0080d000)
+	/* ADD SP (register or immediate).  */
+	found_stack_adjust = 1;
+      else if ((insn & 0x0df0f000) == 0x0040d000)
+	/* SUB SP (register or immediate).  */
+	found_stack_adjust = 1;
+      else if ((insn & 0x0ffffff0) == 0x01a0d000)
+	/* MOV SP.  */
+	found_return = 1;
+      else if ((insn & 0x0fff0000) == 0x08bd0000)
+	/* POP (LDMIA).  */
+	found_stack_adjust = 1;
+    }
+
+  if (found_stack_adjust)
+    return 1;
+
+  return 0;
+}
+
+
 /* When arguments must be pushed onto the stack, they go on in reverse
    order.  The code below implements a FILO (stack) to do this.  */
 
@@ -6881,6 +7025,9 @@ arm_gdbarch_init (struct gdbarch_info in
   /* Advance PC across function entry code.  */
   set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue);
 
+  /* Detect whether PC is in function epilogue.  */
+  set_gdbarch_in_function_epilogue_p (gdbarch, arm_in_function_epilogue_p);
+
   /* Skip trampolines.  */
   set_gdbarch_skip_trampoline_code (gdbarch, arm_skip_stub);
 

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


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