RFC: fix PR backtrace/15558

RFC: fix PR backtrace/15558

[Tom Tromey]

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())" ".*"

Re: RFC: fix PR backtrace/15558

[Pedro Alves]

Hi Tom,

On 06/06/2013 08:35 PM, Tom Tromey wrote:
> 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.

Yeah, I was suggesting that "internal" / non-user-facing code should
not be using get_prev_frame, but get_prev_frame_1 instead, bypassing
all the checks.  (Or rather wondering why that isn't so).  Strongly more
so in an unwinder's innards.  get_prev_frame uses need to be investigated
on a case-by-case manner manner to decide the best course of action, IMO.

Focusing on inline_frame_this_id first, the case in question for
the bug:

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're building the frame id for the inline frame.  I say the CLI
"set backtrace limit/past-entry/past-main" settings should really have
no bearing on this.  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 it 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 stack frame,
then we wouldn't have an inline frame either, by design.  It's just
logically impossible.  That's what the assertion catches:

  /* 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
     apply to all frames, all the time.  That would fix the ambiguity
     of null_frame_id (between "no/any frame" and "the outermost
     frame").  This will take work.  */
  gdb_assert (frame_id_p (*this_id));

A note on that NOTE.  It's stale; we have outer_frame_id to distinguish
from null_frame_id now.  (though that should really die.)

Hard to imagine the main function being inlined (at least on c-like
languages), but if any of the NULL returns in get_prev_frame hit here,
you'll trip on this assertion again.  It seems like the zero PC case
at the bottom of get_prev_frame can trigger.  I can't say I understand what
exactly that is supposed to catch.

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_raw (this_frame);

And note how the somewhat 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;
}

So conceptually, in this case, I think what makes most sense it to skip
_all_ the checks in get_prev_frame* that might return NULL, as there should
always be a prev frame for an inline frame.  IOW, in this case, I believe
we should be making inline_frame_this_id call get_prev_frame_1, or whatever
it gets renamed to, or equivalent.

-- 
Pedro Alves

Re: RFC: fix PR backtrace/15558

[Tom Tromey]

Pedro> Yeah, I was suggesting that "internal" / non-user-facing code
Pedro> should not be using get_prev_frame, but get_prev_frame_1 instead,
Pedro> bypassing all the checks.  (Or rather wondering why that isn't
Pedro> so).  Strongly more so in an unwinder's innards.  get_prev_frame
Pedro> uses need to be investigated on a case-by-case manner manner to
Pedro> decide the best course of action, IMO.

I agree from a design standpoint that this is superior.

My main concern is that I am not confident that all the unwinders in the
tree actually stop sanely.  If we believe that they do then it seems
straightforward to do the split as you suggest.

Normally I don't like to code to work around potential bugs elsewhere.
However in some parts of gdb, like this one, it is difficult to do
otherwise, due to the testing problem.

Anyway, this is why I split the function where I did.

Pedro> So conceptually, in this case, I think what makes most sense it
Pedro> to skip _all_ the checks in get_prev_frame* that might return
Pedro> NULL, as there should always be a prev frame for an inline frame.
Pedro> IOW, in this case, I believe we should be making
Pedro> inline_frame_this_id call get_prev_frame_1, or whatever it gets
Pedro> renamed to, or equivalent.

That sounds reasonable.  I'll rework the patch next week.

Tom

Re: RFC: fix PR backtrace/15558

[Andrew Pinski]

