Fix PR backtrace/15558
authorPedro Alves <palves@redhat.com>
Fri, 18 Apr 2014 09:15:21 +0000 (10:15 +0100)
committerPedro Alves <palves@redhat.com>
Fri, 18 Apr 2014 09:34:09 +0000 (10:34 +0100)
This PR is about an assertion failure in GDB that can be triggered by
setting "backtrace limit" to a value that causes GDB to stop unwinding
after an inline frame.  In this case, an assertion in
inline_frame_this_id will trigger:

  /* We need a valid frame ID, so we need to be based on a valid
     frame.  (...).  */
  gdb_assert (frame_id_p (*this_id));

Looking at the function:

 static void
 inline_frame_this_id (struct frame_info *this_frame,
       void **this_cache,
       struct frame_id *this_id)
 {
   struct symbol *func;

   /* In order to have a stable frame ID for a given inline function,
      we must get the stack / special addresses from the underlying
      real frame's this_id method.  So we must call get_prev_frame.
      Because we are inlined into some function, there must be previous
      frames, so this is safe - as long as we're careful not to
      create any cycles.  */
   *this_id = get_frame_id (get_prev_frame (this_frame));

we see we're computing the frame id for the inline frame.  If this is
an inline frame, which is a virtual frame constructed based on debug
info, on top of a real stack frame, we should _always_ be able to find
where the frame was inlined into, as that ultimately just means
peeling off the virtual frames on top of the real stack frame.  If
there ultimately was no prev (real) stack frame, then we wouldn't have
been able to construct the inline frame either, by design.  That's
what the assertion catches.

So we have an inline frame, we should _always_ be able to compute its
ID, even if that means bypassing the user backtrace limits to get at
the real stack frame's info.  The problem is that inline_frame_id
calls get_prev_frame, and that takes user backtrace limits into
account.  Code that wants to bypass the limits calls get_prev_frame_1
instead.

Note how get_prev_frame_1 already skips all checks for inline frames:

   /* If we are unwinding from an inline frame, all of the below tests
      were already performed when we unwound from the next non-inline
      frame.  We must skip them, since we can not get THIS_FRAME's ID
      until we have unwound all the way down to the previous non-inline
      frame.  */
   if (get_frame_type (this_frame) == INLINE_FRAME)
     return get_prev_frame_if_no_cycle (this_frame);

And note how the related frame_unwind_caller_id function also uses
get_prev_frame_1:

 struct frame_id
 frame_unwind_caller_id (struct frame_info *next_frame)
 {
   struct frame_info *this_frame;

   /* Use get_prev_frame_1, and not get_prev_frame.  The latter will truncate
      the frame chain, leading to this function unintentionally
      returning a null_frame_id (e.g., when a caller requests the frame
      ID of "main()"s caller.  */

   next_frame = skip_artificial_frames (next_frame);
   this_frame = get_prev_frame_1 (next_frame);
   if (this_frame)
     return get_frame_id (skip_artificial_frames (this_frame));
   else
     return null_frame_id;
 }

get_prev_frame_1 is currently static in frame.c.  As a _1 suffix is
not a good name for an extern function, I've renamed it.

Tested on x86-64 Fedora 17.

gdb/
2014-04-18  Pedro alves  <palves@redhat.com>
    Tom Tromey  <tromey@redhat.com>

PR backtrace/15558
* frame.c (get_prev_frame_1): Rename to ...
(get_prev_frame_always): ... this, and make extern.  Adjust.
(skip_artificial_frames): Use get_prev_frame_always.
(frame_unwind_caller_id, frame_pop, get_prev_frame)
(get_frame_unwind_stop_reason): Adjust to rename.
* frame.h (get_prev_frame_always): Declare.
* inline-frame.c: Include frame.h.
(inline_frame_this_id): Use get_prev_frame_always.

gdb/testsuite/
2014-04-18  Tom Tromey  <palves@redhat.com>
    Pedro alves  <tromey@redhat.com>

PR backtrace/15558
* gdb.opt/inline-bt.exp: Test backtracing from an inline function
with a backtrace limit.
* gdb.python/py-frame-inline.exp: Test running to an inline
function with a backtrace limit, and printing the newest frame.
* gdb.python/py-frame-inline.c (main): Call f.

gdb/ChangeLog
gdb/frame.c
gdb/frame.h
gdb/inline-frame.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.opt/inline-bt.exp
gdb/testsuite/gdb.python/py-frame-inline.c
gdb/testsuite/gdb.python/py-frame-inline.exp

index c9b914ae54e908c9c7a50a7824fcb5a98102f0de..cad4602b0c3308362a7d8503b8ee3a1992f451cf 100644 (file)
@@ -1,3 +1,16 @@
+2014-04-18  Pedro alves  <palves@redhat.com>
+           Tom Tromey  <tromey@redhat.com>
+
+       PR backtrace/15558
+       * frame.c (get_prev_frame_1): Rename to ...
+       (get_prev_frame_always): ... this, and make extern.  Adjust.
+       (skip_artificial_frames): Use get_prev_frame_always.
+       (frame_unwind_caller_id, frame_pop, get_prev_frame)
+       (get_frame_unwind_stop_reason): Adjust to rename.
+       * frame.h (get_prev_frame_always): Declare.
+       * inline-frame.c: Include frame.h.
+       (inline_frame_this_id): Use get_prev_frame_always.
+
 2014-04-18  Tristan Gingold  <gingold@adacore.com>
 
        * solib-darwin.c (darwin_solib_create_inferior_hook): Simplify
index 97d54e91a92164ac8e6f2eddbe8cdc9c32e7914f..013d602feca7d922f3f99225de9b47cf0d48ec46 100644 (file)
@@ -46,7 +46,6 @@
 #include "hashtab.h"
 #include "valprint.h"
 
-static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);
 static struct frame_info *get_prev_frame_raw (struct frame_info *this_frame);
 static const char *frame_stop_reason_symbol_string (enum unwind_stop_reason reason);
 
