From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26253 invoked by alias); 1 Sep 2009 23:07:47 -0000 Received: (qmail 26235 invoked by uid 22791); 1 Sep 2009 23:07:45 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 01 Sep 2009 23:07:36 +0000 Received: from zps76.corp.google.com (zps76.corp.google.com [172.25.146.76]) by smtp-out.google.com with ESMTP id n81N7XYG001541 for ; Tue, 1 Sep 2009 16:07:33 -0700 Received: from ywh12 (ywh12.prod.google.com [10.192.8.12]) by zps76.corp.google.com with ESMTP id n81N7VS4004263 for ; Tue, 1 Sep 2009 16:07:31 -0700 Received: by ywh12 with SMTP id 12so600596ywh.12 for ; Tue, 01 Sep 2009 16:07:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.150.32.1 with SMTP id f1mr11387450ybf.96.1251846450202; Tue, 01 Sep 2009 16:07:30 -0700 (PDT) In-Reply-To: References: Date: Tue, 01 Sep 2009 23:07:00 -0000 Message-ID: Subject: Re: RFC: DW_OP_call_frame_cfa, again From: Doug Evans To: Tom Tromey Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes 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 X-SW-Source: 2009-09/txt/msg00038.txt.bz2 On Tue, Sep 1, 2009 at 11:08 AM, Tom Tromey wrote: >>>>>> "Tom" =3D=3D Tom Tromey writes: > > Tom> Here is a new version of my patch to implement DW_OP_call_frame_cfa. > Tom> Previous thread here: > Tom> =A0 =A0 http://sourceware.org/ml/gdb-patches/2009-06/msg00191.html > Tom> With followup the next month (I wish our mailing list archive handled > Tom> this more nicely): > Tom> =A0 =A0 http://sourceware.org/ml/gdb-patches/2009-07/msg00570.html > > Here is an updated version. =A0This fixes a couple of bugs found by > testing it in Fedora rawhide. > > I'm happy with this version, so I plan to check it in next week or so. > Comment soon if you think it needs some change. > > Tom I hate to nitpick on style issues (much :-)) but since this one bothers me, and I'm not clear on what the rules are, and there are clear rules for other things, I thought I'd bring it up. IWBN IMHO to require a blank line between a function's comment and its definition. It helps readability (to me anyway). I notice this patch has a mixture of both having the blank line and not having it. [I don't mind the absence of a blank line in declarations as much, I've left that for a separate discussion.] > > 2009-09-01 =A0Tom Tromey =A0 > > =A0 =A0 =A0 =A0* frame.h (frame_unwinder_is): Declare. > =A0 =A0 =A0 =A0* frame.c (frame_unwinder_is): New function. > =A0 =A0 =A0 =A0* dwarf2loc.c: Include dwarf2-frame.h. > =A0 =A0 =A0 =A0(dwarf_expr_frame_cfa): New function. > =A0 =A0 =A0 =A0(dwarf2_evaluate_loc_desc): Use it. > =A0 =A0 =A0 =A0(needs_frame_frame_cfa): New function. > =A0 =A0 =A0 =A0(dwarf2_loc_desc_needs_frame): Use it. > =A0 =A0 =A0 =A0* dwarf2expr.h (struct dwarf_expr_context) = : New > =A0 =A0 =A0 =A0field. > =A0 =A0 =A0 =A0* dwarf2expr.c (execute_stack_op) : = New > =A0 =A0 =A0 =A0case. > =A0 =A0 =A0 =A0* dwarf2-frame.h (dwarf2_frame_cfa): Declare. > =A0 =A0 =A0 =A0* dwarf2-frame.c (no_get_frame_cfa): New function. > =A0 =A0 =A0 =A0(execute_stack_op): Use it. > =A0 =A0 =A0 =A0(dwarf2_frame_cfa): New function. > > 2009-09-01 =A0Tom Tromey =A0 > > =A0 =A0 =A0 =A0* gdb.dwarf2/callframecfa.exp: New file. > =A0 =A0 =A0 =A0* gdb.dwarf2/callframecfa.S: New file. > > diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c > index 427f58f..f32a44f 100644 > --- a/gdb/dwarf2-frame.c > +++ b/gdb/dwarf2-frame.c > @@ -309,6 +309,14 @@ no_get_frame_base (void *baton, gdb_byte **start, si= ze_t *length) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0_("Support for DW_OP_fbreg is unimplem= ented")); > =A0} > > +/* Helper function for execute_stack_op. =A0*/ > +static CORE_ADDR > +no_get_frame_cfa (void *baton) > +{ > + =A0internal_error (__FILE__, __LINE__, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 _("Support for DW_OP_call_frame_cfa is = unimplemented")); > +} > + > =A0static CORE_ADDR > =A0no_get_tls_address (void *baton, CORE_ADDR offset) > =A0{ > @@ -363,6 +371,7 @@ execute_stack_op (gdb_byte *exp, ULONGEST len, int ad= dr_size, > =A0 ctx->read_reg =3D read_reg; > =A0 ctx->read_mem =3D read_mem; > =A0 ctx->get_frame_base =3D no_get_frame_base; > + =A0ctx->get_frame_cfa =3D no_get_frame_cfa; > =A0 ctx->get_tls_address =3D no_get_tls_address; > > =A0 dwarf_expr_push (ctx, initial); > @@ -1250,6 +1259,20 @@ dwarf2_frame_base_sniffer (struct frame_info *this= _frame) > > =A0 return NULL; > =A0} > + > +/* Compute the CFA for THIS_FRAME, but only if THIS_FRAME came from > + =A0 the DWARF unwinder. =A0This is used to implement > + =A0 DW_OP_call_frame_cfa. =A0*/ > + > +CORE_ADDR > +dwarf2_frame_cfa (struct frame_info *this_frame) > +{ > + =A0while (get_frame_type (this_frame) =3D=3D INLINE_FRAME) > + =A0 =A0this_frame =3D get_prev_frame (this_frame); > + =A0if (! frame_unwinder_is (this_frame, &dwarf2_frame_unwind)) > + =A0 =A0error (_("can't compute CFA for this frame")); > + =A0return get_frame_base (this_frame); > +} > > =A0const struct objfile_data *dwarf2_frame_objfile_data; > > diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h > index b203661..dd03d59 100644 > --- a/gdb/dwarf2-frame.h > +++ b/gdb/dwarf2-frame.h > @@ -118,4 +118,8 @@ extern const struct frame_base * > > =A0void dwarf2_frame_build_info (struct objfile *objfile); > > +/* Compute the DWARF CFA for a frame. =A0*/ > + > +CORE_ADDR dwarf2_frame_cfa (struct frame_info *this_frame); > + > =A0#endif /* dwarf2-frame.h */ > diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c > index 2721065..6401e72 100644 > --- a/gdb/dwarf2expr.c > +++ b/gdb/dwarf2expr.c > @@ -716,6 +716,10 @@ execute_stack_op (struct dwarf_expr_context *ctx, > =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0break; > > + =A0 =A0 =A0 case DW_OP_call_frame_cfa: > + =A0 =A0 =A0 =A0 result =3D (ctx->get_frame_cfa) (ctx->baton); > + =A0 =A0 =A0 =A0 break; > + > =A0 =A0 =A0 =A0case DW_OP_GNU_push_tls_address: > =A0 =A0 =A0 =A0 =A0/* Variable is at a constant offset in the thread-local > =A0 =A0 =A0 =A0 =A0storage block into the objfile for the current thread = and > diff --git a/gdb/dwarf2expr.h b/gdb/dwarf2expr.h > index 2306e49..6b3a068 100644 > --- a/gdb/dwarf2expr.h > +++ b/gdb/dwarf2expr.h > @@ -55,6 +55,9 @@ struct dwarf_expr_context > =A0 =A0 =A0expression evaluation is complete. =A0*/ > =A0 void (*get_frame_base) (void *baton, gdb_byte **start, size_t *length= ); > > + =A0/* Return the CFA for the frame. =A0*/ > + =A0CORE_ADDR (*get_frame_cfa) (void *baton); > + > =A0 /* Return the thread-local storage address for > =A0 =A0 =A0DW_OP_GNU_push_tls_address. =A0*/ > =A0 CORE_ADDR (*get_tls_address) (void *baton, CORE_ADDR offset); > diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c > index b6c9b11..95ee2f5 100644 > --- a/gdb/dwarf2loc.c > +++ b/gdb/dwarf2loc.c > @@ -36,6 +36,7 @@ > =A0#include "dwarf2.h" > =A0#include "dwarf2expr.h" > =A0#include "dwarf2loc.h" > +#include "dwarf2-frame.h" > > =A0#include "gdb_string.h" > =A0#include "gdb_assert.h" > @@ -194,6 +195,15 @@ dwarf_expr_frame_base (void *baton, gdb_byte **start= , size_t * length) > =A0 =A0 =A0 =A0 =A0 SYMBOL_NATURAL_NAME (framefunc)); > =A0} > > +/* Helper function for dwarf2_evaluate_loc_desc. =A0Computes the CFA for > + =A0 the frame in BATON. =A0*/ > +static CORE_ADDR > +dwarf_expr_frame_cfa (void *baton) > +{ > + =A0struct dwarf_expr_baton *debaton =3D (struct dwarf_expr_baton *) bat= on; > + =A0return dwarf2_frame_cfa (debaton->frame); > +} > + > =A0/* Using the objfile specified in BATON, find the address for the > =A0 =A0current thread's thread-local storage with offset OFFSET. =A0*/ > =A0static CORE_ADDR > @@ -237,6 +247,7 @@ dwarf2_evaluate_loc_desc (struct symbol *var, struct = frame_info *frame, > =A0 ctx->read_reg =3D dwarf_expr_read_reg; > =A0 ctx->read_mem =3D dwarf_expr_read_mem; > =A0 ctx->get_frame_base =3D dwarf_expr_frame_base; > + =A0ctx->get_frame_cfa =3D dwarf_expr_frame_cfa; > =A0 ctx->get_tls_address =3D dwarf_expr_tls_address; > > =A0 dwarf_expr_eval (ctx, data, size); > @@ -331,6 +342,15 @@ needs_frame_frame_base (void *baton, gdb_byte **star= t, size_t * length) > =A0 nf_baton->needs_frame =3D 1; > =A0} > > +/* CFA accesses require a frame. =A0*/ > +static CORE_ADDR > +needs_frame_frame_cfa (void *baton) > +{ > + =A0struct needs_frame_baton *nf_baton =3D baton; > + =A0nf_baton->needs_frame =3D 1; > + =A0return 1; > +} > + > =A0/* Thread-local accesses do require a frame. =A0*/ > =A0static CORE_ADDR > =A0needs_frame_tls_address (void *baton, CORE_ADDR offset) > @@ -363,6 +383,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned= short size, > =A0 ctx->read_reg =3D needs_frame_read_reg; > =A0 ctx->read_mem =3D needs_frame_read_mem; > =A0 ctx->get_frame_base =3D needs_frame_frame_base; > + =A0ctx->get_frame_cfa =3D needs_frame_frame_cfa; > =A0 ctx->get_tls_address =3D needs_frame_tls_address; > > =A0 dwarf_expr_eval (ctx, data, size); > diff --git a/gdb/frame.c b/gdb/frame.c > index 67e0607..3f32471 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -1843,6 +1843,17 @@ get_frame_args_address (struct frame_info *fi) > =A0 return fi->base->this_args (fi, &fi->base_cache); > =A0} > > +/* Return true if the frame base for frame FI is BASE; false > + =A0 otherwise. =A0*/ > + > +int > +frame_unwinder_is (struct frame_info *fi, const struct frame_unwind *unw= inder) > +{ > + =A0if (fi->unwind =3D=3D NULL) > + =A0 =A0fi->unwind =3D frame_unwind_find_by_frame (fi, &fi->prologue_cac= he); > + =A0return fi->unwind =3D=3D unwinder; > +} > + > =A0/* Level of the selected frame: 0 for innermost, 1 for its caller, ... > =A0 =A0or -1 for a NULL frame. =A0*/ > > diff --git a/gdb/frame.h b/gdb/frame.h > index febef5c..611c6d3 100644 > --- a/gdb/frame.h > +++ b/gdb/frame.h > @@ -696,4 +696,10 @@ extern struct frame_info *deprecated_safe_get_select= ed_frame (void); > > =A0extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR = pc); > > +/* Return true if the frame unwinder for frame FI is UNWINDER; false > + =A0 otherwise. =A0*/ > + > +extern int frame_unwinder_is (struct frame_info *fi, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const struct fr= ame_unwind *unwinder); > + > =A0#endif /* !defined (FRAME_H) =A0*/ >