Re: [RFA]: dwarf2expr.[ch]

[Jim Blandy <jimb@redhat.com>]

Daniel Berlin <dberlin@dberlin.org> writes:
> Just some questions and statements:
> > > +   return ctx->stack[ctx->stack_len - (1+n)];
> > > + 
> > > + }     
> > 
> > This should check for underflow, too.  Look at what DW_OP_rot will do
> > on an empty stack.
> It does.
> Look at the lines above it.
> (Yours won't have internal_error, just (ctx->error))
> if (ctx->stack_len < n)
>      internal_error (__FILE__, __LINE__, "Asked for position %d of stack, 
> stack only has %d elements on it\n",
>                      n, ctx->stack_len);
> 
> If you ask for one item, and the stack has 0, this will catch it.

You're right, it does.  I misread 'n' as indexing from bottom to top,
not top to bottom.

> > > + 
> > > + 	  switch (op)
> > > + 	    {
> > > + 	    case DW_OP_deref:
> > > + 	      {
> > > + 		result = (CORE_ADDR) 
> > > + 		  (ctx->read_mem) (ctx->read_mem_baton, 
> > > + 				   result, 
> > > + 				   TARGET_PTR_BIT / TARGET_CHAR_BIT);
> > 
> > Since CORE_ADDR may be wider than the target's address,
> > I think this
> > should mask off and/or sign extend as appropriate, depending on the
> > current gdbarch.  Same anywhere we call ctx->read_mem, I think.
> Shouldn't the read_mem function do this for us?
> read_mem is returning a CORE_ADDR (the cast is pointless, i'll remove 
> it) anyway, so it would seem to be *it's* job to make sure the 
> CORE_ADDR it gives us is the right thing.

I'm worried about about strange addresses being produced because the
evaluator is using stack elements wider than officially specified.
Since the evaluator is the source of the behavior I'm concerned about,
I think it's better to correct it there than to require the
surrounding code in GDB to cope with it.

You're going to need a truncation function for all the operations that
are sensitive to the upper bits anyway (divide, shift right, compare),
so it doesn't seem a big deal to drop in an application here, too.