On Fri, Jun 21, 2013 at 1:41 PM, Tom Tromey <tromey@redhat.com> wrote:
> Pedro> Yeah, I was suggesting that "internal" / non-user-facing code
> Pedro> should not be using get_prev_frame, but get_prev_frame_1 instead,
> Pedro> bypassing all the checks.  (Or rather wondering why that isn't
> Pedro> so).  Strongly more so in an unwinder's innards.  get_prev_frame
> Pedro> uses need to be investigated on a case-by-case manner manner to
> Pedro> decide the best course of action, IMO.
>
> I agree from a design standpoint that this is superior.
>
> My main concern is that I am not confident that all the unwinders in the
> tree actually stop sanely.  If we believe that they do then it seems
> straightforward to do the split as you suggest.
>
> Normally I don't like to code to work around potential bugs elsewhere.
> However in some parts of gdb, like this one, it is difficult to do
> otherwise, due to the testing problem.
>
> Anyway, this is why I split the function where I did.
>
> Pedro> So conceptually, in this case, I think what makes most sense it
> Pedro> to skip _all_ the checks in get_prev_frame* that might return
> Pedro> NULL, as there should always be a prev frame for an inline frame.
> Pedro> IOW, in this case, I believe we should be making
> Pedro> inline_frame_this_id call get_prev_frame_1, or whatever it gets
> Pedro> renamed to, or equivalent.
>
> That sounds reasonable.  I'll rework the patch next week.

Hi Tom,
  What happened to this patch, I don't see any reference to it latter
on?  I think I am running into the same problem as this patch is
fixing.

Thanks,
Andrew Pinski


>
> Tom

Re: RFC: fix PR backtrace/15558

[Tom Tromey]

>>>>> "Andrew" == Andrew Pinski <pinskia@gmail.com> writes:

Andrew>   What happened to this patch, I don't see any reference to it latter
Andrew> on?  I think I am running into the same problem as this patch is
Andrew> fixing.

I never got back to fixing it according to Pedro's review.

As I recall it isn't entirely trivial because for a good fix one would
need to expose some new Python methods for use by the frame filter code.

Tom

Re: RFC: fix PR backtrace/15558

[Pedro Alves]

On 04/14/2014 07:06 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Pinski <pinskia@gmail.com> writes:
> 
> Andrew>   What happened to this patch, I don't see any reference to it latter
> Andrew> on?  I think I am running into the same problem as this patch is
> Andrew> fixing.
> 
> I never got back to fixing it according to Pedro's review.
> 
> As I recall it isn't entirely trivial because for a good fix one would
> need to expose some new Python methods for use by the frame filter code.

Hmm.  Not sure what you're thinking of.  We discussed auditing all
get_prev_frame uses and see if they are better replaced by
get_prev_frame_1, but I don't think that should hold back
fixing the inline frame unwinder.  In the particular case of the
inline frame unwinder, I believe we should simply be making
inline_frame_this_id call get_prev_frame_1 instead of
get_prev_frame.  Let me give it a try.  Hmm, your test doesn't
trigger the bug for me as is, but
I can trigger it in the CLI without Python.  Ah, a "flushregs"
is missing to force re-unwinding and recomputing the frame id.
A sec and I'll post something.

-- 
Pedro Alves

Re: RFC: fix PR backtrace/15558

[Tom Tromey]

Tom> I never got back to fixing it according to Pedro's review.

Tom> As I recall it isn't entirely trivial because for a good fix one would
Tom> need to expose some new Python methods for use by the frame filter code.

Pedro> Hmm.  Not sure what you're thinking of.

There's a good chance I'm confusing it with some other approach I'd
considered.

Pedro> We discussed auditing all
Pedro> get_prev_frame uses and see if they are better replaced by
Pedro> get_prev_frame_1, but I don't think that should hold back
Pedro> fixing the inline frame unwinder.

Ok.

I think what I was thinking is that if one starts this change, then the
question arises: which should the Python unwinder call?  Calling the
user-facing one is plainly incorrect.  But, calling the internal one is
also incorrect, as some termination conditions may be skipped.

Perhaps this only applies if we move all the checks into get_prev_frame_1.
I don't recall whether that was part of the plan or not.

Tom

Re: RFC: fix PR backtrace/15558

[Pedro Alves]