@@ -425,9 +424,15 @@ fprint_frame (struct ui_file *file, struct frame_info *fi)
 static struct frame_info *
 skip_artificial_frames (struct frame_info *frame)
 {
+  /* Note we use get_prev_frame_always, and not get_prev_frame.  The
+     latter will truncate the frame chain, leading to this function
+     unintentionally returning a null_frame_id (e.g., when the user
+     sets a backtrace limit).  This is safe, because as these frames
+     are made up by GDB, there must be a real frame in the chain
+     below.  */
   while (get_frame_type (frame) == INLINE_FRAME
         || get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame (frame);
+    frame = get_prev_frame_always (frame);
 
   return frame;
 }
@@ -484,13 +489,13 @@ frame_unwind_caller_id (struct frame_info *next_frame)
 {
   struct frame_info *this_frame;
 
-  /* Use get_prev_frame_1, and not get_prev_frame.  The latter will truncate
-     the frame chain, leading to this function unintentionally
-     returning a null_frame_id (e.g., when a caller requests the frame
-     ID of "main()"s caller.  */
+  /* Use get_prev_frame_always, and not get_prev_frame.  The latter
+     will truncate the frame chain, leading to this function
+     unintentionally returning a null_frame_id (e.g., when a caller
+     requests the frame ID of "main()"s caller.  */
 
   next_frame = skip_artificial_frames (next_frame);
-  this_frame = get_prev_frame_1 (next_frame);
+  this_frame = get_prev_frame_always (next_frame);
   if (this_frame)
     return get_frame_id (skip_artificial_frames (this_frame));
   else
@@ -956,7 +961,7 @@ frame_pop (struct frame_info *this_frame)
     }
 
   /* Ensure that we have a frame to pop to.  */
-  prev_frame = get_prev_frame_1 (this_frame);
+  prev_frame = get_prev_frame_always (this_frame);
 
   if (!prev_frame)
     error (_("Cannot pop the initial frame."));
@@ -1775,8 +1780,8 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame)
    Unlike get_prev_frame, this function always tries to unwind the
    frame.  */
 
-static struct frame_info *
-get_prev_frame_1 (struct frame_info *this_frame)
+struct frame_info *
+get_prev_frame_always (struct frame_info *this_frame)
 {
   struct gdbarch *gdbarch;
 
@@ -1785,7 +1790,7 @@ get_prev_frame_1 (struct frame_info *this_frame)
 
   if (frame_debug)
     {
-      fprintf_unfiltered (gdb_stdlog, "{ get_prev_frame_1 (this_frame=");
+      fprintf_unfiltered (gdb_stdlog, "{ get_prev_frame_always (this_frame=");
       if (this_frame != NULL)
        fprintf_unfiltered (gdb_stdlog, "%d", this_frame->level);
       else
@@ -2137,7 +2142,7 @@ get_prev_frame (struct frame_info *this_frame)
       return NULL;
     }
 
-  return get_prev_frame_1 (this_frame);
+  return get_prev_frame_always (this_frame);
 }
 
 CORE_ADDR
@@ -2523,7 +2528,7 @@ enum unwind_stop_reason
 get_frame_unwind_stop_reason (struct frame_info *frame)
 {
   /* Fill-in STOP_REASON.  */
-  get_prev_frame_1 (frame);
+  get_prev_frame_always (frame);
   gdb_assert (frame->prev_p);
 
   return frame->stop_reason;
index e451a9395e99df36e2d72eed97f9effbb5c62f38..b88bd281cbe0e1161a392c4f8297821462fafc3b 100644 (file)
@@ -307,6 +307,13 @@ 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 *);
 
