This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][2/2] Rework value_from_register


Jim Blandy wrote:

> If we partially initialize the value before passing it to
> gdbarch_value_from_register, then all the per-arch functions
> implementing that method are potentially dependent on the
> initialization; the initialization is effectively part of the
> interface of the gdbarch method.  I'd almost prefer passing all the
> arguments to value_from_register directly through to the gdbarch
> method, and making it allocate the value; value_from_register's
> interface has been unchanged as far back as our CVS history goes.

OK, I've switched the interface accordingly.  The implementation
of the spu and s390 versions actually got even a bit shorter ...

> Could we include a comment for value_from_register in gdbarch.sh?  I
> see that the functions around it are not commented, but I (at least)
> find it very helpful.  Ideally, the comment should have enough detail
> for you to understand how to use the method, without having to look
> much at its implementations or uses.

I've added a corresponding comment.

> Other than that, I think this looks great.  Thank you for doing all
> the revisions you've already done --- I'm sorry for not reviewing more
> promptly.  Tested with no regressions on IA-32 Fedora Core 6.  If my
> suggestions seem reasonable, please revise and commit.

Please find below the version I've committed -- I hope this is what
you were thinking about.  Retested on s390-ibm-linux, s390x-ibm-linux,
and spu-elf.

Thanks,
Ulrich

ChangeLog:

	* gdbarch.sh (value_from_register): New gdbarch function.
	* gdbarch.c, gdbarch.h: Regenerate.
	* findvar.c (default_value_from_register): New function.
	(value_from_register): Use gdbarch_value_from_register.
	* value.h (default_value_from_register): Declare.
	* spu-tdep.c (spu_convert_register_p, spu_register_to_value,
	spu_value_to_register): Remove.
	(spu_value_from_register): New function.
	(spu_gdbarch_init): Do not call set_gdbarch_convert_register_p,
	set_gdbarch_register_to_value, set_gdbarch_value_to_register.
	Call set_gdbarch_value_from_register.
	* s390-tdep.c (s390_convert_register_p, s390_register_to_value,
	s390_value_to_register): Remove.
	(s390_value_from_register): New function.
	(s390_gdbarch_init): Do not call set_gdbarch_convert_register_p,
	set_gdbarch_register_to_value, set_gdbarch_value_to_register.
	Call set_gdbarch_value_from_register.

diff -urp gdb-orig/gdb/findvar.c gdb-head/gdb/findvar.c
--- gdb-orig/gdb/findvar.c	2007-01-08 19:27:28.316132000 +0100
+++ gdb-head/gdb/findvar.c	2007-01-08 19:39:32.129082880 +0100
@@ -599,20 +599,44 @@ addresses have not been bound by the dyn
   return v;
 }
 
+/* Install default attributes for register values.  */
+
+struct value *
+default_value_from_register (struct type *type, int regnum,
+			     struct frame_info *frame)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  int len = TYPE_LENGTH (type);
+  struct value *value = allocate_value (type);
+
+  VALUE_LVAL (value) = lval_register;
+  VALUE_FRAME_ID (value) = get_frame_id (frame);
+  VALUE_REGNUM (value) = regnum;
+
+  /* Any structure stored in more than one register will always be
+     an integral number of registers.  Otherwise, you need to do
+     some fiddling with the last register copied here for little
+     endian machines.  */
+  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
+      && len < register_size (gdbarch, regnum))
+    /* Big-endian, and we want less than full size.  */
+    set_value_offset (value, register_size (gdbarch, regnum) - len);
+  else
+    set_value_offset (value, 0);
+
+  return value;
+}
+
 /* Return a value of type TYPE, stored in register REGNUM, in frame FRAME.  */
 
 struct value *
 value_from_register (struct type *type, int regnum, struct frame_info *frame)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct value *v = allocate_value (type);
-  CHECK_TYPEDEF (type);
+  struct type *type1 = check_typedef (type);
+  struct value *v;
 
