[binutils-gdb] Skip unwritable frames in command "finish"

[Yao Qi <qiyao at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7eb895307f53af3435440d3fe67c0e4e679d99b2

commit 7eb895307f53af3435440d3fe67c0e4e679d99b2
Author: Yao Qi <yao.qi@arm.com>
Date:   Mon May 23 17:32:56 2016 +0100

    Skip unwritable frames in command "finish"
    
    Nowadays, GDB can't insert breakpoint on the return address of the
    exception handler on ARM M-profile, because the address is a magic
    one 0xfffffff9,
    
     (gdb) bt
     #0  CT32B1_IRQHandler () at ../src/timer.c:67
     #1  <signal handler called>
     #2  main () at ../src/timer.c:127
    
    (gdb) info frame
    Stack level 0, frame at 0x200ffa8:
     pc = 0x4ec in CT32B1_IRQHandler (../src/timer.c:67); saved pc = 0xfffffff9
     called by frame at 0x200ffc8
     source language c.
     Arglist at 0x200ffa0, args:
     Locals at 0x200ffa0, Previous frame's sp is 0x200ffa8
     Saved registers:
      r7 at 0x200ffa0, lr at 0x200ffa4
    
    (gdb) x/x 0xfffffff9
    0xfffffff9:     Cannot access memory at address 0xfffffff9
    
    (gdb) finish
    Run till exit from #0  CT32B1_IRQHandler () at ../src/timer.c:67
    Ed:15: Target error from Set break/watch: Et:96: Pseudo-address (0xFFFFFFxx) for EXC_RETURN is invalid (GDB error?)
    
    Warning:
    Cannot insert hardware breakpoint 0.
    Could not insert hardware breakpoints:
    You may have requested too many hardware breakpoints/watchpoints.
    
    Command aborted.
    
    even some debug probe can't set hardware breakpoint on the magic
    address too,
    
    (gdb) hbreak *0xfffffff9
    Hardware assisted breakpoint 2 at 0xfffffff9
    (gdb) c
    Continuing.
    Ed:15: Target error from Set break/watch: Et:96: Pseudo-address (0xFFFFFFxx) for EXC_RETURN is invalid (GDB error?)
    
    Warning:
    Cannot insert hardware breakpoint 2.
    Could not insert hardware breakpoints:
    You may have requested too many hardware breakpoints/watchpoints.
    
    Command aborted.
    
    The problem described above is quite similar to PR 8841, in which GDB
    can't set breakpoint on signal trampoline, which is mapped to a read-only
    page by kernel.  The rationale of this patch is to skip "unwritable"
    frames when looking for caller frames in command "finish", and a new
    gdbarch method code_of_frame_writable is added.  This patch fixes
    the problem on ARM cortex-m target, but it can be used to fix
    PR 8841 too.
    
    gdb:
    
    2016-05-10  Yao Qi  <yao.qi@arm.com>
    
    	* arch-utils.c (default_code_of_frame_writable): New function.
    	* arch-utils.h (default_code_of_frame_writable): Declare.
    	* arm-tdep.c (arm_code_of_frame_writable): New function.
    	(arm_gdbarch_init): Install gdbarch method
    	code_of_frame_writable if the target is M-profile.
    	* frame.c (skip_unwritable_frames): New function.
    	* frame.h (skip_unwritable_frames): Declare.
    	* gdbarch.sh (code_of_frame_writable): New.
    	* gdbarch.c, gdbarch.h: Re-generated.
    	* infcmd.c (finish_command): Call skip_unwritable_frames.

Diff:
---
 gdb/ChangeLog    | 13 +++++++++++++
 gdb/arch-utils.c |  7 +++++++
 gdb/arch-utils.h |  3 +++
 gdb/arm-tdep.c   | 19 +++++++++++++++++++
 gdb/frame.c      | 13 +++++++++++++
 gdb/frame.h      |  5 +++++
 gdb/gdbarch.c    | 23 +++++++++++++++++++++++
 gdb/gdbarch.h    |  6 ++++++
 gdb/gdbarch.sh   |  3 +++
 gdb/infcmd.c     |  2 ++
 10 files changed, 94 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 07bc5d2..680a7df 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2016-05-23  Yao Qi  <yao.qi@arm.com>
+
+	* arch-utils.c (default_code_of_frame_writable): New function.
+	* arch-utils.h (default_code_of_frame_writable): Declare.
+	* arm-tdep.c (arm_code_of_frame_writable): New function.
+	(arm_gdbarch_init): Install gdbarch method
+	code_of_frame_writable if the target is M-profile.
+	* frame.c (skip_unwritable_frames): New function.
+	* frame.h (skip_unwritable_frames): Declare.
+	* gdbarch.sh (code_of_frame_writable): New.
+	* gdbarch.c, gdbarch.h: Re-generated.
+	* infcmd.c (finish_command): Call skip_unwritable_frames.
+
 2016-05-23  Tom Tromey  <tom@tromey.com>
 
 	PR python/19438, PR python/18393:
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c3d7802..604042f 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -132,6 +132,13 @@ generic_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
   return 0;
 }
 
