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]

RFC: fix PR backtrace/15558


PR backtrace/15558 concerns an assertion failure in gdb that can be
triggered by setting "backtrace limit" to a value that causes gdb to
stop unwinding in the middle of a series of inlined frames.  In this
case, an assertion in inline_frame_this_id will trigger.

The bug is essentially that get_prev_frame checks backtrace_limit.
However, this is not needed or desirable in most cases.  E.g., the
Python API to unwinding should probably not be limited by this setting.

This patch removes the check from get_prev_frame and introduces a new
checking function that is used when printing a stack trace.

Pedro had suggested removing all the other checks from get_prev_frame.
However, I am not at all confident that this would be correct for all
unwinders.  E.g., do they all know how to stop when the stack is
exhausted?

Advice solicited.

Meanwhile the appended shows the bug -- the test case is a bit odd, but
it was the simplest way I knew of to guarantee that the problem would be
seen -- and the fix is reasonably minimal.

Built and regtested on x86-64 Fedora 18.

Tom

2013-06-06  Tom Tromey  <tromey@redhat.com>

	PR backtrace/15558:
	* frame.c (backtrace_limit): No longer 'static'.
	(get_prev_frame): Don't check backtrace_limit.
	(get_prev_frame_limited): New function.
	* frame.h (get_prev_frame_limited, backtrace_limit): Declare.
	* mi/mi-cmd-stack.c (mi_cmd_stack_list_frames)
	(mi_cmd_stack_info_depth, mi_cmd_stack_list_args): Use
	get_prev_frame_limited.
	* python/py-framefilter.c (py_print_frame): Check
	frame_relative_level.
	* stack.c (backtrace_command_1): Use get_prev_frame_limited.

2013-06-06  Tom Tromey  <tromey@redhat.com>

	* gdb.python/py-frame-inline.c (z, y, x, w): New functions.
	(main): Call w.
	* gdb.python/py-frame-inline.exp: Add test.

diff --git a/gdb/frame.c b/gdb/frame.c
index d52c26a..a9dc820 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -265,7 +265,7 @@ show_backtrace_past_entry (struct ui_file *file, int from_tty,
 		    value);
 }
 
-static unsigned int backtrace_limit = UINT_MAX;
+unsigned int backtrace_limit = UINT_MAX;
 static void
 show_backtrace_limit (struct ui_file *file, int from_tty,
 		      struct cmd_list_element *c, const char *value)
@@ -1985,17 +1985,6 @@ get_prev_frame (struct frame_info *this_frame)
       return NULL;
     }
 
-  /* If the user's backtrace limit has been exceeded, stop.  We must
-     add two to the current level; one of those accounts for backtrace_limit
-     being 1-based and the level being 0-based, and the other accounts for
-     the level of the new frame instead of the level of the current
-     frame.  */
-  if (this_frame->level + 2 > backtrace_limit)
-    {
-      frame_debug_got_null_frame (this_frame, "backtrace limit exceeded");
-      return NULL;
-    }
-
   /* If we're already inside the entry function for the main objfile,
      then it isn't valid.  Don't apply this test to a dummy frame -
      dummy frame PCs typically land in the entry func.  Don't apply
@@ -2044,6 +2033,25 @@ get_prev_frame (struct frame_info *this_frame)
   return get_prev_frame_1 (this_frame);
 }
 
+/* A wrapper for get_prev_frame that respects the backtrace limit.  */
+
+struct frame_info *
+get_prev_frame_limited (struct frame_info *this_frame)
+{
+  /* If the user's backtrace limit has been exceeded, stop.  We must
+     add two to the current level; one of those accounts for backtrace_limit
+     being 1-based and the level being 0-based, and the other accounts for
+     the level of the new frame instead of the level of the current
+     frame.  */
+  if (this_frame->level + 2 > backtrace_limit)
+    {
+      frame_debug_got_null_frame (this_frame, "backtrace limit exceeded");
+      return NULL;
+    }
+
+  return get_prev_frame (this_frame);
+}
+
 CORE_ADDR
 get_frame_pc (struct frame_info *frame)
 {
diff --git a/gdb/frame.h b/gdb/frame.h
index 31b9cb7..59e68d6 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -275,6 +275,9 @@ extern void select_frame (struct frame_info *);
 extern struct frame_info *get_prev_frame (struct frame_info *);
 extern struct frame_info *get_next_frame (struct frame_info *);
 
+/* Like get_prev_frame, but respects the backtrace limit.  */
+extern struct frame_info *get_prev_frame_limited (struct frame_info *);
+
 /* Given a frame's ID, relocate the frame.  Returns NULL if the frame
    is not found.  */
 extern struct frame_info *frame_find_by_id (struct frame_id id);
@@ -683,6 +686,7 @@ extern const char print_entry_values_both[];
 extern const char print_entry_values_compact[];
 extern const char print_entry_values_default[];
 extern const char *print_entry_values;
+extern unsigned int backtrace_limit;
 
 /* Inferior function parameter value read in from a frame.  */
 
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 4b21015..4cf2ffc 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -133,7 +133,7 @@ mi_cmd_stack_list_frames (char *command, char **argv, int argc)
      displaying, or if frame_low is 0.  */
   for (i = 0, fi = get_current_frame ();
        fi && i < frame_low;
-       i++, fi = get_prev_frame (fi));
+       i++, fi = get_prev_frame_limited (fi));
 
   if (fi == NULL)
     error (_("-stack-list-frames: Not enough frames in stack."));
