[PATCH 0/2] Add arm epilogue unwinder

[PATCH 0/2] Add arm epilogue unwinder

[Yao Qi]

This patch series adds arm epilogue unwinder so that GDB can unwind
frame successfully when the program is already in epilogue.  This is
quite useful in reverse debugging, because program can be executed
reversely to the epilogue of function, and GDB needs to determine the
"inner than" relationship between frames.

Patch 1 is to refactor arm_stack_frame_destroyed_p, so that some code
can be used for epilogue unwdiner in patch 2.

Regression tested on arm-linux.

*** BLURB HERE ***

Yao Qi (2):
  Refactor arm_stack_frame_destroyed_p
  Add arm epilogue unwinder

 gdb/arm-tdep.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 107 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 1/2] Refactor arm_stack_frame_destroyed_p

[Yao Qi]

This patch is to refactor arm_stack_frame_destroyed_p, so that the code
can be used in both arm_stack_frame_destroyed_p and arm epilogue
unwinder I am going to add in the next patch.  In fact, the code
is the same in two places, but checking whether it is thumb mode
is slightly different.  arm_stack_frame_destroyed_p uses
arm_pc_is_thumb, and epilogue unwinder should use arm_frame_is_thumb.

gdb:

2016-03-24  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (arm_stack_frame_destroyed_p): Rename it ...
	(arm_stack_frame_destroyed_p_1): ... here.  Don't call
	arm_pc_is_thumb.
	(arm_stack_frame_destroyed_p): Call
	thumb_stack_frame_destroyed_p and
	arm_stack_frame_destroyed_p_1.
---
 gdb/arm-tdep.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d4d17f3..afd51f1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3124,19 +3124,14 @@ thumb_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
   return found_stack_adjust;
 }
 
-/* Implement the stack_frame_destroyed_p gdbarch method.  */
-
 static int
-arm_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+arm_stack_frame_destroyed_p_1 (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;
   CORE_ADDR func_start, func_end;
 
-  if (arm_pc_is_thumb (gdbarch, pc))
-    return thumb_stack_frame_destroyed_p (gdbarch, pc);
-
   if (!find_pc_partial_function (pc, NULL, &func_start, &func_end))
     return 0;
 
@@ -3178,6 +3173,16 @@ arm_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
   return 0;
 }
 
+/* Implement the stack_frame_destroyed_p gdbarch method.  */
+
+static int
+arm_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  if (arm_pc_is_thumb (gdbarch, pc))
+    return thumb_stack_frame_destroyed_p (gdbarch, pc);
+  else
+    return arm_stack_frame_destroyed_p_1 (gdbarch, pc);
+}
 
 /* When arguments must be pushed onto the stack, they go on in reverse
    order.  The code below implements a FILO (stack) to do this.  */
-- 
1.9.1

[PATCH 2/2] Add arm epilogue unwinder

[Yao Qi]

Nowadays, GDB can't unwind successfully from epilogue on arm,

 (gdb) bt
 #0  0x76ff65a2 in shr1 () from /home/yao/Source/gnu/build/gdb/testsuite/gdb.reverse/shr1.sl
 #1  0x0000869e in main () at /home/yao/Source/gnu/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:34
 Backtrace stopped: previous frame inner to this frame (corrupt stack?)

(gdb) disassemble shr1
Dump of assembler code for function shr1:
   ....
   0x76ff659a <+10>:	adds	r7, #12
   0x76ff659c <+12>:	mov	sp, r7
   0x76ff659e <+14>:	ldr.w	r7, [sp], #4
   0x76ff65a2 <+18>:	bx	lr
End of assembler dump.

in this case, prologue unwinder is used.  It analyzes the prologue and
get the offsets of saved registers to SP.  However, in epilogue, the
SP has been restored, prologue unwinder gets the registers from the
wrong address, and even the frame id is wrong.

In reverse debugging, this case (program stops at the last instruction
of function) happens quite frequently due to the reverse execution.
There are many test fails due to missing epilogue unwinder.

This adds epilogue unwinder, but the frame cache is still get by
prologue unwinder except that SP is fixed up separately, because SP
is restored in epilogue.

