From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7478 invoked by alias); 6 Nov 2013 13:03:58 -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 7455 invoked by uid 89); 6 Nov 2013 13:03:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mga14.intel.com Received: from Unknown (HELO mga14.intel.com) (143.182.124.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Nov 2013 13:03:21 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga102.ch.intel.com with ESMTP; 06 Nov 2013 05:03:13 -0800 X-ExtLoop1: 1 Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by azsmga001.ch.intel.com with ESMTP; 06 Nov 2013 05:03:07 -0800 Received: from irsmsx152.ger.corp.intel.com (163.33.192.66) by IRSMSX103.ger.corp.intel.com (163.33.3.157) with Microsoft SMTP Server (TLS) id 14.3.123.3; Wed, 6 Nov 2013 13:03:06 +0000 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.89]) by IRSMSX152.ger.corp.intel.com ([163.33.192.66]) with mapi id 14.03.0123.003; Wed, 6 Nov 2013 13:03:06 +0000 From: "Metzger, Markus T" To: Jan Kratochvil CC: "gdb-patches@sourceware.org" Subject: RE: [patch v6 18/21] record-btrace: extend unwinder Date: Wed, 06 Nov 2013 13:45:00 -0000 Message-ID: References: <1379676639-31802-1-git-send-email-markus.t.metzger@intel.com> <1379676639-31802-19-git-send-email-markus.t.metzger@intel.com> <20131006194943.GD28020@host2.jankratochvil.net> In-Reply-To: <20131006194943.GD28020@host2.jankratochvil.net> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00119.txt.bz2 > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Jan Kratochvil > Sent: Sunday, October 06, 2013 9:50 PM Hello Jan, Thanks for your feedback. I've been busy otherwise the last few weeks; now I'm switching back to the btrace series. > > + while (bfun->segment.prev !=3D NULL) > > + bfun =3D bfun->segment.prev; > > + > > + stack =3D 0; >=20 > As discussed elsewhere I do not find correct to set stack_p =3D 1 && stac= k =3D 0. > I think frame_id_p() can return true even if special_p =3D=3D 1 but stack= _p =3D=3D 0. > You sure need some new function similar to frame_id_build_special(). Besides frame_id_p I also had to change frame_id_eq. Here's how it look like. Are you OK with such changes? Should I put the frame.[hc] changes i= nto a separate patch, as well? I had to make the first test in frame_id_eq stricter than it was since otherwise all artificial frames would have been equal to each other. I ran the full test suite without regressions (native on 64bit IA FC19), bu= t I'm Not exactly sure what has been tested. The report lists 34 untested and 133 unsupported for both runs. diff --git a/gdb/frame.c b/gdb/frame.c index 3157167..e8dd029 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -491,6 +491,19 @@ frame_id_build_wild (CORE_ADDR stack_addr) return id; } =20 +struct frame_id +frame_id_build_artificial (CORE_ADDR code_addr, + CORE_ADDR special_addr) +{ + struct frame_id id =3D null_frame_id; + + id.code_addr =3D code_addr; + id.code_addr_p =3D 1; + id.special_addr =3D special_addr; + id.special_addr_p =3D 1; + return id; +} + int frame_id_p (struct frame_id l) { @@ -501,6 +514,9 @@ frame_id_p (struct frame_id l) /* outer_frame_id is also valid. */ if (!p && memcmp (&l, &outer_frame_id, sizeof (l)) =3D=3D 0) p =3D 1; + /* An artificial frame is also valid. */ + if (!p && l.code_addr_p && l.special_addr_p) + p =3D 1; if (frame_debug) { fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l=3D"); @@ -524,13 +540,11 @@ frame_id_eq (struct frame_id l, struct frame_id r) { int eq; =20 - if (!l.stack_addr_p && l.special_addr_p - && !r.stack_addr_p && r.special_addr_p) - /* The outermost frame marker is equal to itself. This is the - dodgy thing about outer_frame_id, since between execution steps - we might step into another function - from which we can't - unwind either. More thought required to get rid of - outer_frame_id. */ + if (memcmp (&l, &r, sizeof (l)) =3D=3D 0) + /* Every frame is equal to itself. + This is the dodgy thing about outer_frame_id, since between executi= on + steps we might step into another function - from which we can't unw= ind + either. More thought required to get rid of outer_frame_id. */ eq =3D 1; else if (!l.stack_addr_p || !r.stack_addr_p) /* Like a NaN, if either ID is invalid, the result is false. diff --git a/gdb/frame.h b/gdb/frame.h index 7109dbc..9fd278c 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -174,6 +174,12 @@ extern struct frame_id frame_id_build_special (CORE_AD= DR stack_addr, as the special identifier address are set to indicate wild cards. */ extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr); =20 +/* Construct an artificial frame ID. The first parameter is the frame's + constant code address (typically the entry point), and the second the + frame's special identifier address. */ +extern struct frame_id frame_id_build_artificial (CORE_ADDR code_addr, + CORE_ADDR special_addr); + /* Returns non-zero when L is a valid frame (a valid frame has a non-zero .base). The outermost frame is valid even without an ID. */ diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index 02b5e3d..63f09c6 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -1036,7 +1036,7 @@ record_btrace_frame_this_id (struct frame_info *this_= frame, void **this_cache, { const struct btrace_frame_cache *cache; const struct btrace_function *bfun; - CORE_ADDR stack, code, special; + CORE_ADDR code, special; =20 cache =3D *this_cache; =20 @@ -1046,11 +1046,10 @@ record_btrace_frame_this_id (struct frame_info *thi= s_frame, void **this_cache, while (bfun->segment.prev !=3D NULL) bfun =3D bfun->segment.prev; =20 - stack =3D 0; code =3D get_frame_func (this_frame); special =3D bfun->number; =20 - *this_id =3D frame_id_build_special (stack, code, special); + *this_id =3D frame_id_build_artificial (code, special); =20 DEBUG ("[frame] %s id: (!stack, pc=3D%s, special=3D%s)", btrace_get_bfun_name (cache->bfun), > > + code =3D get_frame_func (this_frame); > > + special =3D (CORE_ADDR) bfun; >=20 > This is not safe, GDB host may be 64-bit and target 32-bit and in such ca= se > without --enable-64-bit-bfd there will be sizeof (CORE_ADDR) =3D=3D 4, > therefore different BFUNs may get the same SPECIAL. > bfun->insn_offset or bfun->insn_offset should serve better I think. I changed this to use bfun->number, instead. > Also there could be a comment like (it still applies if one uses any of b= fun, > bfun->insn_offset or bfun->insn_offset): > /* The btrace_function structures can be rebuilt but only after inferio= r has > run. In such case all btrace frame have been deleted and there rema= in no > stale uses of BFUN addresses. */ Doesn't gdb keep frame id's alive during stepping? I thought they were used to detect steps into subroutines. We could end up with the same frame id but a different frame if we recomputed the entire trace (i.e. the trace buffer overflows and we can't stitch traces) and we happened to have the same function at the same position in the trace. But this might also happen for all other frame id's. Regards, Markus. Intel GmbH Dornacher Strasse 1 85622 Feldkirchen/Muenchen, Deutschland Sitz der Gesellschaft: Feldkirchen bei Muenchen Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk Registergericht: Muenchen HRB 47456 Ust.-IdNr./VAT Registration No.: DE129385895 Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052