From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11454 invoked by alias); 9 Feb 2016 14:43:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 11430 invoked by uid 89); 9 Feb 2016 14:43:03 -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=ouch, Hx-languages-length:3517, drill X-HELO: mga11.intel.com Received: from mga11.intel.com (HELO mga11.intel.com) (192.55.52.93) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Feb 2016 14:43:01 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 09 Feb 2016 06:43:00 -0800 X-ExtLoop1: 1 Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by fmsmga002.fm.intel.com with ESMTP; 09 Feb 2016 06:42:59 -0800 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by irsmsx105.ger.corp.intel.com (163.33.3.28) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 9 Feb 2016 14:42:58 +0000 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.210]) by irsmsx112.ger.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0248.002; Tue, 9 Feb 2016 14:42:58 +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: Tue, 09 Feb 2016 14:43:00 -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> In-Reply-To: <56B9D620.2020104@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00235.txt.bz2 > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, February 9, 2016 1:06 PM > To: Metzger, Markus T > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type >=20 > On 02/05/2016 02:18 PM, Markus Metzger wrote: >=20 > > diff --git a/gdb/frame.c b/gdb/frame.c index 6ab8834..cea7003 100644 > > --- a/gdb/frame.c > > +++ b/gdb/frame.c > > @@ -420,7 +420,8 @@ fprint_frame (struct ui_file *file, struct > > frame_info *fi) > > > > /* Given FRAME, return the enclosing frame as found in real frames rea= d- > in from > > inferior memory. Skip any previous frames which were made up by GD= B. > > - Return the original frame if no immediate previous frames exist. */ > > + Return FRAME if FRAME is a non-artificial frame. > > + Return NULL if FRAME is NULL or the start of an artificial-only > > + chain. */ >=20 > Missing double-space after period. But, most importantly, I'm not sure I= like > that this accepts a NULL frame. Fine with me. > > @@ -511,11 +517,12 @@ frame_unwind_caller_id (struct frame_info > *next_frame) > > requests the frame ID of "main()"s caller. */ > > > > next_frame =3D skip_artificial_frames (next_frame); > > - this_frame =3D get_prev_frame_always (next_frame); > > - if (this_frame) > > - return get_frame_id (skip_artificial_frames (this_frame)); > > - else > > + if (next_frame =3D=3D NULL) > > return null_frame_id; > > + > > + this_frame =3D get_prev_frame_always (next_frame); this_frame =3D > > + skip_artificial_frames (this_frame); return get_frame_id > > + (this_frame); > > } > > > > const struct frame_id null_frame_id =3D { 0 }; /* All zeros. */ @@ > > -795,6 +802,9 @@ frame_find_by_id (struct frame_id id) static > > CORE_ADDR frame_unwind_pc (struct frame_info *this_frame) { > > + if (this_frame =3D=3D NULL) > > + throw_error (NOT_AVAILABLE_ERROR, _("PC not available")); >=20 > How can this happen? One of its callers, frame_unwind_caller_pc, calls it with the result of skip_artificial_frames like this: CORE_ADDR frame_unwind_caller_pc (struct frame_info *this_frame) { return frame_unwind_pc (skip_artificial_frames (this_frame)); } Rather than handling the skip_artificial_frames() NULL return here, I made frame_unwind_pc handle a NULL frame argument. I can move the check into frame_unwind_caller_pc if you prefer. > > + /* If the frame chain consists only of artificial frames, use > NEXT_FRAME's. > > + > > + For tailcall frames, we (i.e. the DWARF frame unwinder) assume th= at > they > > + have the gdbarch of the bottom (callee) frame. If NEXT_FRAME is = an > > + artificial frame, as well, we should drill down to the bottom in > > + frame_unwind_arch either directly via the tailcall unwinder or vi= a a > chain > > + of get_frame_arch calls. */ > > + caller =3D skip_artificial_frames (next_frame); if (caller =3D=3D N= ULL) > > + get_frame_arch (next_frame); > > + > > + return frame_unwind_arch (next_frame); >=20 > Hmm, this looks odd. Is the get_frame_arch call there for side effects, = or did > you mean to make that "return get_frame_arch" ? Ouch. Yes, I meant to return that frame. 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