On 04/14/2014 07:52 PM, Tom Tromey wrote:
> Tom> I never got back to fixing it according to Pedro's review.
> 
> Tom> As I recall it isn't entirely trivial because for a good fix one would
> Tom> need to expose some new Python methods for use by the frame filter code.
> 
> Pedro> Hmm.  Not sure what you're thinking of.
> 
> There's a good chance I'm confusing it with some other approach I'd
> considered.
> 
> Pedro> We discussed auditing all
> Pedro> get_prev_frame uses and see if they are better replaced by
> Pedro> get_prev_frame_1, but I don't think that should hold back
> Pedro> fixing the inline frame unwinder.
> 
> Ok.

See patch below.  I've simplified your test a bit, and added a test
that doesn't depend on Python, to the existing gdb.opt/inline-bt.exp.

> I think what I was thinking is that if one starts this change, then the
> question arises: which should the Python unwinder call?  Calling the
> user-facing one is plainly incorrect.

Not sure.  I guess Python code might want to be implementing some
new frame-related CLI command, where the limits make sense.  Maybe
Python should really have access to both variants, somehow?

> But, calling the internal one is
> also incorrect, as some termination conditions may be skipped.
>
> Perhaps this only applies if we move all the checks into get_prev_frame_1.
> I don't recall whether that was part of the plan or not.

Well, the plan didn't go as far as prescribe what to do exactly, only
auditing/thinking through this.  :-)  The only termination condition in
get_prev_frame that is a not user setting is the "zero PC" one, and
it's arguable whether that one should be there or in the internal
one.

-------------
[PATCH] Fix PR backtrace/15558

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.  FSF submission NOTE: this would be a good assertion to
     apply to all frames, all the time.  That would fix the ambiguity
     of null_frame_id (between "no/any frame" and "the outermost
     frame").  This will take work.  */
  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.

(A note on that NOTE above.  It's stale: we have outer_frame_id to
distinguish from null_frame_id now.)

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-14  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-14  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.
	* py-frame-inline.exp: Test running to an inline function with a
	bactrace limit, and printing the newest frame.
---
 gdb/frame.c                                  | 31 ++++++++++++++++------------
 gdb/frame.h                                  |  7 +++++++
 gdb/inline-frame.c                           | 16 +++++++-------
 gdb/testsuite/gdb.opt/inline-bt.exp          | 16 ++++++++++++++
 gdb/testsuite/gdb.python/py-frame-inline.c   |  4 +++-
 gdb/testsuite/gdb.python/py-frame-inline.exp | 14 +++++++++++++
 6 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 97d54e9..013d602 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -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;
diff --git a/gdb/frame.h b/gdb/frame.h
index e451a93..b88bd28 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -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);
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index 05ba9ff..6255630 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -26,6 +26,7 @@
 #include "regcache.h"
 #include "symtab.h"
 #include "vec.h"
+#include "frame.h"
 
 #include "gdb_assert.h"
 