This patch fixes many fails in solib-precsave.exp and solib-reverse.exp.

gdb:

2016-03-24  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c: (arm_make_epilogue_frame_cache): New function.
	(arm_epilogue_frame_this_id): New function.
	(arm_epilogue_frame_prev_register): New function.
	(arm_epilogue_frame_sniffer): New function.
	(arm_epilogue_frame_unwind): New.
	(arm_gdbarch_init): Append unwinder arm_epilogue_frame_unwind.
---
 gdb/arm-tdep.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index afd51f1..030d979 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -2669,6 +2669,101 @@ struct frame_unwind arm_exidx_unwind = {
   arm_exidx_unwind_sniffer
 };
 
+static struct arm_prologue_cache *
+arm_make_epilogue_frame_cache (struct frame_info *this_frame)
+{
+  struct arm_prologue_cache *cache;
+  CORE_ADDR sp;
+  int reg;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
+  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+
+  /* Still rely on the offset calculated from prologue.  */
+  arm_scan_prologue (this_frame, cache);
+
+  /* Since we are in epilogue, the SP has been restored.  */
+  cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+
+  /* Calculate actual addresses of saved registers using offsets
+     determined by arm_scan_prologue.  */
+  for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
+    if (trad_frame_addr_p (cache->saved_regs, reg))
+      cache->saved_regs[reg].addr += cache->prev_sp;
+
+  return cache;
+}
+
+static void
+arm_epilogue_frame_this_id (struct frame_info *this_frame,
+			    void **this_cache,
+			    struct frame_id *this_id)
+{
+  struct arm_prologue_cache *cache;
+  CORE_ADDR pc, func;
+
+  if (*this_cache == NULL)
+    *this_cache = arm_make_epilogue_frame_cache (this_frame);
+  cache = (struct arm_prologue_cache *) *this_cache;
+
+  /* Use function start address as part of the frame ID.  If we cannot
+     identify the start address (due to missing symbol information),
+     fall back to just using the current PC.  */
+  pc = get_frame_pc (this_frame);
+  func = get_frame_func (this_frame);
+  if (!func)
+    func = pc;
+
+  (*this_id) = frame_id_build (cache->prev_sp, pc);
+}
+
+static struct value *
+arm_epilogue_frame_prev_register (struct frame_info *this_frame,
+				  void **this_cache, int regnum)
+{
+  struct arm_prologue_cache *cache;
+
+  if (*this_cache == NULL)
+    *this_cache = arm_make_epilogue_frame_cache (this_frame);
+  cache = (struct arm_prologue_cache *) *this_cache;
+
+  return arm_prologue_prev_register (this_frame, this_cache, regnum);
+}
+
+static int arm_stack_frame_destroyed_p_1 (struct gdbarch *gdbarch,
+					  CORE_ADDR pc);
+static int thumb_stack_frame_destroyed_p (struct gdbarch *gdbarch,
+					  CORE_ADDR pc);
+
+static int
+arm_epilogue_frame_sniffer (const struct frame_unwind *self,
+			    struct frame_info *this_frame,
+			    void **this_prologue_cache)
+{
+  if (frame_relative_level (this_frame) == 0)
+    {
+      struct gdbarch *gdbarch = get_frame_arch (this_frame);
+      CORE_ADDR pc = get_frame_pc (this_frame);
+
+      if (arm_frame_is_thumb (this_frame))
+	return thumb_stack_frame_destroyed_p (gdbarch, pc);
+      else
+	return arm_stack_frame_destroyed_p_1 (gdbarch, pc);
+    }
+  else
+    return 0;
+}
+
+static const struct frame_unwind arm_epilogue_frame_unwind =
+{
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  arm_epilogue_frame_this_id,
+  arm_epilogue_frame_prev_register,
+  NULL,
+  arm_epilogue_frame_sniffer,
+};
+
 /* Recognize GCC's trampoline for thumb call-indirect.  If we are in a
    trampoline, return the target PC.  Otherwise return 0.
 
@@ -9281,6 +9376,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   frame_unwind_append_unwinder (gdbarch, &arm_stub_unwind);
   dwarf2_append_unwinders (gdbarch);
   frame_unwind_append_unwinder (gdbarch, &arm_exidx_unwind);
+  frame_unwind_append_unwinder (gdbarch, &arm_epilogue_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &arm_prologue_unwind);
 
   /* Now we have tuned the configuration, set a few final things,
-- 
1.9.1

Re: [PATCH 2/2] Add arm epilogue unwinder

[Pedro Alves]

LGTM. A couple super minor nits below.

On 03/24/2016 02:39 PM, Yao Qi wrote:

> +static struct arm_prologue_cache *
> +arm_make_epilogue_frame_cache (struct frame_info *this_frame)
> +{

Missing intro comments in several functions.

> +  pc = get_frame_pc (this_frame);
> +  func = get_frame_func (this_frame);
> +  if (!func)

func == NULL.

Thanks,
Pedro Alves

Re: [PATCH 2/2] Add arm epilogue unwinder

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Missing intro comments in several functions.
>
>> +  pc = get_frame_pc (this_frame);
>> +  func = get_frame_func (this_frame);
>> +  if (!func)
>
> func == NULL.

Both are fixed.  Patch below is what I pushed in.

-- 
Yao (齐尧)
From 779aa56f2c160ef508ca98fac1ffd23cad6fc63f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Wed, 30 Mar 2016 16:44:24 +0100
Subject: [PATCH] Add arm epilogue unwinder

Nowadays, GDB can't unwind successfully from epilogue on arm,

 (gdb) bt
 #0  0x76ff65a2 in shr1 () from /home/yao/Source/gnu/build/gdb/testsuite/gdb.reverse/shr1.sl
 #1  0x0000869e in main () at /home/yao/Source/gnu/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.reverse/solib-reverse.c:34
 Backtrace stopped: previous frame inner to this frame (corrupt stack?)

(gdb) disassemble shr1
Dump of assembler code for function shr1:
   ....
   0x76ff659a <+10>:	adds	r7, #12
   0x76ff659c <+12>:	mov	sp, r7
   0x76ff659e <+14>:	ldr.w	r7, [sp], #4
   0x76ff65a2 <+18>:	bx	lr
End of assembler dump.

in this case, prologue unwinder is used.  It analyzes the prologue and
get the offsets of saved registers to SP.  However, in epilogue, the
SP has been restored, prologue unwinder gets the registers from the
wrong address, and even the frame id is wrong.

In reverse debugging, this case (program stops at the last instruction
of function) happens quite frequently due to the reverse execution.
There are many test fails due to missing epilogue unwinder.

This adds epilogue unwinder, but the frame cache is still get by
prologue unwinder except that SP is fixed up separately, because SP
is restored in epilogue.

This patch fixes many fails in solib-precsave.exp, and solib-reverse.exp.

gdb:

2016-03-30  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c: (arm_make_epilogue_frame_cache): New function.
	(arm_epilogue_frame_this_id): New function.
	(arm_epilogue_frame_prev_register): New function.
	(arm_epilogue_frame_sniffer): New function.
	(arm_epilogue_frame_unwind): New.
	(arm_gdbarch_init): Append unwinder arm_epilogue_frame_unwind.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index efd9da6..bb9b974 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2016-03-30  Yao Qi  <yao.qi@linaro.org>
 
+	* arm-tdep.c: (arm_make_epilogue_frame_cache): New function.
+	(arm_epilogue_frame_this_id): New function.
+	(arm_epilogue_frame_prev_register): New function.
+	(arm_epilogue_frame_sniffer): New function.
+	(arm_epilogue_frame_unwind): New.
+	(arm_gdbarch_init): Append unwinder arm_epilogue_frame_unwind.
+
+2016-03-30  Yao Qi  <yao.qi@linaro.org>
+
 	* arm-tdep.c (arm_stack_frame_destroyed_p): Rename it ...
 	(arm_stack_frame_destroyed_p_1): ... here.  Don't call
 	arm_pc_is_thumb.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 6ede3a9..01f53d6 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -2669,6 +2669,112 @@ struct frame_unwind arm_exidx_unwind = {
   arm_exidx_unwind_sniffer
 };
 
+static struct arm_prologue_cache *
+arm_make_epilogue_frame_cache (struct frame_info *this_frame)
+{
+  struct arm_prologue_cache *cache;
+  CORE_ADDR sp;
+  int reg;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
+  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+
+  /* Still rely on the offset calculated from prologue.  */
+  arm_scan_prologue (this_frame, cache);
+
+  /* Since we are in epilogue, the SP has been restored.  */
+  cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+
+  /* Calculate actual addresses of saved registers using offsets
+     determined by arm_scan_prologue.  */
+  for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
+    if (trad_frame_addr_p (cache->saved_regs, reg))
+      cache->saved_regs[reg].addr += cache->prev_sp;
+
+  return cache;
+}
+
+/* Implementation of function hook 'this_id' in
+   'struct frame_uwnind' for epilogue unwinder.  */
+
+static void
+arm_epilogue_frame_this_id (struct frame_info *this_frame,
+			    void **this_cache,
+			    struct frame_id *this_id)
+{
+  struct arm_prologue_cache *cache;
+  CORE_ADDR pc, func;
+
+  if (*this_cache == NULL)
+    *this_cache = arm_make_epilogue_frame_cache (this_frame);
+  cache = (struct arm_prologue_cache *) *this_cache;
+
+  /* Use function start address as part of the frame ID.  If we cannot
+     identify the start address (due to missing symbol information),
+     fall back to just using the current PC.  */
+  pc = get_frame_pc (this_frame);
+  func = get_frame_func (this_frame);
+  if (func == NULL)
+    func = pc;
+
+  (*this_id) = frame_id_build (cache->prev_sp, pc);
+}
+
+/* Implementation of function hook 'prev_register' in
+   'struct frame_uwnind' for epilogue unwinder.  */
+
+static struct value *
+arm_epilogue_frame_prev_register (struct frame_info *this_frame,
+				  void **this_cache, int regnum)
+{
+  struct arm_prologue_cache *cache;
+
+  if (*this_cache == NULL)
+    *this_cache = arm_make_epilogue_frame_cache (this_frame);
+  cache = (struct arm_prologue_cache *) *this_cache;
+
+  return arm_prologue_prev_register (this_frame, this_cache, regnum);
+}
+
+static int arm_stack_frame_destroyed_p_1 (struct gdbarch *gdbarch,
+					  CORE_ADDR pc);
+static int thumb_stack_frame_destroyed_p (struct gdbarch *gdbarch,
+					  CORE_ADDR pc);
+
+/* Implementation of function hook 'sniffer' in
+   'struct frame_uwnind' for epilogue unwinder.  */
+
+static int
+arm_epilogue_frame_sniffer (const struct frame_unwind *self,
+			    struct frame_info *this_frame,
+			    void **this_prologue_cache)
+{
+  if (frame_relative_level (this_frame) == 0)
+    {
+      struct gdbarch *gdbarch = get_frame_arch (this_frame);
+      CORE_ADDR pc = get_frame_pc (this_frame);
+
+      if (arm_frame_is_thumb (this_frame))
+	return thumb_stack_frame_destroyed_p (gdbarch, pc);
+      else
+	return arm_stack_frame_destroyed_p_1 (gdbarch, pc);
+    }
+  else
+    return 0;
+}
+
+/* Frame unwinder from epilogue.  */
+
+static const struct frame_unwind arm_epilogue_frame_unwind =
+{
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  arm_epilogue_frame_this_id,
+  arm_epilogue_frame_prev_register,
+  NULL,
+  arm_epilogue_frame_sniffer,
+};
+
 /* Recognize GCC's trampoline for thumb call-indirect.  If we are in a
    trampoline, return the target PC.  Otherwise return 0.
 
@@ -9285,6 +9391,7 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   frame_unwind_append_unwinder (gdbarch, &arm_stub_unwind);
   dwarf2_append_unwinders (gdbarch);
   frame_unwind_append_unwinder (gdbarch, &arm_exidx_unwind);
+  frame_unwind_append_unwinder (gdbarch, &arm_epilogue_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &arm_prologue_unwind);
 
   /* Now we have tuned the configuration, set a few final things,