From patchwork Thu Feb 11 15:42:50 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: 10825 Received: (qmail 118018 invoked by alias); 11 Feb 2016 15:42:57 -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 117997 invoked by uid 89); 11 Feb 2016 15:42:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mga02.intel.com Received: from mga02.intel.com (HELO mga02.intel.com) (134.134.136.20) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Feb 2016 15:42:55 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga101.jf.intel.com with ESMTP; 11 Feb 2016 07:42:53 -0800 X-ExtLoop1: 1 Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga002.fm.intel.com with ESMTP; 11 Feb 2016 07:42:52 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.210]) by IRSMSX108.ger.corp.intel.com ([169.254.11.102]) with mapi id 14.03.0248.002; Thu, 11 Feb 2016 15:42:51 +0000 From: "Metzger, Markus T" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type Date: Thu, 11 Feb 2016 15:42:50 +0000 Message-ID: References: <1454681922-2228-1-git-send-email-markus.t.metzger@intel.com> <1454681922-2228-3-git-send-email-markus.t.metzger@intel.com> <56B9D620.2020104@redhat.com> <56BA61C6.8060807@redhat.com> <56BB0A0D.80502@redhat.com> <56BB5872.2000604@redhat.com> <56BC8EF0.4000202@redhat.com> In-Reply-To: <56BC8EF0.4000202@redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Pedro Alves > Sent: Thursday, February 11, 2016 2:39 PM > To: Metzger, Markus T > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type > > There are other cases where frame_unwind_caller_xxx callers don't > > check > > frame_unwind_caller_id: > > > > gdb/mips-linux-tdep.c > > gdb/glibc-tdep.c > > gdb/obsd-tdep.c > > gdb/tic6x-linux-tdep.c > > gdb/sol2-tdep.c > > gdb/nios2-linux-tdep.c > > > > They're used for skipping syscalls and ld.so. > > > > The latter should be called via gdbarch_skip_solib_resolver (gdbarch, > > stop_pc) from infrun.c. > > > > Who is supposed to do the check in those cases? Maybe they are already > OK? > > In the syscall cases, we're trying to determine the next PC where to place a > breakpoint, in order to do a software single-step. If we don't know where > the caller is, we can't single-step, so we should probably error out. OTOH, if > the target_ops is record-like and we're single-stepping through the trace log, > we shouldn't be trying to use software single-step at all. So I think those are > probably OK. > > In the glibc_skip_solib_resolver case -- in theory, I guess it would be possible > to construct a branch trace that records a tailcall to _dl_fixup, and that > doesn't have any frame above that one? _dl_runtime_resolve would need to tail-call _dl_fixup. I don't think it can since it needs to (tail-)call the resolved function after _dl_fixup returns. This should be safe, as well, then. > If we don't know where the caller is, we can't skip the resolver in one go, so > best to do is probably to return 0, and let infrun's stepping logic continue > single-stepping. You think we should add the check nevertheless in gdb/glibc-tdep.c? The others I won't be able to test. I could do the changes and rely on buildbot to flag issues. If we really want to change them. I put the "info frame" changes into a separate patch and a test into the tailcall-only.exp test. Here are the stack.c changes: commit 0dc54d83f375e8901061244807e078f281fca070 Author: Markus Metzger Date: Thu Feb 11 11:07:09 2016 +0100 stack: check frame_unwind_caller_id Callers of frame_unwind_caller_* functions are supposed to check frame_unwind_caller_id. Add such a check to frame_info and treat an invalid caller ID as if the caller PC were not available. 2016-02-11 Markus Metzger gdb/ * stack.c (frame_info): Check frame_unwind_caller_id. 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/stack.c b/gdb/stack.c index 89879f3..6e3acc7 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1509,27 +1509,32 @@ frame_info (char *addr_exp, int from_tty) wrap_here (" "); printf_filtered ("saved %s = ", pc_regname); - TRY - { - caller_pc = frame_unwind_caller_pc (fi); - caller_pc_p = 1; - } - CATCH (ex, RETURN_MASK_ERROR) + if (!frame_id_p (frame_unwind_caller_id (fi))) + val_print_unavailable (gdb_stdout); + else { - switch (ex.error) + TRY { - case NOT_AVAILABLE_ERROR: - val_print_unavailable (gdb_stdout); - break; - case OPTIMIZED_OUT_ERROR: - val_print_not_saved (gdb_stdout); - break; - default: - fprintf_filtered (gdb_stdout, _(""), ex.message); - break; + caller_pc = frame_unwind_caller_pc (fi); + caller_pc_p = 1; } + CATCH (ex, RETURN_MASK_ERROR) + { + switch (ex.error) + { + case NOT_AVAILABLE_ERROR: + val_print_unavailable (gdb_stdout); + break; + case OPTIMIZED_OUT_ERROR: + val_print_not_saved (gdb_stdout); + break; + default: + fprintf_filtered (gdb_stdout, _(""), ex.message); + break; + } + } + END_CATCH } - END_CATCH if (caller_pc_p) fputs_filtered (paddress (gdbarch, caller_pc), gdb_stdout);