From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10140 invoked by alias); 20 Feb 2003 23:08:19 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 10111 invoked from network); 20 Feb 2003 23:08:18 -0000 Received: from unknown (HELO zenia.red-bean.com) (66.244.67.22) by 172.16.49.205 with SMTP; 20 Feb 2003 23:08:18 -0000 Received: from zenia.red-bean.com (localhost.localdomain [127.0.0.1]) by zenia.red-bean.com (8.12.5/8.12.5) with ESMTP id h1KN1w8A031048; Thu, 20 Feb 2003 18:01:58 -0500 Received: (from jimb@localhost) by zenia.red-bean.com (8.12.5/8.12.5/Submit) id h1KN1vr3031044; Thu, 20 Feb 2003 18:01:57 -0500 To: Daniel Jacobowitz CC: gdb-patches@sources.redhat.com Subject: Re: [drow@mvista.com: Re: RFA: LOC_COMPUTED support] References: <20030213211157.GA13537@nevyn.them.org> From: Jim Blandy Date: Thu, 20 Feb 2003 23:08:00 -0000 In-Reply-To: <20030213211157.GA13537@nevyn.them.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-02/txt/msg00505.txt.bz2 I've only found obvious-fix problems with this, so if you change the things I note here, this can go in. (Whew.) I think I found some switches that still need LOC_COMPUTED{,_ARG} cases: - ada-lang.c (ada_resolve_subexp, symtab_for_sym, ada_add_block_symbols, fill_in_ada_prototype) - symtab.c (lookup_block_symbol) - mi-cmd-stack.c (list_args_or_locals) Daniel Jacobowitz writes: > On Wed, Feb 05, 2003 at 12:24:34AM -0500, Jim Blandy wrote: > > The comment needs to come clean about this: > > > > /* For a LOC_COMPUTED symbol, this is the baton and location_funcs > > structure to find its value. For a LOC_BLOCK symbol for a > > function in a compilation unit compiled with Dwarf 2 information, > > this is information used internally by the Dwarf 2 code --- > > specifically, the Dwarf location description that computes frame > > base for that function. */ > > > > Maybe I'm going too far, but that is at least more honest about how > > messy this is. I'm still not sure I'm cool with this. > > I've adapted your comment. I'd like to leave this for another time, to > be honest... after we kill some of the current excess memory usage in > symbols, like the dead aliases/ranges code which this is obsoleting. > > Another way we could do this would be to make the frame base a magic > symbol in the function's block, and look it up. That's probably a > safer approach; more memory per-function but that doesn't bother me as > much as per-symbol does. And easier to look up, too. Okay. Since we're putting this off for later, can you file a PR, or record the idea for the improvement in a comment the code? > 2003-02-05 Daniel Jacobowitz > > Based on a patch from Daniel Berlin (dberlin@dberlin.org). > * symtab.h: Add opaque declarations of struct axs_value and > struct agent_expr. > (enum address_class): Add LOC_COMPUTED and LOC_COMPUTED_ARG. > (struct location_funcs): New type. > (struct symbol): Add "loc" to aux_value. > (SYMBOL_LOCATION_BATON, SYMBOL_LOCATION_FUNCS): New macros. > * dwarf2read.c: Include "dwarf2expr.h". > (dwarf2_symbol_mark_computed): New function. > (read_func_scope): Use it. > (var_decode_location): New function. > (new_symbol): Use it. > * dwarf2expr.c, dwarf2expr.h, dwarf2loc.c: New files. Don't forget dwarf2loc.h! > +/* Return the type of an address, for unsigned arithmetic. */ > + > +static struct type * > +unsigned_address_type (void) > +{ > + switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT) > + { > + case 4: > + return builtin_type_uint32; > + case 8: > + return builtin_type_uint64; > + default: > + internal_error (__FILE__, __LINE__, > + "Unsupported address size.\n"); > + } > +} > + > +/* Return the type of an address, for signed arithmetic. */ > + > +static struct type * > +signed_address_type (void) > +{ > + switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT) > + { > + case 4: > + return builtin_type_int32; > + case 8: > + return builtin_type_int64; > + default: > + internal_error (__FILE__, __LINE__, > + "Unsupported address size.\n"); > + } > +} These will both need a case for 16 bits, too. I'm pretty sure I know of a 16-bit target that uses Dwarf 2. Also, shouldn't we be using the address size from the compilation unit header, instead of TARGET_ADDR_BIT? (I have this strong impression we've discussed this before, but I've not been able to find it in the ML archives to see what we decided.) On a 16-bit machine in 32-bit ELF, it's not clear to me what the compilation unit header's address size will be: if the architecture has separate code and data address spaces, and we treat them as 64k subranges of a unified 32-bit address space for linking, as is often done, it seems to me the compilation unit header's address size could be 32 bits, since that's what you need to store a symbol's value. Since there's no easy way at the moment to get that compilation unit header address size (it's thrown away after we're done reading Dwarf 2 info, and the baton needs to be small), this could be a PR. > + case DW_OP_reg29: > + case DW_OP_reg30: > + case DW_OP_reg31: > + if (op_ptr != op_end) > + error ("DWARF-2 expression error: DW_OP_reg operations must be " > + "used alone."); > + > + result = (ctx->read_reg) (ctx->baton, op - DW_OP_reg0, &expr_lval, > + &memaddr); > + > + if (expr_lval == lval_register) > + { > + ctx->regnum = op - DW_OP_reg0; > + ctx->in_reg = 1; > + } > + else > + result = memaddr; > + > + break; > + > + case DW_OP_regx: > + op_ptr = read_uleb128 (op_ptr, op_end, ®); > + if (op_ptr != op_end) > + error ("DWARF-2 expression error: DW_OP_reg operations must be " > + "used alone."); > + > + result = (ctx->read_reg) (ctx->baton, reg, &expr_lval, &memaddr); > + > + if (expr_lval == lval_register) > + { > + ctx->regnum = reg; > + ctx->in_reg = 1; > + } > + else > + result = memaddr; > + > + break; This also isn't worth delaying the patch for, but it's something I don't really understand, so I'll ask here before I forget: why do the register name operators have to do anything beyond simply setting ctx->in_reg and ctx->regnum, and returning? The caller should be responsible for figuring out whether that register has been saved on the stack or not, as it already does for all the other register address classes. > Index: dwarf2loc.c ... > +/* This is the baton used when performing dwarf2 expression > + evaluation. */ > +struct dwarf_expr_baton > +{ > + struct symbol *var; > + struct frame_info *frame; > + struct objfile *objfile; > +}; You forgot to delete this definition of 'struct dwarf_expr_baton' from dwarf2loc.c after putting it in dwarf2loc.h. > +/* Using the objfile specified in BATON, find the address for the > + current thread's thread-local storage with offset OFFSET. */ > +static CORE_ADDR > +dwarf_expr_tls_address (void *baton, CORE_ADDR offset) > +{ > + struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton; > + CORE_ADDR addr; > + > + if (target_get_thread_local_address_p ()) > + addr = target_get_thread_local_address (inferior_ptid, > + SYMBOL_OBJFILE (debaton->var), > + offset); Don't you mean 'debaton->objfile', not 'SYMBOL_OBJFILE (debaton->var)'?