+int
+default_code_of_frame_writable (struct gdbarch *gdbarch,
+				struct frame_info *frame)
+{
+  return 1;
+}
+
 /* Helper functions for gdbarch_inner_than */
 
 int
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 9e1e70e..ad3f126 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -107,6 +107,9 @@ extern int generic_in_solib_return_trampoline (struct gdbarch *gdbarch,
 extern int generic_stack_frame_destroyed_p (struct gdbarch *gdbarch,
 					    CORE_ADDR pc);
 
+extern int default_code_of_frame_writable (struct gdbarch *gdbarch,
+					   struct frame_info *frame);
+
 /* By default, registers are not convertible.  */
 extern int generic_convert_register_p (struct gdbarch *gdbarch, int regnum,
 				       struct type *type);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 157f8a1..278f639 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8862,6 +8862,22 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
   /* Otherwise we don't have a useful guess.  */
 }
 
+/* Implement the code_of_frame_writable gdbarch method.  */
+
+static int
+arm_code_of_frame_writable (struct gdbarch *gdbarch, struct frame_info *frame)
+{
+  if (gdbarch_tdep (gdbarch)->is_m
+      && get_frame_type (frame) == SIGTRAMP_FRAME)
+    {
+      /* M-profile exception frames return to some magic PCs, where
+	 isn't writable at all.  */
+      return 0;
+    }
+  else
+    return 1;
+}
+
 
 /* Initialize the current architecture based on INFO.  If possible,
    re-use an architecture from ARCHES, which is a list of
@@ -9312,6 +9328,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_push_dummy_call (gdbarch, arm_push_dummy_call);
   set_gdbarch_frame_align (gdbarch, arm_frame_align);
 
+  if (is_m)
+    set_gdbarch_code_of_frame_writable (gdbarch, arm_code_of_frame_writable);
+
   set_gdbarch_write_pc (gdbarch, arm_write_pc);
 
   /* Frame handling.  */
diff --git a/gdb/frame.c b/gdb/frame.c
index d621dd7..c25ce4c 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -444,6 +444,19 @@ skip_artificial_frames (struct frame_info *frame)
   return frame;
 }
 
+struct frame_info *
+skip_unwritable_frames (struct frame_info *frame)
+{
+  while (gdbarch_code_of_frame_writable (get_frame_arch (frame), frame) == 0)
+    {
+      frame = get_prev_frame (frame);
+      if (frame == NULL)
+	break;
+    }
+
+  return frame;
+}
+
 /* See frame.h.  */
 
 struct frame_info *
diff --git a/gdb/frame.h b/gdb/frame.h
index 8ee64f1..5f21bb8 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -826,4 +826,9 @@ extern enum language get_frame_language (struct frame_info *frame);
 
 extern struct frame_info *skip_tailcall_frames (struct frame_info *frame);
 
+/* Return the first frame above FRAME or FRAME of which the code is
+   writable.  */
+
+extern struct frame_info *skip_unwritable_frames (struct frame_info *frame);
+
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..313502b 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -205,6 +205,7 @@ struct gdbarch
   gdbarch_push_dummy_call_ftype *push_dummy_call;
   int call_dummy_location;
   gdbarch_push_dummy_code_ftype *push_dummy_code;
+  gdbarch_code_of_frame_writable_ftype *code_of_frame_writable;
   gdbarch_print_registers_info_ftype *print_registers_info;
   gdbarch_print_float_info_ftype *print_float_info;
   gdbarch_print_vector_info_ftype *print_vector_info;
@@ -387,6 +388,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->dwarf2_reg_to_regnum = no_op_reg_to_regnum;
   gdbarch->deprecated_fp_regnum = -1;
   gdbarch->call_dummy_location = AT_ENTRY_POINT;