@@ -154,17 +155,14 @@ 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
-     apply to all frames, all the time.  That would fix the ambiguity
-     of null_frame_id (between "no/any frame" and "the outermost
-     frame").  This will take work.  */
+     frame.  */
   gdb_assert (frame_id_p (*this_id));
 
   /* For now, require we don't match outer_frame_id either (see
diff --git a/gdb/testsuite/gdb.opt/inline-bt.exp b/gdb/testsuite/gdb.opt/inline-bt.exp
index c437383..ce73623 100644
--- a/gdb/testsuite/gdb.opt/inline-bt.exp
+++ b/gdb/testsuite/gdb.opt/inline-bt.exp
@@ -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"
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.c b/gdb/testsuite/gdb.python/py-frame-inline.c
index a3669bc..f08e84b 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.c
+++ b/gdb/testsuite/gdb.python/py-frame-inline.c
@@ -39,5 +39,7 @@ g (void)
 int
 main (void)
 {
-  return g ();
+  int x = g ();
+  x += f ();
+  return x;
 }
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp
index f5cf33e..8851d87 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.exp
+++ b/gdb/testsuite/gdb.python/py-frame-inline.exp
@@ -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())" ".*"
-- 
1.7.11.7

Re: RFC: fix PR backtrace/15558

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> See patch below.  I've simplified your test a bit, and added a test
Pedro> that doesn't depend on Python, to the existing gdb.opt/inline-bt.exp.

Thanks.

Pedro> Not sure.  I guess Python code might want to be implementing some
Pedro> new frame-related CLI command, where the limits make sense.  Maybe
Pedro> Python should really have access to both variants, somehow?

I was thinking perhaps new methods on the python frame object.

Pedro> 	PR backtrace/15558
Pedro> 	* gdb.opt/inline-bt.exp: Test backtracing from an inline function
Pedro> 	with a backtrace limit.
Pedro> 	* py-frame-inline.exp: Test running to an inline function with a
Pedro> 	bactrace limit, and printing the newest frame.

Typo in "backtrace" on the last line.

The patch looks good but the ChangeLog has a missing directory name and
doesn't mention :

>  gdb/testsuite/gdb.python/py-frame-inline.c   |  4 +++-

Tom

Re: RFC: fix PR backtrace/15558

[Pedro Alves]

On 04/16/2014 09:03 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> Pedro> Not sure.  I guess Python code might want to be implementing some
> Pedro> new frame-related CLI command, where the limits make sense.  Maybe
> Pedro> Python should really have access to both variants, somehow?
> 
> I was thinking perhaps new methods on the python frame object.

Works for me.

> Pedro> 	PR backtrace/15558
> Pedro> 	* gdb.opt/inline-bt.exp: Test backtracing from an inline function
> Pedro> 	with a backtrace limit.
> Pedro> 	* py-frame-inline.exp: Test running to an inline function with a
> Pedro> 	bactrace limit, and printing the newest frame.
> 
> Typo in "backtrace" on the last line.
> 
> The patch looks good but the ChangeLog has a missing directory name and
> doesn't mention :
> 
>>  gdb/testsuite/gdb.python/py-frame-inline.c   |  4 +++-
> 

Thank you.  Fixed and pushed as below.  I also removed the change
to the comment about outer frames from the patch, as it's an
unrelated change.

------
[PATCH] Fix PR backtrace/15558

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                                | 13 ++++++++++++
 gdb/frame.c                                  | 31 ++++++++++++++++------------
 gdb/frame.h                                  |  7 +++++++
 gdb/inline-frame.c                           | 11 +++++-----
 gdb/testsuite/ChangeLog                      | 10 +++++++++
 gdb/testsuite/gdb.opt/inline-bt.exp          | 16 ++++++++++++++
 gdb/testsuite/gdb.python/py-frame-inline.c   |  4 +++-
 gdb/testsuite/gdb.python/py-frame-inline.exp | 14 +++++++++++++
 8 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c9b914a..cad4602 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -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
diff --git a/gdb/frame.c b/gdb/frame.c
index 97d54e9..013d602 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -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;
diff --git a/gdb/frame.h b/gdb/frame.h
index e451a93..b88bd28 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -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);
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index 05ba9ff..eb82143 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -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
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6f5f9aa..08a3a61 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -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.
diff --git a/gdb/testsuite/gdb.opt/inline-bt.exp b/gdb/testsuite/gdb.opt/inline-bt.exp
index c437383..ce73623 100644
--- a/gdb/testsuite/gdb.opt/inline-bt.exp
+++ b/gdb/testsuite/gdb.opt/inline-bt.exp
@@ -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"
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.c b/gdb/testsuite/gdb.python/py-frame-inline.c
index a3669bc..f08e84b 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.c
+++ b/gdb/testsuite/gdb.python/py-frame-inline.c
@@ -39,5 +39,7 @@ g (void)
 int
 main (void)
 {
-  return g ();
+  int x = g ();
+  x += f ();
+  return x;
 }
diff --git a/gdb/testsuite/gdb.python/py-frame-inline.exp b/gdb/testsuite/gdb.python/py-frame-inline.exp
index f5cf33e..8851d87 100644
--- a/gdb/testsuite/gdb.python/py-frame-inline.exp
+++ b/gdb/testsuite/gdb.python/py-frame-inline.exp
@@ -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())" ".*"
-- 
1.7.11.7