@@ -164,7 +164,7 @@ mi_cmd_stack_list_frames (char *command, char **argv, int argc)
 	 frames in the stack.  */
       for (;
 	   fi && (i <= frame_high || frame_high == -1);
-	   i++, fi = get_prev_frame (fi))
+	   i++, fi = get_prev_frame_limited (fi))
 	{
 	  QUIT;
 	  /* Print the location and the address always, even for level 0.
@@ -195,7 +195,7 @@ mi_cmd_stack_info_depth (char *command, char **argv, int argc)
 
   for (i = 0, fi = get_current_frame ();
        fi && (i < frame_high || frame_high == -1);
-       i++, fi = get_prev_frame (fi))
+       i++, fi = get_prev_frame_limited (fi))
     QUIT;
 
   ui_out_field_int (current_uiout, "depth", i);
@@ -283,7 +283,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
      displaying, or if frame_low is 0.  */
   for (i = 0, fi = get_current_frame ();
        fi && i < frame_low;
-       i++, fi = get_prev_frame (fi));
+       i++, fi = get_prev_frame_limited (fi));
 
   if (fi == NULL)
     error (_("-stack-list-arguments: Not enough frames in stack."));
@@ -315,7 +315,7 @@ mi_cmd_stack_list_args (char *command, char **argv, int argc)
 	 frames in the stack.  */
       for (;
 	   fi && (i <= frame_high || frame_high == -1);
-	   i++, fi = get_prev_frame (fi))
+	   i++, fi = get_prev_frame_limited (fi))
 	{
 	  struct cleanup *cleanup_frame;
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index d62c596..747471c 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1037,6 +1037,12 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
   if (frame == NULL)
     goto error;
 
+  if (frame_relative_level (frame) + 1> backtrace_limit)
+    {
+      do_cleanups (cleanup_stack);
+      return PY_BT_COMPLETED;
+    }
+
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
       gdbarch = get_frame_arch (frame);
diff --git a/gdb/stack.c b/gdb/stack.c
index a4b392e..53a33bc 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1687,7 +1687,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 	  while (current && count--)
 	    {
 	      QUIT;
-	      current = get_prev_frame (current);
+	      current = get_prev_frame_limited (current);
 	    }
 
 	  /* Will stop when CURRENT reaches the top of the stack.
@@ -1695,8 +1695,8 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 	  while (current)
 	    {
 	      QUIT;
-	      trailing = get_prev_frame (trailing);
-	      current = get_prev_frame (current);
+	      trailing = get_prev_frame_limited (trailing);
+	      current = get_prev_frame_limited (current);
 	      trailing_level++;
 	    }
 
@@ -1722,7 +1722,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
          people have strong opinions against reading symbols for
          backtrace this may have to be an option.  */
       i = count;
-      for (fi = trailing; fi != NULL && i--; fi = get_prev_frame (fi))
+      for (fi = trailing; fi != NULL && i--; fi = get_prev_frame_limited (fi))
 	{
 	  CORE_ADDR pc;
 
@@ -1755,7 +1755,9 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
      "no-filters" has been specified from the command.  */
   if (no_filters ||  result == PY_BT_NO_FILTERS)
     {
-      for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi))
+      for (i = 0, fi = trailing;
+	   fi && count--;
+	   i++, fi = get_prev_frame_limited (fi))
 	{
 	  QUIT;
 
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.c b/gdb/testsuite/gdb.python/py-frame-inline.c
index 68b93db..eda5ec4 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.c
+++ b/gdb/testsuite/gdb.python/py-frame-inline.c
@@ -36,8 +36,34 @@ g (void)
   return f ();
 }
 
+inline __attribute__((__always_inline__)) int
+z (void)
+{
+  return f ();
+}
+
+inline __attribute__((__always_inline__)) int
+y (void)
+{
+  return z ();
+}
+
+inline __attribute__((__always_inline__)) int
+x (void)
+{
+  return y ();
+}
+
+inline __attribute__((__always_inline__)) int
+w (void)
+{
+  return x ();
+}
+
 int
 main (void)
 {
-  return g ();
+  int x = g ();
+  x += w ();
+  return x;
 }
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp
index 00978b0..386e2d5 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.exp
+++ b/gdb/testsuite/gdb.python/py-frame-inline.exp
@@ -37,3 +37,12 @@ gdb_test "info frame" "inlined into frame 1\r\n.*"
 gdb_test "up" "#1  g .*"
 
 gdb_test "python print (gdb.selected_frame().read_var('l'))" "\r\n42"
+
+# A regression test for having unwinding stop in the middle of a
+# sequence of inlined frames.  This test case is tricky due to the
+# conditions under which it can be reproduced: it needs at least 3
+# inlined frames and gdb needs to try to compute the frame_id.
+# See PR backtrace/15558.
+gdb_continue_to_breakpoint "Block break here."
+gdb_test_no_output "set backtrace limit 1"
+gdb_test "python print (gdb.newest_frame())" ".*"


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