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

[Joel Brobecker <brobecker@adacore.com>]

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

> I have also started looking at converting the register_to_value
> gdbarch method to using values instead of buffer and a couple of
> parameters. Not very difficult, but not trivial either. That makes
> me worried about introducing bugs during the conversion, and
> I won't be able to test most configurations.
> 
> I'll send a patch as a follow-up email to this one.  It's not complete,
> but gives us an idea, and we can decide whether we want to continue
> or not (FYI: I will have very little time for this within the next
> couple of weeks).

So, here is the patch. I only did the conversion for alpha and
i386/i387. It shows the benefits, but it also reveals that the
implementation can be a little awkward, because the support routines
that are used of course where implemented using buffers rather than
values.  And in the end, the transformations need to be performed
on the contents of those buffers, so in their context, it might
even make some kind of sense.

Overall, this still seems like a step in the right direction, but
I am definitely concerned about introducing bugs on platforms that
I cannot test...

So, what do you think we should do?

-- 
Joel

[-- Attachment #2: wip-register_to_value.diff --]
[-- Type: text/x-diff, Size: 10874 bytes --]

commit 50bc176d304fef14f4823670e5ce6075c64419d7
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Wed Oct 26 11:11:56 2011 -0700

    WIP (gdbarch_register_to_value)

diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index b6b9cf8..e67bdec 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -492,7 +492,7 @@ F:int:get_longjmp_target:struct frame_info *frame, CORE_ADDR *pc:frame, pc
 v:int:believe_pcc_promotion:::::::
 #
 m:int:convert_register_p:int regnum, struct type *type:regnum, type:0:generic_convert_register_p::0
-f:int:register_to_value:struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep:frame, regnum, type, buf, optimizedp, unavailablep:0
+f:struct value *:register_to_value:struct frame_info *frame, int regnum, struct type *type:frame, regnum, type:0
 f:void:value_to_register:struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf:frame, regnum, type, buf:0
 # Construct a value representing the contents of register REGNUM in
 # frame FRAME, interpreted as type TYPE.  The routine needs to
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 7fedca7..975377f 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -402,8 +402,8 @@ typedef int (gdbarch_convert_register_p_ftype) (struct gdbarch *gdbarch, int reg
 extern int gdbarch_convert_register_p (struct gdbarch *gdbarch, int regnum, struct type *type);
 extern void set_gdbarch_convert_register_p (struct gdbarch *gdbarch, gdbarch_convert_register_p_ftype *convert_register_p);
 
-typedef int (gdbarch_register_to_value_ftype) (struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep);
-extern int gdbarch_register_to_value (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep);
+typedef struct value * (gdbarch_register_to_value_ftype) (struct frame_info *frame, int regnum, struct type *type);
+extern struct value * gdbarch_register_to_value (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type);
 extern void set_gdbarch_register_to_value (struct gdbarch *gdbarch, gdbarch_register_to_value_ftype *register_to_value);
 
 typedef void (gdbarch_value_to_register_ftype) (struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index db4d882..cfa55b0 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2339,14 +2339,14 @@ set_gdbarch_convert_register_p (struct gdbarch *gdbarch,
   gdbarch->convert_register_p = convert_register_p;
 }
 
-int
-gdbarch_register_to_value (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep)
+struct value *
+gdbarch_register_to_value (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->register_to_value != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_register_to_value called\n");
-  return gdbarch->register_to_value (frame, regnum, type, buf, optimizedp, unavailablep);
+  return gdbarch->register_to_value (frame, regnum, type);
 }
 
 void
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 54e7b4a..5b8e679 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -663,8 +663,6 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
 
   if (gdbarch_convert_register_p (gdbarch, regnum, type1))
     {
-      int optim, unavail, ok;
-
       /* The ISA/ABI need to something weird when obtaining the
          specified value from this register.  It might need to
          re-order non-adjacent, starting with REGNUM (see MIPS and
@@ -672,21 +670,7 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
          the corresponding [integer] type (see Alpha).  The assumption
          is that gdbarch_register_to_value populates the entire value
          including the location.  */
-      v = allocate_value (type);
-      VALUE_LVAL (v) = lval_register;
-      VALUE_FRAME_ID (v) = get_frame_id (frame);
-      VALUE_REGNUM (v) = regnum;
-      ok = gdbarch_register_to_value (gdbarch, frame, regnum, type1,
-				      value_contents_raw (v), &optim,
-				      &unavail);
-
-      if (!ok)
-	{
-	  if (optim)
-	    set_value_optimized_out (v, 1);
-	  if (unavail)
-	    mark_value_bytes_unavailable (v, 0, TYPE_LENGTH (type));
-	}
+      v = gdbarch_register_to_value (gdbarch, frame, regnum, type1);
     }
   else
     {
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 2e4bb6f..3913ca7 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -211,7 +211,10 @@ alpha_lds (struct gdbarch *gdbarch, void *out, const void *in)
 }
 
 /* Similarly, this represents exactly the conversion performed by
-   the STS instruction.  */
+   the STS instruction.
+
+   Note: IN and OUT can be equal, in which case the buffer contents
+   will simply be replaced by the value after conversion.  */
 
 static void
 alpha_sts (struct gdbarch *gdbarch, void *out, const void *in)
@@ -238,25 +241,23 @@ alpha_convert_register_p (struct gdbarch *gdbarch, int regno,
 	  && TYPE_LENGTH (type) != 8);
 }
 
-static int
+static struct value *
 alpha_register_to_value (struct frame_info *frame, int regnum,
-			 struct type *valtype, gdb_byte *out,
-			int *optimizedp, int *unavailablep)
+			 struct type *valtype)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  gdb_byte in[MAX_REGISTER_SIZE];
+  struct value *value;
 
-  /* Convert to TYPE.  */
-  if (!get_frame_register_bytes (frame, regnum, 0,
-				 register_size (gdbarch, regnum),
-				 in, optimizedp, unavailablep))
-    return 0;
+  value = gdbarch_value_from_register (gdbarch, valtype, regnum, frame);
+  read_frame_register_value (value, frame);
+  if (value_optimized_out (value) || !value_entirely_available (value))
+    return value;
 
   if (TYPE_LENGTH (valtype) == 4)
     {
-      alpha_sts (gdbarch, out, in);
-      *optimizedp = *unavailablep = 0;
-      return 1;
+      gdb_byte *contents = value_contents_writeable (value);
+      alpha_sts (gdbarch, contents, contents);
+      return value;
     }
 
   error (_("Cannot retrieve value from floating point register"));
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a381a51..eedcb29 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3106,39 +3106,20 @@ i386_convert_register_p (struct gdbarch *gdbarch,
 /* Read a value of type TYPE from register REGNUM in frame FRAME, and
    return its contents in TO.  */
 
-static int
+static struct value *
 i386_register_to_value (struct frame_info *frame, int regnum,
-			struct type *type, gdb_byte *to,
-			int *optimizedp, int *unavailablep)
+			struct type *type)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  int len = TYPE_LENGTH (type);
+  struct value *value;
 
   if (i386_fp_regnum_p (gdbarch, regnum))
-    return i387_register_to_value (frame, regnum, type, to,
-				   optimizedp, unavailablep);
-
-  /* Read a value spread across multiple registers.  */
-
-  gdb_assert (len > 4 && len % 4 == 0);
-
-  while (len > 0)
-    {
-      gdb_assert (regnum != -1);
-      gdb_assert (register_size (gdbarch, regnum) == 4);
-
-      if (!get_frame_register_bytes (frame, regnum, 0,
-				     register_size (gdbarch, regnum),
-				     to, optimizedp, unavailablep))
-	return 0;
+    return i387_register_to_value (frame, regnum, type);
 
-      regnum = i386_next_regnum (regnum);
-      len -= 4;
-      to += 4;
-    }
-
-  *optimizedp = *unavailablep = 0;
-  return 1;
+  value = gdbarch_value_from_register (gdbarch, type, regnum, frame);
+  read_frame_register_value (value, frame);
+  
+  return value;
 }
 
 /* Write the contents FROM of a value of type TYPE into register
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index c4ace82..e505421 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -307,12 +307,12 @@ i387_convert_register_p (struct gdbarch *gdbarch, int regnum,
 /* Read a value of type TYPE from register REGNUM in frame FRAME, and
    return its contents in TO.  */
 
