This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [drow@mvista.com: Re: RFA: LOC_COMPUTED support]


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 <drow at mvista dot com> 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  <drow at mvista dot com>
> 
> 	Based on a patch from Daniel Berlin (dberlin at dberlin dot 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, &reg);
> +	  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)'?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]