diff mbox

[v2,2/3] frame: use get_prev_frame_always in skip_tailcall_frames

Message ID A78C989F6D9628469189715575E55B2333264126@IRSMSX104.ger.corp.intel.com
State New
Headers show

Commit Message

Metzger, Markus T Feb. 19, 2016, 11:36 a.m. UTC
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, February 17, 2016 4:32 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; Joel Brobecker
> <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames
> On 02/15/2016 09:50 AM, Metzger, Markus T wrote:
> > I'm wondering in which cases GDB should ignore the user-defined
> > backtrace limit.  And if GDB should ignore it at all.
> >
> > If the limit is set, some aspects of GDB may not function any longer.
> > But that's to be expected, isn't it?
> >
> > GDB shouldn't crash, of course.  But I'm not sure if it should ignore
> > user settings in too many cases.
> I'm starting to think the same way.  Want to give it a try and see what breaks?

With this change, the regression tests for PR backtrace/15558 in
gdb.opt/inline-bt.exp and gdb.python/py-frame-inline.exp still pass.

I have not debugged it but with the recent changes skip_artificial_frames
should return NULL which should be handled by its callers and this seems
to do the right thing for those tests.

Or maybe they just rely on get_prev_frame_always in inline_frame_this_id.
Which makes you wonder why skip_artificial_frames was changed as part
of the bug-fix.

With skip_artificial_frames not unwinding past the backtrace limit, it
doesn't really make sense to call get_prev_frame_always in
frame_unwind_caller_id and frame_pop.

I changed those back to get_prev_frame but I left get_prev_frame_always
in inline_frame_this_id.  Curiously, tailcall_frame_this_id uses get_prev_frame
instead of get_prev_frame_always.  I somehow expected those to be aligned.

When I add a backtrace limit of 1 to the following tests...

#       modified:   gdb/testsuite/gdb.ada/out_of_line_in_inlined.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-cxx.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-noret.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-self.exp
#       modified:   gdb/testsuite/gdb.base/finish.exp
#       modified:   gdb/testsuite/gdb.base/return.exp
#       modified:   gdb/testsuite/gdb.base/return2.exp
#       modified:   gdb/testsuite/gdb.dwarf2/dw2-inline-param.exp

(gdb.opt/inline-bt.exp already sets a backtrace limit)

...I get a few new fails in


	Just needs handling the error messages we now get when the
	finish and return commands fail when they can no longer skip
	the tailcall frames.


	This one is more interesting.  The backtrace output changes from:

	#0  g (x=x@entry=2) at gdb.arch/amd64-tailcall-cxx1.cc:23


	#0  g (x=2) at gdb.arch/amd64-tailcall-cxx1.cc:23

	I have not looked into it.

The rest doesn't give any new fails.

Adding a backtrace limit of 1 to gdb.opt/inline-cmds.exp causes most of the
tests to fail.  It looks like stepping commands are affected by such an aggressive
backtrace limit.

Looking at infrun, GDB uses frame_unwind_caller_id and get_prev_frame (in
stepped_in_from) to detect calls.  Those should depend on the backtrace limit.

> We need to also keep in mind that there may be cases where
> skip_artificial_frames might be used in internal-facing code, where it might
> still be necessary get past inline frames to reach the real stack frame.  I guess
> sticking a "set backtrace limit 1" in some of the inline tests would expose this.

Stepping should break with such an aggressive backtrace limit.  So I looked into
tail-called and inlined functions.

For tailcalls, I was not able to "next" over a tail-called function.  I always ended
up in the function I called.  Even without a backtrace limit and with
skip_artificial_frames use get_prev_frame_always.  Is this the expected behavior?

For inlined functions, GDB crashes in an inline-frame-skipping loop that uses
get_prev_frame in dwarf2_frame_cfa.

With that fixed I would imagine that for every backtrace limit you could construct
a case where "next" over an inlined function breaks by stacking enough inlined
function calls.  I have not tried it.

For normal calls, a backtrace limit of 2 should suffice.  I have not tried it.

The question remains whether the backtrace limit should just cap the output of
the "backtrace" command or whether it should be a global limit.  If it were just the
former I would have expected the limit check to be in the "backtrace" command


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
diff mbox


diff --git a/gdb/frame.c b/gdb/frame.c
index d621dd7..f436010 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -436,7 +436,7 @@  skip_artificial_frames (struct frame_info *frame)
   while (get_frame_type (frame) == INLINE_FRAME
         || get_frame_type (frame) == TAILCALL_FRAME)
-      frame = get_prev_frame_always (frame);
+      frame = get_prev_frame (frame);
       if (frame == NULL)