-int
+struct value *
 i387_register_to_value (struct frame_info *frame, int regnum,
-			struct type *type, gdb_byte *to,
-			int *optimizedp, int *unavailablep)
+			struct type *type)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
+  struct value *value;
   gdb_byte from[I386_MAX_REGISTER_SIZE];
 
   gdb_assert (i386_fp_regnum_p (gdbarch, regnum));
@@ -321,19 +321,24 @@ i387_register_to_value (struct frame_info *frame, int regnum,
   if (TYPE_CODE (type) != TYPE_CODE_FLT)
     {
       warning (_("Cannot convert floating-point register value "
-	       "to non-floating-point type."));
-      *optimizedp = *unavailablep = 0;
-      return 0;
+               "to non-floating-point type."));
+      value = allocate_value (type);
+      mark_value_bytes_unavailable (value, 0,
+                                    TYPE_LENGTH (check_typedef (type)));
+				    
+      return value;
     }
 
   /* Convert to TYPE.  */
-  if (!get_frame_register_bytes (frame, regnum, 0, TYPE_LENGTH (type),
-				 from, optimizedp, unavailablep))
-    return 0;
-
-  convert_typed_floating (from, i387_ext_type (gdbarch), to, type);
-  *optimizedp = *unavailablep = 0;
-  return 1;
+  value = gdbarch_value_from_register (gdbarch, type, regnum, frame);
+  read_frame_register_value (value, frame);
+  if (value_optimized_out (value) || !value_entirely_available (value))
+    return value;
+
+  memcpy (from, value_contents (value), TYPE_LENGTH (check_typedef (type)));
+  convert_typed_floating (from, i387_ext_type (gdbarch),
+                          value_contents_writeable (value), type);
+  return value;
 }
 
 /* Write the contents FROM of a value of type TYPE into register
diff --git a/gdb/i387-tdep.h b/gdb/i387-tdep.h
index b876549..d317a25 100644
--- a/gdb/i387-tdep.h
+++ b/gdb/i387-tdep.h
@@ -26,6 +26,7 @@ struct frame_info;
 struct regcache;
 struct type;
 struct ui_file;
+struct value;
 
 /* Number of i387 floating point registers.  */
 #define I387_NUM_REGS	16
@@ -63,12 +64,10 @@ extern void i387_print_float_info (struct gdbarch *gdbarch,
 extern int i387_convert_register_p (struct gdbarch *gdbarch, int regnum,
 				    struct type *type);
 
-/* Read a value of type TYPE from register REGNUM in frame FRAME, and
-   return its contents in TO.  */
+/* Return a value of type TYPE from register REGNUM in frame FRAME.  */
 
-extern int i387_register_to_value (struct frame_info *frame, int regnum,
-				   struct type *type, gdb_byte *to,
-				   int *optimizedp, int *unavailablep);
+extern struct value *i387_register_to_value (struct frame_info *frame,
+					     int regnum, struct type *type);
 
 /* Write the contents FROM of a value of type TYPE into register
    REGNUM in frame FRAME.  */