-  VALUE_LVAL (v) = lval_register;
-  VALUE_FRAME_ID (v) = get_frame_id (frame);
-  VALUE_REGNUM (v) = regnum;
-      
-  if (CONVERT_REGISTER_P (regnum, type))
+  if (CONVERT_REGISTER_P (regnum, type1))
     {
       /* The ISA/ABI need to something weird when obtaining the
          specified value from this register.  It might need to
@@ -621,22 +645,18 @@ value_from_register (struct type *type, 
          the corresponding [integer] type (see Alpha).  The assumption
          is that REGISTER_TO_VALUE populates the entire value
          including the location.  */
-      REGISTER_TO_VALUE (frame, regnum, type, value_contents_raw (v));
+      v = allocate_value (type);
+      VALUE_LVAL (v) = lval_register;
+      VALUE_FRAME_ID (v) = get_frame_id (frame);
+      VALUE_REGNUM (v) = regnum;
+      REGISTER_TO_VALUE (frame, regnum, type1, value_contents_raw (v));
     }
   else
     {
       int len = TYPE_LENGTH (type);
 
-      /* Any structure stored in more than one register will always be
-         an integral number of registers.  Otherwise, you need to do
-         some fiddling with the last register copied here for little
-         endian machines.  */
-      if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
-	  && len < register_size (gdbarch, regnum))
-	/* Big-endian, and we want less than full size.  */
-	set_value_offset (v, register_size (gdbarch, regnum) - len);
-      else
-	set_value_offset (v, 0);
+      /* Construct the value.  */
+      v = gdbarch_value_from_register (gdbarch, type, regnum, frame);
 
       /* Get the data.  */
       if (!get_frame_register_bytes (frame, regnum, value_offset (v), len,
diff -urp gdb-orig/gdb/gdbarch.c gdb-head/gdb/gdbarch.c
--- gdb-orig/gdb/gdbarch.c	2007-01-08 19:27:28.328131000 +0100
+++ gdb-head/gdb/gdbarch.c	2007-01-08 19:28:58.240043816 +0100
@@ -183,6 +183,7 @@ struct gdbarch
   gdbarch_convert_register_p_ftype *convert_register_p;
   gdbarch_register_to_value_ftype *register_to_value;
   gdbarch_value_to_register_ftype *value_to_register;
+  gdbarch_value_from_register_ftype *value_from_register;
   gdbarch_pointer_to_address_ftype *pointer_to_address;
   gdbarch_address_to_pointer_ftype *address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address;
@@ -311,6 +312,7 @@ struct gdbarch startup_gdbarch =
   0,  /* convert_register_p */
   0,  /* register_to_value */
   0,  /* value_to_register */
+  0,  /* value_from_register */
   0,  /* pointer_to_address */
   0,  /* address_to_pointer */
   0,  /* integer_to_address */
@@ -433,6 +435,7 @@ gdbarch_alloc (const struct gdbarch_info
   current_gdbarch->cannot_fetch_register = cannot_register_not;
   current_gdbarch->cannot_store_register = cannot_register_not;
   current_gdbarch->convert_register_p = generic_convert_register_p;
+  current_gdbarch->value_from_register = default_value_from_register;
   current_gdbarch->pointer_to_address = unsigned_pointer_to_address;
   current_gdbarch->address_to_pointer = unsigned_address_to_pointer;
   current_gdbarch->return_value = legacy_return_value;
@@ -566,6 +569,7 @@ verify_gdbarch (struct gdbarch *current_
   /* Skip verify of cannot_store_register, invalid_p == 0 */
   /* Skip verify of get_longjmp_target, has predicate */
   /* Skip verify of convert_register_p, invalid_p == 0 */
+  /* Skip verify of value_from_register, invalid_p == 0 */
   /* Skip verify of pointer_to_address, invalid_p == 0 */
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate */
@@ -1592,6 +1596,9 @@ gdbarch_dump (struct gdbarch *current_gd
   fprintf_unfiltered (file,
                       "gdbarch_dump: unwind_sp = <0x%lx>\n",
                       (long) current_gdbarch->unwind_sp);
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: value_from_register = <0x%lx>\n",
+                      (long) current_gdbarch->value_from_register);
 #ifdef VALUE_TO_REGISTER
   fprintf_unfiltered (file,
                       "gdbarch_dump: %s # %s\n",
@@ -2648,6 +2655,23 @@ set_gdbarch_value_to_register (struct gd
   gdbarch->value_to_register = value_to_register;
 }
 
+struct value *
+gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_info *frame)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->value_from_register != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_value_from_register called\n");
+  return gdbarch->value_from_register (type, regnum, frame);
+}
+
+void
+set_gdbarch_value_from_register (struct gdbarch *gdbarch,
+                                 gdbarch_value_from_register_ftype value_from_register)
+{
+  gdbarch->value_from_register = value_from_register;
+}
+
 CORE_ADDR
 gdbarch_pointer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf)
 {
diff -urp gdb-orig/gdb/gdbarch.h gdb-head/gdb/gdbarch.h
--- gdb-orig/gdb/gdbarch.h	2007-01-08 19:27:28.373124000 +0100
+++ gdb-head/gdb/gdbarch.h	2007-01-08 19:28:58.284037128 +0100
@@ -704,6 +704,15 @@ extern void set_gdbarch_value_to_registe
 #define VALUE_TO_REGISTER(frame, regnum, type, buf) (gdbarch_value_to_register (current_gdbarch, frame, regnum, type, buf))
 #endif
 
+/* Construct a value representing the contents of register REGNUM in
+   frame FRAME, interpreted as type TYPE.  The routine needs to
+   allocate and return a struct value with all value attributes
+   (but not the value contents) filled in. */
+
+typedef struct value * (gdbarch_value_from_register_ftype) (struct type *type, int regnum, struct frame_info *frame);
+extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_info *frame);
+extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register);
+
 typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct type *type, const gdb_byte *buf);
 extern CORE_ADDR gdbarch_pointer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
 extern void set_gdbarch_pointer_to_address (struct gdbarch *gdbarch, gdbarch_pointer_to_address_ftype *pointer_to_address);
