From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16457 invoked by alias); 8 Feb 2013 18:27:51 -0000 Received: (qmail 16433 invoked by uid 22791); 8 Feb 2013 18:27:49 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Feb 2013 18:27:42 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r18IRff7025005 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 8 Feb 2013 13:27:41 -0500 Received: from host2.jankratochvil.net (ovpn-116-18.ams2.redhat.com [10.36.116.18]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r18IRZ06026455 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 8 Feb 2013 13:27:37 -0500 Date: Fri, 08 Feb 2013 18:27:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [3/3] unconditionally call via SYMBOL_COMPUTED_OPS Message-ID: <20130208182734.GA17552@host2.jankratochvil.net> References: <871udlhzzb.fsf@fleche.redhat.com> <20130207163233.GA15297@host2.jankratochvil.net> <87d2wbt445.fsf@fleche.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87d2wbt445.fsf@fleche.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2013-02/txt/msg00220.txt.bz2 On Fri, 08 Feb 2013 12:52:10 +0100, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil writes: > > Jan> Without the full virtualization / unconditional calls via > Jan> SYMBOL_COMPUTED_OPS I do not understand this patch and I am against > Jan> it. > > This was troubling me yesterday and I finally remembered that I did > implement the needed change, but in a non-obvious way. I now think > either your analysis is incorrect or I didn't understand it. I was wrong as I see. IIUC one of the goals of this patchset is that now one can incrementally start to virtualize each LOC_* handling by filling in separate SYMBOL_COMPUTED_OPS vector for each LOC_* value. > Ah, I see. Your follow-on patch restored the vtable in all cases. > This isn't correct according to the approach of patch #3 and the design > implied by the comments and the PR. Yes, my previous patch broke what your patches have achieved. I have filled in the missing frame base cleanup, now is the frame base vs. location separation better understandable IMO (or maybe just to me, not sure). No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu. Thanks, Jan gdb/ 2013-02-08 Jan Kratochvil Code cleanup. * dwarf2loc.c (locexpr_find_frame_base_location) (dwarf2_block_frame_base_locexpr_funcs) (loclist_find_frame_base_location) (dwarf2_block_frame_base_loclist_funcs): New. (dwarf_expr_frame_base_1): Call SYMBOL_BLOCK_OPS, remove internal_error. (dwarf2_locexpr_funcs, dwarf2_loclist_funcs): Add location_has_loclist. * dwarf2loc.h (dwarf2_locexpr_block_index, dwarf2_loclist_block_index): Remove. (dwarf2_block_frame_base_locexpr_funcs) (dwarf2_block_frame_base_loclist_funcs): New. * dwarf2read.c (dwarf2_locexpr_block_index, dwarf2_loclist_block_index): Make them static. (var_decode_location): Use location_has_loclist. (_initialize_dwarf2_read): Replace register_symbol_alias_impl by register_symbol_block_impl. * symtab.c (register_symbol_alias_impl): Remove. (register_symbol_block_impl): New. * symtab.h (struct symbol_computed_ops): Add location_has_loclist. (struct symbol_block_ops): New. (struct symbol_impl): Add ops_block. (SYMBOL_BLOCK_OPS): New. (register_symbol_alias_impl): Remove declaration. (register_symbol_block_impl): New declaration. diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 6047f09..206a318 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -357,34 +357,59 @@ dwarf_expr_frame_base (void *baton, const gdb_byte **start, size_t * length) start, length); } +/* Implement find_frame_base_location method for LOC_BLOCK functions using + DWARF expression for its DW_AT_frame_base. */ + +static void +locexpr_find_frame_base_location (struct symbol *framefunc, CORE_ADDR pc, + const gdb_byte **start, size_t *length) +{ + struct dwarf2_locexpr_baton *symbaton = SYMBOL_LOCATION_BATON (framefunc); + + *length = symbaton->size; + *start = symbaton->data; +} + +/* Vector for inferior functions as represented by LOC_BLOCK, if the inferior + function uses DWARF expression for its DW_AT_frame_base. */ + +const struct symbol_block_ops dwarf2_block_frame_base_locexpr_funcs = +{ + locexpr_find_frame_base_location +}; + +/* Implement find_frame_base_location method for LOC_BLOCK functions using + DWARF location list for its DW_AT_frame_base. */ + +static void +loclist_find_frame_base_location (struct symbol *framefunc, CORE_ADDR pc, + const gdb_byte **start, size_t *length) +{ + struct dwarf2_loclist_baton *symbaton = SYMBOL_LOCATION_BATON (framefunc); + + *start = dwarf2_find_location_expression (symbaton, length, pc); +} + +/* Vector for inferior functions as represented by LOC_BLOCK, if the inferior + function uses DWARF location list for its DW_AT_frame_base. */ + +const struct symbol_block_ops dwarf2_block_frame_base_loclist_funcs = +{ + loclist_find_frame_base_location +}; + static void dwarf_expr_frame_base_1 (struct symbol *framefunc, CORE_ADDR pc, const gdb_byte **start, size_t *length) { - if (SYMBOL_LOCATION_BATON (framefunc) == NULL) - *length = 0; - else if (SYMBOL_ACLASS_INDEX (framefunc) == dwarf2_loclist_block_index) + if (SYMBOL_BLOCK_OPS (framefunc) != NULL) { - struct dwarf2_loclist_baton *symbaton; + const struct symbol_block_ops *ops_block = SYMBOL_BLOCK_OPS (framefunc); - symbaton = SYMBOL_LOCATION_BATON (framefunc); - *start = dwarf2_find_location_expression (symbaton, length, pc); - } - else if (SYMBOL_ACLASS_INDEX (framefunc) == dwarf2_locexpr_block_index) - { - struct dwarf2_locexpr_baton *symbaton; - - symbaton = SYMBOL_LOCATION_BATON (framefunc); - if (symbaton != NULL) - { - *length = symbaton->size; - *start = symbaton->data; - } - else - *length = 0; + ops_block->find_frame_base_location (framefunc, pc, start, length); } else - internal_error (__FILE__, __LINE__, _("invalid function aclass index")); + *length = 0; if (*length == 0) error (_("Could not find the frame base for \"%s\"."), @@ -3978,6 +4003,7 @@ const struct symbol_computed_ops dwarf2_locexpr_funcs = { locexpr_read_variable_at_entry, locexpr_read_needs_frame, locexpr_describe_location, + 0, /* location_has_loclist */ locexpr_tracepoint_var_ref }; @@ -4156,6 +4182,7 @@ const struct symbol_computed_ops dwarf2_loclist_funcs = { loclist_read_variable_at_entry, loclist_read_needs_frame, loclist_describe_location, + 1, /* location_has_loclist */ loclist_tracepoint_var_ref }; diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h index 9060a65..78448e6 100644 --- a/gdb/dwarf2loc.h +++ b/gdb/dwarf2loc.h @@ -133,9 +133,8 @@ struct dwarf2_loclist_baton extern const struct symbol_computed_ops dwarf2_locexpr_funcs; extern const struct symbol_computed_ops dwarf2_loclist_funcs; -/* Two variables from dwarf2read.c. */ -extern int dwarf2_locexpr_block_index; -extern int dwarf2_loclist_block_index; +extern const struct symbol_block_ops dwarf2_block_frame_base_locexpr_funcs; +extern const struct symbol_block_ops dwarf2_block_frame_base_loclist_funcs; /* Compile a DWARF location expression to an agent expression. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 6086051..43b92e5 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -96,8 +96,8 @@ static const struct objfile_data *dwarf2_objfile_data_key; static int dwarf2_locexpr_index; static int dwarf2_loclist_index; -int dwarf2_locexpr_block_index; -int dwarf2_loclist_block_index; +static int dwarf2_locexpr_block_index; +static int dwarf2_loclist_block_index; struct dwarf2_section_info { @@ -15748,7 +15748,7 @@ var_decode_location (struct attribute *attr, struct symbol *sym, dwarf2_symbol_mark_computed (attr, sym, cu, 0); - if (SYMBOL_COMPUTED_OPS (sym) == &dwarf2_loclist_funcs) + if (SYMBOL_COMPUTED_OPS (sym)->location_has_loclist) cu->has_loclist = 1; } @@ -20709,6 +20709,8 @@ Usage: save gdb-index DIRECTORY"), dwarf2_loclist_index = register_symbol_computed_impl (LOC_COMPUTED, &dwarf2_loclist_funcs); - dwarf2_locexpr_block_index = register_symbol_alias_impl (LOC_BLOCK); - dwarf2_loclist_block_index = register_symbol_alias_impl (LOC_BLOCK); + dwarf2_locexpr_block_index = register_symbol_block_impl (LOC_BLOCK, + &dwarf2_block_frame_base_locexpr_funcs); + dwarf2_loclist_block_index = register_symbol_block_impl (LOC_BLOCK, + &dwarf2_block_frame_base_loclist_funcs); } diff --git a/gdb/symtab.c b/gdb/symtab.c index 11ee5b0..685f1ff 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -5089,17 +5089,25 @@ register_symbol_computed_impl (enum address_class aclass, return result; } -/* Register a symbol type to be recognized by its index. ACLASS can be - anything. This returns the new index, which should be used as the - aclass_index field for symbols of this type. */ +/* Register a function with frame base type. ACLASS must be LOC_BLOCK. + OPS is the ops vector associated with this index. This returns the + new index, which should be used as the aclass_index field for symbols + of this type. */ int -register_symbol_alias_impl (enum address_class aclass) +register_symbol_block_impl (enum address_class aclass, + const struct symbol_block_ops *ops) { int result = next_aclass_value++; + gdb_assert (aclass == LOC_BLOCK); gdb_assert (result < MAX_SYMBOL_IMPLS); symbol_impl[result].aclass = aclass; + symbol_impl[result].ops_block = ops; + + /* Sanity check OPS. */ + gdb_assert (ops != NULL); + gdb_assert (ops->find_frame_base_location != NULL); return result; } diff --git a/gdb/symtab.h b/gdb/symtab.h index 293c443..c69f39e 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -575,6 +575,9 @@ struct symbol_computed_ops void (*describe_location) (struct symbol * symbol, CORE_ADDR addr, struct ui_file * stream); + /* Non-zero if this symbol's address computation is dependent on PC. */ + unsigned char location_has_loclist; + /* Tracepoint support. Append bytecodes to the tracepoint agent expression AX that push the address of the object SYMBOL. Set VALUE appropriately. Note --- for objects in registers, this @@ -586,6 +589,20 @@ struct symbol_computed_ops struct agent_expr *ax, struct axs_value *value); }; +/* The methods needed to implement LOC_BLOCK for inferior functions. + These methods can use the symbol's .aux_value for additional + per-symbol information. */ + +struct symbol_block_ops +{ + /* Fill in *START and *LENGTH with DWARF block data of function + FRAMEFUNC valid for inferior context address PC. Set *LENGTH to + zero if such location is not valid for PC; *START is left + uninitialized in such case. */ + void (*find_frame_base_location) (struct symbol *framefunc, CORE_ADDR pc, + const gdb_byte **start, size_t *length); +}; + /* Functions used with LOC_REGISTER and LOC_REGPARM_ADDR. */ struct symbol_register_ops @@ -603,6 +620,9 @@ struct symbol_impl /* Used with LOC_COMPUTED. */ const struct symbol_computed_ops *ops_computed; + /* Used with LOC_BLOCK. */ + const struct symbol_block_ops *ops_block; + /* Used with LOC_REGISTER and LOC_REGPARM_ADDR. */ const struct symbol_register_ops *ops_register; }; @@ -697,13 +717,15 @@ extern const struct symbol_impl *symbol_impls; #define SYMBOL_LINE(symbol) (symbol)->line #define SYMBOL_SYMTAB(symbol) (symbol)->symtab #define SYMBOL_COMPUTED_OPS(symbol) (SYMBOL_IMPL (symbol).ops_computed) +#define SYMBOL_BLOCK_OPS(symbol) (SYMBOL_IMPL (symbol).ops_block) #define SYMBOL_REGISTER_OPS(symbol) (SYMBOL_IMPL (symbol).ops_register) #define SYMBOL_LOCATION_BATON(symbol) (symbol)->aux_value extern int register_symbol_computed_impl (enum address_class, const struct symbol_computed_ops *); -extern int register_symbol_alias_impl (enum address_class); +extern int register_symbol_block_impl (enum address_class aclass, + const struct symbol_block_ops *ops); extern int register_symbol_register_impl (enum address_class, const struct symbol_register_ops *);