+  gdbarch->code_of_frame_writable = default_code_of_frame_writable;
   gdbarch->print_registers_info = default_print_registers_info;
   gdbarch->print_float_info = default_print_float_info;
   gdbarch->register_sim_regno = legacy_register_sim_regno;
@@ -552,6 +554,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of push_dummy_call, has predicate.  */
   /* Skip verify of call_dummy_location, invalid_p == 0 */
   /* Skip verify of push_dummy_code, has predicate.  */
+  /* Skip verify of code_of_frame_writable, invalid_p == 0 */
   /* Skip verify of print_registers_info, invalid_p == 0 */
   /* Skip verify of print_float_info, invalid_p == 0 */
   /* Skip verify of print_vector_info, has predicate.  */
@@ -804,6 +807,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: char_signed = %s\n",
                       plongest (gdbarch->char_signed));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: code_of_frame_writable = <%s>\n",
+                      host_address_to_string (gdbarch->code_of_frame_writable));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: coff_make_msymbol_special = <%s>\n",
                       host_address_to_string (gdbarch->coff_make_msymbol_special));
   fprintf_unfiltered (file,
@@ -2314,6 +2320,23 @@ set_gdbarch_push_dummy_code (struct gdbarch *gdbarch,
   gdbarch->push_dummy_code = push_dummy_code;
 }
 
+int
+gdbarch_code_of_frame_writable (struct gdbarch *gdbarch, struct frame_info *frame)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->code_of_frame_writable != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_code_of_frame_writable called\n");
+  return gdbarch->code_of_frame_writable (gdbarch, frame);
+}
+
+void
+set_gdbarch_code_of_frame_writable (struct gdbarch *gdbarch,
+                                    gdbarch_code_of_frame_writable_ftype code_of_frame_writable)
+{
+  gdbarch->code_of_frame_writable = code_of_frame_writable;
+}
+
 void
 gdbarch_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 859ba85..a6366fc 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -395,6 +395,12 @@ typedef CORE_ADDR (gdbarch_push_dummy_code_ftype) (struct gdbarch *gdbarch, CORE
 extern CORE_ADDR gdbarch_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache);
 extern void set_gdbarch_push_dummy_code (struct gdbarch *gdbarch, gdbarch_push_dummy_code_ftype *push_dummy_code);
 
+/* Return true if the code of FRAME is writable. */
+
+typedef int (gdbarch_code_of_frame_writable_ftype) (struct gdbarch *gdbarch, struct frame_info *frame);
+extern int gdbarch_code_of_frame_writable (struct gdbarch *gdbarch, struct frame_info *frame);
+extern void set_gdbarch_code_of_frame_writable (struct gdbarch *gdbarch, gdbarch_code_of_frame_writable_ftype *code_of_frame_writable);
+
 typedef void (gdbarch_print_registers_info_ftype) (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all);
 extern void gdbarch_print_registers_info (struct gdbarch *gdbarch, struct ui_file *file, struct frame_info *frame, int regnum, int all);
 extern void set_gdbarch_print_registers_info (struct gdbarch *gdbarch, gdbarch_print_registers_info_ftype *print_registers_info);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index c8787c2..f170c10 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -485,6 +485,9 @@ M:CORE_ADDR:push_dummy_call:struct value *function, struct regcache *regcache, C
 v:int:call_dummy_location::::AT_ENTRY_POINT::0
 M:CORE_ADDR:push_dummy_code:CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache:sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache
 
+# Return true if the code of FRAME is writable.
+m:int:code_of_frame_writable:struct frame_info *frame:frame::default_code_of_frame_writable::0
+
 m:void:print_registers_info:struct ui_file *file, struct frame_info *frame, int regnum, int all:file, frame, regnum, all::default_print_registers_info::0
 m:void:print_float_info:struct ui_file *file, struct frame_info *frame, const char *args:file, frame, args::default_print_float_info::0
 M:void:print_vector_info:struct ui_file *file, struct frame_info *frame, const char *args:file, frame, args
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 51fef85..14d51fd 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2029,6 +2029,8 @@ finish_command (char *arg, int from_tty)
 	 entering THISFRAME.  */
       frame = skip_tailcall_frames (frame);
 
+      frame = skip_unwritable_frames (frame);
+
       if (frame == NULL)
 	error (_("Cannot find the caller frame."));