From patchwork Fri Feb 19 11:36:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Metzger, Markus T" X-Patchwork-Id: 10920 Received: (qmail 51796 invoked by alias); 19 Feb 2016 11:36:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 51716 invoked by uid 89); 19 Feb 2016 11:36:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL, BAYES_50, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:out_of_, UD:out_of_line_in_inlined.exp, sk:pyfram, sk:py-fram X-HELO: mga04.intel.com Received: from mga04.intel.com (HELO mga04.intel.com) (192.55.52.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Feb 2016 11:36:47 +0000 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP; 19 Feb 2016 03:36:45 -0800 X-ExtLoop1: 1 Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga003.jf.intel.com with ESMTP; 19 Feb 2016 03:36:44 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.210]) by IRSMSX154.ger.corp.intel.com ([169.254.12.208]) with mapi id 14.03.0248.002; Fri, 19 Feb 2016 11:36:43 +0000 From: "Metzger, Markus T" To: 'Pedro Alves' , Joel Brobecker CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames Date: Fri, 19 Feb 2016 11:36:42 +0000 Message-ID: References: <1454681922-2228-1-git-send-email-markus.t.metzger@intel.com> <1454681922-2228-2-git-send-email-markus.t.metzger@intel.com> <20160207130057.GE20874@adacore.com> <56B9D08F.6060507@redhat.com> <20160209115819.GH15342@adacore.com> <56B9FA90.5030602@redhat.com> <56C49276.8080601@redhat.com> In-Reply-To: <56C49276.8080601@redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Wednesday, February 17, 2016 4:32 PM > To: Metzger, Markus T ; Joel Brobecker > > 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 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 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; }