+/* Return a "struct frame_info" corresponding to the frame that called
+   THIS_FRAME.  Returns NULL if there is no such frame.
+
+   Unlike get_prev_frame, this function always tries to unwind the
+   frame.  */
+extern struct frame_info *get_prev_frame_always (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);
index 05ba9ff8dbb3506be87e8ac6032b9c057bd039e0..eb821435cf23656d99907e3aeca2e64253846120 100644 (file)
@@ -26,6 +26,7 @@
 #include "regcache.h"
 #include "symtab.h"
 #include "vec.h"
+#include "frame.h"
 
 #include "gdb_assert.h"
 
@@ -154,11 +155,11 @@ inline_frame_this_id (struct frame_info *this_frame,
 
   /* In order to have a stable frame ID for a given inline function,
      we must get the stack / special addresses from the underlying
-     real frame's this_id method.  So we must call get_prev_frame.
-     Because we are inlined into some function, there must be previous
-     frames, so this is safe - as long as we're careful not to
-     create any cycles.  */
-  *this_id = get_frame_id (get_prev_frame (this_frame));
+     real frame's this_id method.  So we must call
+     get_prev_frame_always.  Because we are inlined into some
+     function, there must be previous frames, so this is safe - as
+     long as we're careful not to create any cycles.  */
+  *this_id = get_frame_id (get_prev_frame_always (this_frame));
 
   /* We need a valid frame ID, so we need to be based on a valid
      frame.  FSF submission NOTE: this would be a good assertion to
index 6f5f9aadfb40a7af421f73acc65edd27f4082626..08a3a61bf4088ddd5f0318b9e18df44a5f6e7ffa 100644 (file)
@@ -1,3 +1,13 @@
+2014-04-18  Tom Tromey  <palves@redhat.com>
+           Pedro alves  <tromey@redhat.com>
+
+       PR backtrace/15558
+       * gdb.opt/inline-bt.exp: Test backtracing from an inline function
+       with a backtrace limit.
+       * gdb.python/py-frame-inline.exp: Test running to an inline
+       function with a backtrace limit, and printing the newest frame.
+       * gdb.python/py-frame-inline.c (main): Call f.
+
 2014-04-17  Marcus Shawcroft  <marcus.shawcroft@arm.com>
 
        * gdb.java/jnpe.exp: Drop srcdir from untested path.
index c4373835557b4d93f11fb7f231ef7c3b7d10528e..ce7362352d608d116202beecca68867a074e955e 100644 (file)
@@ -50,3 +50,19 @@ gdb_test "up" "#1  .*func1.*" "up from bar (3)"
 gdb_test "info frame" ".*inlined into frame.*" "func1 inlined (3)"
 gdb_test "up" "#2  .*func2.*" "up from func1 (3)"
 gdb_test "info frame" ".*inlined into frame.*" "func2 inlined (3)"
+
+# A regression test for having a backtrace limit that forces unwinding
+# to stop after an inline frame.  GDB needs to compute the frame_id of
+# the inline frame, which requires unwinding past all the inline
+# frames to the real stack frame, even if that means bypassing the
+# user visible backtrace limit.  See PR backtrace/15558.
+#
+# Set a backtrace limit that forces an unwind stop after an inline
+# function.
+gdb_test_no_output "set backtrace limit 2"
+# Force flushing the frame cache.
+gdb_test "flushregs" "Register cache flushed."
+gdb_test "up" "#1  .*func1.*" "up from bar (4)"
+gdb_test "info frame" ".*in func1.*" "info frame still works"
+# Verify the user visible limit works as expected.
+gdb_test "up" "Initial frame selected; you cannot go up." "up hits limit"
index a3669bc1050146fc958a236b72912c616d2e0164..f08e84b6e41cfed7b6a3b9dc20b34472b445facd 100644 (file)
@@ -39,5 +39,7 @@ g (void)
 int
 main (void)
 {
-  return g ();
+  int x = g ();
+  x += f ();
+  return x;
 }
index f5cf33e7f5d8e5208d08fd43c6055f03f1a27ee2..8851d878dfd4c04f5a4e22527c8659ad559489a5 100644 (file)
@@ -37,3 +37,17 @@ 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 a backtrace limit that forces unwinding
+# to stop after an inline frame.  GDB needs to compute the frame_id of
+# the inline frame, which requires unwinding past all the inline
+# frames to the real stack frame, even if that means bypassing the
+# user visible backtrace limit.  See PR backtrace/15558.
+#
+# Set the limit, and run to an inline function.  It's important that
+# the frame cache is flushed somehow after setting the limit, to force
+# frame id recomputation.
+gdb_test_no_output "set backtrace limit 1"
+gdb_continue_to_breakpoint "Block break here."
+
+gdb_test "python print (gdb.newest_frame())" ".*"