[PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames
Metzger, Markus T
markus.t.metzger@intel.com
Fri Feb 19 11:36:00 GMT 2016
> -----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?
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)
break;
}
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
gdb.base/finish.exp
gdb.base/return.exp
gdb.base/return2.exp
gdb.arch/amd64-tailcall-ret.exp
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.
gdb.arch/amd64-tailcall-cxx.exp
This one is more interesting. The backtrace output changes from:
#0 g (x=x@entry=2) at gdb.arch/amd64-tailcall-cxx1.cc:23
to
#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
function.
Regards,
Markus.
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
More information about the Gdb-patches
mailing list