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

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]

Hi Pedro,

> This is a bit a step backwards in that it doesn't allow
> marking parts of the value as unavailable when the type
> is longer than one register.  get_frame_register_value
> was invented to allow for partially available registers.
> 
> > --- a/gdb/findvar.c
> > +++ b/gdb/findvar.c
> > @@ -668,9 +668,35 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
> >        v = gdbarch_value_from_register (gdbarch, type, regnum, frame);
> >  
> >        /* Get the data.  */
> > -      v2 = get_frame_register_value (frame, regnum);
> > +      if (len > register_size (gdbarch, regnum))
> > +       {
> 
> I'd rather we get rid of get_frame_register_bytes.

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.

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.

Other two uses that are different:
  - dwarf2loc.c: For DW_OP_piece (read/write) support;
  - 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.

Thanks,
-- 
Joel

[-- Attachment #2: multi-register-value.diff --]
[-- Type: text/x-diff, Size: 2807 bytes --]

commit 1c14f39bb70a05475bb9dfaf5e21103772a9605f
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Fri Oct 21 14:34:54 2011 -0700

    handle variables stored in muliple consecutive registers
    
    gdb/ChangeLog:
    
            * value.c (read_frame_register_value): New function.
            * findvar.c (value_from_register): Use read_frame_register_value
            instead of get_frame_register_value + value_contents_copy
            to get value contents.

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 8e986f1..a417c02 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -661,16 +661,11 @@ 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.  */
-      v2 = get_frame_register_value (frame, regnum);
-
-      value_contents_copy (v, 0, v2, value_offset (v), len);
+      read_frame_register_value (v, frame);
     }
 
   return v;
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)
+{
+  int offset = 0;
+  int regnum = value->regnum;
+  const int len = TYPE_LENGTH (value_type (value));
+
+  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++;
+    }
+}
+
 /* Set the initialized field in a value struct.  */
 
 void
diff --git a/gdb/value.h b/gdb/value.h
index 5d61a0b..a933958 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -493,6 +493,9 @@ extern struct value *value_from_register (struct type *type, int regnum,
 extern CORE_ADDR address_from_register (struct type *type, int regnum,
 					struct frame_info *frame);
 
+extern void read_frame_register_value (struct value *value,
+				       struct frame_info *frame);
+
 extern struct value *value_of_variable (struct symbol *var, struct block *b);
 
 extern struct value *address_of_variable (struct symbol *var, struct block *b);