Re: [RFA/RFC] Restore old handling of multi-register variables

[Pedro Alves <pedro at codesourcery dot com>]

On Saturday 22 October 2011 00:38:02, Joel Brobecker wrote:

> Purely in terms of solving the AVR problem, what do you think
> of the attached patch? Does it look correct to you?
> 
> I tested it on AVR as well as x86_64-linux, and it seems to work.
> 
> Going beyond that, the new function doesn't provide the extended
> interface that you suggest. Doing so seems to be complicating
> the implementation more than it's worth. I think that what we
> should do, we want to eliminate get_frame_register_value, is look
> at the current uses and try to eliminate them.

Yeah.

> The biggest culprit is the register_to_value gdbarch method (11
> hits). But there is only one location where it's actually called,
> and it is.... value_from_register! (just above the section of code
> that we're improving). I think it would be easy to change the
> profile of that method to return a value. 
> Then the register_to_value implementations could use get_frame_register_value instead.

Yeah, though I suspect that your new read_frame_register_value
method may evolve into looking what I suggested anyway.  :-)

> Other two uses that are different:
>   - dwarf2loc.c: For DW_OP_piece (read/write) support;

Looks fixable.  Hmm, I like this.

>   - spu-tdep.c: We just read the contents of a single register
>     (get_frame_register_value + extract_unsigned_integer,
>     so it should be easy to replace them with something else.


> diff --git a/gdb/value.c b/gdb/value.c
> index 087cdfd..8dc9258 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3140,6 +3140,35 @@ using_struct_return (struct gdbarch *gdbarch,
>           != RETURN_VALUE_REGISTER_CONVENTION);
>  }
>  
> +/* VALUE must be an lval_register value.  If regnum is the value's
> +   associated register number, and len the length of the values type,
> +   read one or more registers in FRAME, starting with register REGNUM,
> +   until we've read LEN bytes.  */
> +
> +void
> +read_frame_register_value (struct value *value, struct frame_info *frame)
> +{

I think this should be in frame.c instead.  value.c is for core
struct value stuff.

> +  int offset = 0;
> +  int regnum = value->regnum;
> +  const int len = TYPE_LENGTH (value_type (value));

Do we need check_typedefs here?

> +  gdb_assert (value->lval == lval_register);
> +
> +  while (offset < len)
> +    {
> +      struct value *regval = get_frame_register_value (frame, regnum);
> +      int reg_len = TYPE_LENGTH (value_type (regval));
> +
> +      if (offset + reg_len > len)
> +        reg_len = len - offset;
> +      value_contents_copy (value, offset, regval, value_offset (regval),
> +                          reg_len);
> +
> +      offset += reg_len;
> +      regnum++;
> +    }
> +}

Otherwise looks good to me.  Thanks!

-- 
Pedro Alves