From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112951 invoked by alias); 5 Feb 2016 08:23:16 -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 112932 invoked by uid 89); 5 Feb 2016 08:23:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:169.254.2, Hx-languages-length:2764, commercial, Tel 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; Fri, 05 Feb 2016 08:23:14 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 05 Feb 2016 00:23:12 -0800 X-ExtLoop1: 1 Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga001.jf.intel.com with ESMTP; 05 Feb 2016 00:23:11 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.210]) by IRSMSX102.ger.corp.intel.com ([169.254.2.97]) with mapi id 14.03.0248.002; Fri, 5 Feb 2016 08:23:10 +0000 From: "Metzger, Markus T" To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH] btrace, frame: fix crash in get_frame_type Date: Fri, 05 Feb 2016 08:23:00 -0000 Message-ID: References: <1453828132-2319-1-git-send-email-markus.t.metzger@intel.com> <56B375EE.7020407@redhat.com> In-Reply-To: <56B375EE.7020407@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/msg00136.txt.bz2 > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Pedro Alves > Sent: Thursday, February 4, 2016 5:02 PM > To: Metzger, Markus T > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH] btrace, frame: fix crash in get_frame_type Hi Pedro, Thanks for your review. > > The comment on skip_artificial_frames says > > > > /* Given FRAME, return the enclosing frame as found in real frames read= -in > from > > inferior memory. Skip any previous frames which were made up by GDB. > > Return the original frame if no immediate previous frames exist. > > */ > > > > That last part, "return the original frame if no immediate previous > > frames exist", is missing. I added that. >=20 > Not sure about this. Why does it make sense to return the original frame? > It sounds arbitrary -- could just as well be the outermost? What does the > caller in question do with it, and why is it correct? Looks like I misinterpreted the comment. I first thought (without checking, my bad) that someone had accidentally removed that part without updating the comment. I now think that the comment should rather be read as "If the argument frame is not an artificial frame, return that". The function (originally called skip_inlined_frames) was never able to handle frame chains that didn't end with a normal frame. Let me check the various callers. I'm inclined to return NULL in this case. > > /* Ignore TAILCALL_FRAME type frames, they were executed already > before > > entering THISFRAME. */ > > - while (get_frame_type (prev_frame) =3D=3D TAILCALL_FRAME) > > + while (prev_frame !=3D NULL && get_frame_type (prev_frame) =3D=3D > > + TAILCALL_FRAME) > > prev_frame =3D get_prev_frame (prev_frame); > > > > + /* We cannot pop tailcall frames. */ if (prev_frame =3D=3D NULL) > > + error (_("Cannot pop tailcall frame(s).")); > > + >=20 > How about factoring that out to a skip_tailcall_frames function, similar = to > skip_artificial_frames, and then do: >=20 > prev_frame =3D skip_tailcall_frames (prev_frame); > if (prev_frame =3D=3D NULL) > error (_("Cannot pop tailcall frame(s).")); >=20 > here and similarly in the other case. >=20 > And I wonder whether we should be using get_prev_frame_always for this > too, like skip_artificial_frames uses. I can try that. I'll split the patch as those changes are unrelated. 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