Re: RFC: partially available registers

["Ulrich Weigand" <uweigand@de.ibm.com>]

Tom Tromey wrote:

> 2011-07-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* amd64-tdep.c (amd64_pseudo_register_read_into_value): Rename
> 	from amd64_pseudo_register_read.  Change arguments.  Call
> 	mark_value_bytes_unavailable when needed.
> 	(amd64_init_abi): Use set_gdbarch_pseudo_register_read_value, not
> 	set_gdbarch_pseudo_register_read.
> 	* sentinel-frame.c (sentinel_frame_prev_register): Use
> 	regcache_cooked_read_into_value.
> 	* regcache.h (regcache_cooked_read_into_value): Declare.
> 	* regcache.c (regcache_cooked_read_into_value): New function.
> 	(regcache_cooked_read): Call
> 	gdbarch_pseudo_register_read_into_value if available.
> 	* i386-tdep.h (i386_pseudo_register_read_into_value): Declare.
> 	(i386_pseudo_register_read): Remove.
> 	* i386-tdep.c (i386_pseudo_register_read_into_value): Rename from
> 	i386_pseudo_register_read.  Change arguments.  Call
> 	mark_value_bytes_unavailable when needed.
> 	(i386_gdbarch_init): Call
> 	set_gdbarch_pseudo_register_read_into_value, not
> 	set_gdbarch_pseudo_register_read.
> 	* gdbarch.sh (pseudo_register_read_into_value): New method.
> 	* gdbarch.c, gdbarch.h: Rebuild.
> 	* findvar.c (value_from_register): Call get_frame_register_value.

I'm not completely happy about the "read into value" style interface;
this leaves unclear just how the value passed into that interface is
supposed to have been set up.  Should the callback have to cope with
different types (or even lengths), value offset, or more complex
value properties like enclosing type?  Apparently not -- the actual
callback implementations of your don't attempt to with (or even
check for!) any of that, they just blindly assume the value has been
set up as they expect.

I'd prefer an interface that simply *returns* a value, which then the
callback would be free to set up as desired.  This would also be much
more in line with all the other interfaces that produce values ...

> @@ -647,14 +647,16 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
>    else
>      {
>        int len = TYPE_LENGTH (type);
> +      struct value *v2;
>  
>        /* Construct the value.  */
>        v = gdbarch_value_from_register (gdbarch, type, regnum, frame);
>  
>        /* Get the data.  */
> -      ok = get_frame_register_bytes (frame, regnum, value_offset (v), len,
> -				     value_contents_raw (v),
> -				     &optim, &unavail);
> +      v2 = get_frame_register_value (frame, regnum);
> +
> +      value_contents_copy (v, 0, v2, 0, len);
> +      ok = 1;
>      }

This seems to introduce a bug by ignoring value_offset (v).
If v is of a smaller type than the full register width, only
the appropriate bytes of v2 (which do not always start at the
beginning, in particular on big-endian machines) ought to be
copied over to v.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com