diff -urp gdb-orig/gdb/gdbarch.sh gdb-head/gdb/gdbarch.sh
--- gdb-orig/gdb/gdbarch.sh	2007-01-08 19:27:28.416117000 +0100
+++ gdb-head/gdb/gdbarch.sh	2007-01-08 19:28:58.292035912 +0100
@@ -496,6 +496,11 @@ v:=:int:believe_pcc_promotion:::::::
 f:=:int:convert_register_p:int regnum, struct type *type:regnum, type:0:generic_convert_register_p::0
 f:=:void:register_to_value:struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf:frame, regnum, type, buf: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
+# allocate and return a struct value with all value attributes
+# (but not the value contents) filled in.
+f::struct value *:value_from_register:struct type *type, int regnum, struct frame_info *frame:type, regnum, frame::default_value_from_register::0
 #
 f:=:CORE_ADDR:pointer_to_address:struct type *type, const gdb_byte *buf:type, buf::unsigned_pointer_to_address::0
 f:=:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
diff -urp gdb-orig/gdb/s390-tdep.c gdb-head/gdb/s390-tdep.c
--- gdb-orig/gdb/s390-tdep.c	2007-01-08 19:27:28.461110000 +0100
+++ gdb-head/gdb/s390-tdep.c	2007-01-08 19:37:50.212129304 +0100
@@ -306,36 +306,17 @@ s390x_pseudo_register_write (struct gdba
 /* 'float' values are stored in the upper half of floating-point
    registers, even though we are otherwise a big-endian platform.  */
 
-static int
-s390_convert_register_p (int regno, struct type *type)
-{
-  return (regno >= S390_F0_REGNUM && regno <= S390_F15_REGNUM)
-	 && TYPE_LENGTH (type) < 8;
-}
-
-static void
-s390_register_to_value (struct frame_info *frame, int regnum,
-                        struct type *valtype, gdb_byte *out)
+static struct value *
+s390_value_from_register (struct type *type, int regnum,
+			  struct frame_info *frame)
 {
-  gdb_byte in[8];
-  int len = TYPE_LENGTH (valtype);
-  gdb_assert (len < 8);
+  struct value *value = default_value_from_register (type, regnum, frame);
+  int len = TYPE_LENGTH (type);
 
-  get_frame_register (frame, regnum, in);
-  memcpy (out, in, len);
-}
+  if (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM && len < 8)
+    set_value_offset (value, 0);
 
-static void
-s390_value_to_register (struct frame_info *frame, int regnum,
-                        struct type *valtype, const gdb_byte *in)
-{
-  gdb_byte out[8];
-  int len = TYPE_LENGTH (valtype);
-  gdb_assert (len < 8);
-
-  memset (out, 0, 8);
-  memcpy (out, in, len);
-  put_frame_register (frame, regnum, out);
+  return value;
 }
 
 /* Register groups.  */
@@ -2411,9 +2392,7 @@ s390_gdbarch_init (struct gdbarch_info i
   set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
-  set_gdbarch_convert_register_p (gdbarch, s390_convert_register_p);
-  set_gdbarch_register_to_value (gdbarch, s390_register_to_value);
-  set_gdbarch_value_to_register (gdbarch, s390_value_to_register);
+  set_gdbarch_value_from_register (gdbarch, s390_value_from_register);
   set_gdbarch_register_reggroup_p (gdbarch, s390_register_reggroup_p);
   set_gdbarch_regset_from_core_section (gdbarch,
                                         s390_regset_from_core_section);
diff -urp gdb-orig/gdb/spu-tdep.c gdb-head/gdb/spu-tdep.c
--- gdb-orig/gdb/spu-tdep.c	2007-01-08 19:27:28.502104000 +0100
+++ gdb-head/gdb/spu-tdep.c	2007-01-08 19:38:04.069020712 +0100
@@ -145,37 +145,20 @@ spu_pseudo_register_write (struct gdbarc
 
 /* Value conversion -- access scalar values at the preferred slot.  */
 
-static int
-spu_convert_register_p (int regno, struct type *type)
-{
-  return regno < SPU_NUM_GPRS && TYPE_LENGTH (type) < 16;
-}
-
-static void
-spu_register_to_value (struct frame_info *frame, int regnum,
-                       struct type *valtype, gdb_byte *out)
+static struct value *
+spu_value_from_register (struct type *type, int regnum,
+			 struct frame_info *frame)
 {
-  gdb_byte in[16];
-  int len = TYPE_LENGTH (valtype);
-  int preferred_slot = len < 4 ? 4 - len : 0;
-  gdb_assert (len < 16);
+  struct value *value = default_value_from_register (type, regnum, frame);
+  int len = TYPE_LENGTH (type);
 
-  get_frame_register (frame, regnum, in);
-  memcpy (out, in + preferred_slot, len);
-}
+  if (regnum < SPU_NUM_GPRS && len < 16)
+    {
+      int preferred_slot = len < 4 ? 4 - len : 0;
+      set_value_offset (value, preferred_slot);
+    }
 
-static void
-spu_value_to_register (struct frame_info *frame, int regnum,
-                       struct type *valtype, const gdb_byte *in)
-{
-  gdb_byte out[16];
-  int len = TYPE_LENGTH (valtype);
-  int preferred_slot = len < 4 ? 4 - len : 0;
-  gdb_assert (len < 16);
-
-  memset (out, 0, 16);
-  memcpy (out + preferred_slot, in, len);
-  put_frame_register (frame, regnum, out);
+  return value;
 }
 
 /* Register groups.  */
@@ -1050,9 +1033,7 @@ spu_gdbarch_init (struct gdbarch_info in
   set_gdbarch_register_type (gdbarch, spu_register_type);
   set_gdbarch_pseudo_register_read (gdbarch, spu_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, spu_pseudo_register_write);
-  set_gdbarch_convert_register_p (gdbarch, spu_convert_register_p);
-  set_gdbarch_register_to_value (gdbarch, spu_register_to_value);
-  set_gdbarch_value_to_register (gdbarch, spu_value_to_register);
+  set_gdbarch_value_from_register (gdbarch, spu_value_from_register);
   set_gdbarch_register_reggroup_p (gdbarch, spu_register_reggroup_p);
 
   /* Data types.  */
diff -urp gdb-orig/gdb/value.h gdb-head/gdb/value.h
--- gdb-orig/gdb/value.h	2007-01-08 19:27:28.507103000 +0100
+++ gdb-head/gdb/value.h	2007-01-08 19:36:32.546086528 +0100
@@ -280,6 +280,10 @@ extern struct value *value_from_string (
 extern struct value *value_at (struct type *type, CORE_ADDR addr);
 extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);
 
+extern struct value *default_value_from_register (struct type *type,
+						  int regnum,
+						  struct frame_info *frame);
+
 extern struct value *value_from_register (struct type *type, int regnum,
 					  struct frame_info *frame);
 

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]