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

[Daniel Jacobowitz <dan@codesourcery.com>]

On Wed, Sep 22, 2010 at 08:46:47PM +0200, Ulrich Weigand wrote:
> Hello,
> 
> I've been seeing failures in software watchpoint tests on ARM (Thumb-2),
> due to missing modeling of precise unwind states during function
> epilogues.  This problem is well known from other architectures,
> and is usually fixed by skipping epilogue during sofware watchpoint
> single-stepping via the gdbarch_in_function_epilogue_p callback.
> 
> However, ARM currently does not define this routine.  The following
> patch adds an implementation that detects common Thumb epilogue
> sequences (ARM is missing, but could be added in an analogous way).

Hi Ulrich,

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.

We have at least one other patch related to ARM testsuite failures,
specifically an ERROR (probably infinite backtrace) in gdb1250.exp.
I can extract it for you if you'd like, but I'm not sure it's in
condition to be merged.

I haven't retested this patch against HEAD, but it's in all our
builds.

-- 
Daniel Jacobowitz
CodeSourcery

2010-07-02  Daniel Jacobowitz  <dan@codesourcery.com>

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

--- /net/henry4/scratch/gcc/src/gdb-mainline/gdb/arm-tdep.c	2010-08-30 10:47:04.000000000 -0700
+++ gdb/arm-tdep.c	2010-09-13 12:32:50.000000000 -0700
@@ -1706,6 +1815,168 @@ arm_dwarf2_frame_init_reg (struct gdbarc
     }
 }
 
+/* Return the size in bytes of the complete Thumb instruction whose
+   first halfword is INST1.  */
+
+static int
+thumb_insn_size (unsigned short inst1)
+{
+  if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
+    return 4;
+  else
+    return 2;
+}
+
+/* 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);
+  unsigned int inst1, inst2;
+  int found_return, found_stack_adjust;
+  CORE_ADDR func_start, func_end;
+
+  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;
+  inst1 = read_memory_unsigned_integer (pc, 2, byte_order_for_code);
+
+  if ((inst1 & 0xff00) == 0xbc00 && pc + 2 < func_end)
+    /* POP (without PC).  Potentially loading the return address for BX.  */
+    inst1 = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
+
+  if ((inst1 & 0xff80) == 0x4700)
+    /* BX lr is typically used for returns.  */
+    found_return = 1;
+  else if ((inst1 & 0xff00) == 0xbd00)
+    /* POP, including PC.  */
+    found_return = 1;
+  else if (thumb_insn_size (inst1) == 4)
+    {
+      inst2 = read_memory_unsigned_integer (pc, 2, byte_order_for_code);
+
+      if (inst1 == 0xe8bd && (inst2 & 0x8000) == 0x8000)
+	/* POP.W, including PC.  */
+	found_return = 1;
+      else if (inst1 == 0xf85d && inst2 == 0xfb04)
+	/* POP.W, only PC.  */
+	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 or 16-bit vs 32-bit instructions.  */
+
+  if (pc < func_start + 4)
+    return 0;
+
+  found_stack_adjust = 0;
+  inst1 = read_memory_unsigned_integer (pc - 4, 2, byte_order_for_code);
+  inst2 = read_memory_unsigned_integer (pc - 2, 2, byte_order_for_code);
+  if ((inst2 & 0xff00) == 0xbc00)
+    /* POP (without PC).  */
+    found_stack_adjust = 1;
+  else if (inst1 == 0xe8bd)
+    /* POP.W (register list).  */
+    found_stack_adjust = 1;
+  else if (inst1 == 0xf85d && (inst2 & 0x0fff) == 0x0b04)
+    /* POP.W (one register).  */
+    found_stack_adjust = 1;
+  else if ((inst2 & 0xff00) == 0xb000)
+    /* ADD SP, imm or SUB SP, imm.  */
+    found_stack_adjust = 1;
+
+  /* There are many other stack adjusting instructions which could be
+     recognized here.  */
+
+  if (found_stack_adjust)
+    return 1;
+
+  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.  */
 
@@ -2647,18 +2918,6 @@ bitcount (unsigned long val)
   return nbits;
 }
 
-/* Return the size in bytes of the complete Thumb instruction whose
-   first halfword is INST1.  */
-
-static int
-thumb_insn_size (unsigned short inst1)
-{
-  if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
-    return 4;
-  else
-    return 2;
-}
-
 static int
 thumb_advance_itstate (unsigned int itstate)
 {
@@ -6881,6 +7160,7 @@ arm_gdbarch_init (struct gdbarch_info in
   /* Advance PC across function entry code.  */
   set_gdbarch_skip_prologue (gdbarch, arm_skip_prologue);
 
+  set_gdbarch_in_function_epilogue_p (gdbarch, arm_in_function_epilogue_p);
   /* Skip trampolines.  */
   set_gdbarch_skip_trampoline_code (gdbarch, arm_skip_stub);