[PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

On s390, when a 'long double' variable is located in a pair of FP
registers, its DWARF location looks something like this:

     DW_OP_reg24 (f8); DW_OP_piece: 8; DW_OP_reg25 (f10); DW_OP_piece: 8

If the target has a vector facility, each of its FP registers is
embedded left-aligned in a vector register with the same identical DWARF
register number.  But GDB always takes a register piece from the "least
significant" end, which is exactly opposite from where the FP register
is embedded.  This mismatch causes a regression with store.exp.

This change fixes this issue by taking s390 vector register pieces from
the left end instead.  Since this requires a suitable gdbarch interface,
the gdbarch method 'value_from_register' is replaced by a new method
'register_part' with similar functionality, but a slightly different
interface.  This method is then invoked in read_pieced_value and
write_pieced_value.

gdb/ChangeLog:

	* gdbarch.sh (value_from_register): Remove.
	(register_part): New.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Likewise.
	* dwarf2loc.c (read_pieced_value): Get arch-specific placement of
	a register piece from the new gdbarch method register_part.
	(write_pieced_value): Likewise.
	* value.h (default_value_from_register): Remove.
	(default_register_part): Declare.
	* findvar.c (default_value_from_register): Remove.
	(default_register_part): New.
	(value_from_register): Call gdbarch_register_part instead of
	gdbarch_value_from_register.
	(address_from_register): Likewise.
	* s390-linux-tdep.c (s390_value_from_register): Remove.
	(s390_register_part): New.
	(s390_gdbarch_init): Set new gdbarch method register_part instead
	of value_from_register.
	* spu-tdep.c (spu_value_from_register): Remove.
	(spu_register_part): New.
	(spu_gdbarch_init): Set new gdbarch method register_part instead
	of value_from_register.
---
 gdb/dwarf2loc.c       | 31 ++++++++++++++-----------------
 gdb/findvar.c         | 40 +++++++++++++++++++---------------------
 gdb/gdbarch.c         | 28 ++++++++++++++--------------
 gdb/gdbarch.h         | 15 +++++++--------
 gdb/gdbarch.sh        | 10 +++++-----
 gdb/s390-linux-tdep.c | 31 +++++++++++++------------------
 gdb/spu-tdep.c        | 20 ++++++--------------
 gdb/value.h           |  6 ++----
 8 files changed, 80 insertions(+), 101 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index ba6ed42..270518c 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1715,6 +1715,7 @@ read_pieced_value (struct value *v)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size, this_size_bits;
       long dest_offset_bits, source_offset_bits, source_offset;
       const gdb_byte *intermediate_buffer;
@@ -1759,17 +1760,13 @@ read_pieced_value (struct value *v)
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
 	    int optim, unavail;
-	    int reg_offset = source_offset;
+	    int reg_offset = gdbarch_register_part (arch, piece_bytes,
+						    &gdb_regnum);
 
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size < register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-		/* We want the lower-order THIS_SIZE_BITS of the bytes
-		   we extract from the register.  */
-		source_offset_bits += 8 * this_size - this_size_bits;
-	      }
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
 					   this_size, buffer,
@@ -1890,6 +1887,7 @@ write_pieced_value (struct value *to, struct value *from)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size_bits, this_size;
       long dest_offset_bits, source_offset_bits, dest_offset, source_offset;
       int need_bitwise;
@@ -1941,14 +1939,13 @@ write_pieced_value (struct value *to, struct value *from)
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
-	    int reg_offset = dest_offset;
+	    int reg_offset = gdbarch_register_part (arch, piece_bytes,
+						    &gdb_regnum);
 
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size <= register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-	      }
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (need_bitwise)
 	      {
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a39d897..fca18a1 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -797,29 +797,19 @@ read_var_value (struct symbol *var, const struct block *var_block,
 
 /* Install default attributes for register values.  */
 
-struct value *
-default_value_from_register (struct gdbarch *gdbarch, struct type *type,
-                             int regnum, struct frame_id frame_id)
+int
+default_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
-  int len = TYPE_LENGTH (type);
-  struct value *value = allocate_value (type);
-
-  VALUE_LVAL (value) = lval_register;
-  VALUE_FRAME_ID (value) = frame_id;
-  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 (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG
-      && len < register_size (gdbarch, regnum))
+      && 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 register_size (gdbarch, *regnum) - len;
 
-  return value;
+  return 0;
 }
 
 /* VALUE must be an lval_register value.  If regnum is the value's
@@ -877,6 +867,10 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   struct type *type1 = check_typedef (type);
   struct value *v;
 
+  v = allocate_value (type);
+  VALUE_LVAL (v) = lval_register;
+  VALUE_FRAME_ID (v) = get_frame_id (frame);
+
   if (gdbarch_convert_register_p (gdbarch, regnum, type1))
     {
       int optim, unavail, ok;
@@ -888,9 +882,6 @@ 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,
@@ -907,8 +898,10 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   else
     {
       /* Construct the value.  */
-      v = gdbarch_value_from_register (gdbarch, type,
-				       regnum, get_frame_id (frame));
+      int offset = gdbarch_register_part (gdbarch, TYPE_LENGTH (type),
+					  &regnum);
+      VALUE_REGNUM (v) = regnum;
+      set_value_offset (v, offset);
 
       /* Get the data.  */
       read_frame_register_value (v, frame);
@@ -927,6 +920,7 @@ address_from_register (int regnum, struct frame_info *frame)
   struct type *type = builtin_type (gdbarch)->builtin_data_ptr;
   struct value *value;
   CORE_ADDR result;
+  int offset;
   int regnum_max_excl = (gdbarch_num_regs (gdbarch)
 			 + gdbarch_num_pseudo_regs (gdbarch));
 
@@ -962,7 +956,11 @@ address_from_register (int regnum, struct frame_info *frame)
       return unpack_long (type, buf);
     }
 
-  value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id);
+  offset = gdbarch_register_part (gdbarch, TYPE_LENGTH (type), &regnum);
+  value = allocate_value (type);
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+  set_value_offset (value, offset);
   read_frame_register_value (value, frame);
 
   if (value_optimized_out (value))
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..fc7a723 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -216,7 +216,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_register_part_ftype *register_part;
   gdbarch_pointer_to_address_ftype *pointer_to_address;
   gdbarch_address_to_pointer_ftype *address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address;
@@ -393,7 +393,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->cannot_fetch_register = cannot_register_not;
   gdbarch->cannot_store_register = cannot_register_not;
   gdbarch->convert_register_p = generic_convert_register_p;
-  gdbarch->value_from_register = default_value_from_register;
+  gdbarch->register_part = default_register_part;
   gdbarch->pointer_to_address = unsigned_pointer_to_address;
   gdbarch->address_to_pointer = unsigned_address_to_pointer;
   gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
@@ -560,7 +560,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* 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 register_part, 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.  */
@@ -1242,6 +1242,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: register_name = <%s>\n",
                       host_address_to_string (gdbarch->register_name));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: register_part = <%s>\n",
+                      host_address_to_string (gdbarch->register_part));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: register_reggroup_p = <%s>\n",
                       host_address_to_string (gdbarch->register_reggroup_p));
   fprintf_unfiltered (file,
@@ -1398,9 +1401,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: unwind_sp = <%s>\n",
                       host_address_to_string (gdbarch->unwind_sp));
   fprintf_unfiltered (file,
-                      "gdbarch_dump: value_from_register = <%s>\n",
-                      host_address_to_string (gdbarch->value_from_register));
-  fprintf_unfiltered (file,
                       "gdbarch_dump: value_to_register = <%s>\n",
                       host_address_to_string (gdbarch->value_to_register));
   fprintf_unfiltered (file,
@@ -2514,21 +2514,21 @@ set_gdbarch_value_to_register (struct gdbarch *gdbarch,
   gdbarch->value_to_register = value_to_register;
 }
 
-struct value *
-gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id)
+int
+gdbarch_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
   gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->value_from_register != NULL);
+  gdb_assert (gdbarch->register_part != NULL);
   if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_value_from_register called\n");
-  return gdbarch->value_from_register (gdbarch, type, regnum, frame_id);
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_register_part called\n");
+  return gdbarch->register_part (gdbarch, len, regnum);
 }
 
 void
-set_gdbarch_value_from_register (struct gdbarch *gdbarch,
-                                 gdbarch_value_from_register_ftype value_from_register)
+set_gdbarch_register_part (struct gdbarch *gdbarch,
+                           gdbarch_register_part_ftype register_part)
 {
-  gdbarch->value_from_register = value_from_register;
+  gdbarch->register_part = register_part;
 }
 
 CORE_ADDR
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 252fc4b..ac93c96 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -450,14 +450,13 @@ typedef void (gdbarch_value_to_register_ftype) (struct frame_info *frame, int re
 extern void gdbarch_value_to_register (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf);
 extern void set_gdbarch_value_to_register (struct gdbarch *gdbarch, gdbarch_value_to_register_ftype *value_to_register);
 
-/* Construct a value representing the contents of register REGNUM in
-   frame FRAME_ID, 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 gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register);
+/* Locate a part of size LEN within register *REGNUM, possibly overwriting
+   *REGNUM.  Return the offset of the part within the (possibly adjusted)
+   register. */
+
+typedef int (gdbarch_register_part_ftype) (struct gdbarch *gdbarch, int len, int *regnum);
+extern int gdbarch_register_part (struct gdbarch *gdbarch, int len, int *regnum);
+extern void set_gdbarch_register_part (struct gdbarch *gdbarch, gdbarch_register_part_ftype *register_part);
 
 typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
 extern CORE_ADDR gdbarch_pointer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 37f59b7..96bff43 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -506,11 +506,11 @@ 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: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_ID, 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.
-m:struct value *:value_from_register:struct type *type, int regnum, struct frame_id frame_id:type, regnum, frame_id::default_value_from_register::0
+
+# Locate a part of size LEN within register *REGNUM, possibly overwriting
+# *REGNUM.  Return the offset of the part within the (possibly adjusted)
+# register.
+m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
 #
 m:CORE_ADDR:pointer_to_address:struct type *type, const gdb_byte *buf:type, buf::unsigned_pointer_to_address::0
 m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 8b4efb1..bfc2a8e 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -513,28 +513,23 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
    registers, even though we are otherwise a big-endian platform.  The
    same applies to a 'float' value within a vector.  */
 
-static struct value *
-s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			  int regnum, struct frame_id frame_id)
+static int
+s390_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  unsigned int len = TYPE_LENGTH (check_typedef (type));
-  struct value *value;
 
   /* If the value fits in a sub-register, use that instead.  */
-  if (regnum_is_vxr_full (tdep, regnum) && len <= 8)
-    regnum = regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
-  else if (regnum_is_gpr_full (tdep, regnum) && len <= 4)
-    regnum = regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
-
-  value = default_value_from_register (gdbarch, type, regnum, frame_id);
-
-  if ((regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM && len < 8)
-      || regnum_is_vxr_full (tdep, regnum)
-      || (regnum >= S390_V16_REGNUM && regnum <= S390_V31_REGNUM))
-    set_value_offset (value, 0);
+  if (regnum_is_vxr_full (tdep, *regnum) && len <= 8)
+    *regnum = *regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
+  else if (regnum_is_gpr_full (tdep, *regnum) && len <= 4)
+    *regnum = *regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
+
+  if ((*regnum >= S390_F0_REGNUM && *regnum <= S390_F15_REGNUM && len < 8)
+      || regnum_is_vxr_full (tdep, *regnum)
+      || (*regnum >= S390_V16_REGNUM && *regnum <= S390_V31_REGNUM))
+    return 0;
 
-  return value;
+  return default_register_part (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -8005,7 +8000,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fp0_regnum (gdbarch, S390_F0_REGNUM);
   set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
-  set_gdbarch_value_from_register (gdbarch, s390_value_from_register);
+  set_gdbarch_register_part (gdbarch, s390_register_part);
   set_gdbarch_core_read_description (gdbarch, s390_core_read_description);
   set_gdbarch_iterate_over_regset_sections (gdbarch,
 					    s390_iterate_over_regset_sections);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 8dad5c3..ef0923a 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -357,21 +357,13 @@ spu_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
 
 /* Value conversion -- access scalar values at the preferred slot.  */
 
-static struct value *
-spu_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			 int regnum, struct frame_id frame_id)
+static int
+spu_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
-  struct value *value = default_value_from_register (gdbarch, type,
-						     regnum, frame_id);
-  int len = TYPE_LENGTH (type);
-
-  if (regnum < SPU_NUM_GPRS && len < 16)
-    {
-      int preferred_slot = len < 4 ? 4 - len : 0;
-      set_value_offset (value, preferred_slot);
-    }
+  if (*regnum < SPU_NUM_GPRS && len < 16)
+    return len < 4 ? 4 - len : 0;
 
-  return value;
+  return default_register_part (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -2738,7 +2730,7 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   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_value_from_register (gdbarch, spu_value_from_register);
+  set_gdbarch_register_part (gdbarch, spu_register_part);
   set_gdbarch_register_reggroup_p (gdbarch, spu_register_reggroup_p);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, spu_dwarf_reg_to_regnum);
   set_gdbarch_ax_pseudo_register_collect
diff --git a/gdb/value.h b/gdb/value.h
index f8ec854..79addae 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -647,10 +647,8 @@ extern struct value *value_from_contents_and_address (struct type *,
 						      CORE_ADDR);
 extern struct value *value_from_contents (struct type *, const gdb_byte *);
 
-extern struct value *default_value_from_register (struct gdbarch *gdbarch,
-						  struct type *type,
-						  int regnum,
-						  struct frame_id frame_id);
+extern int default_register_part (struct gdbarch *gdbarch,
+				  int len, int *regnum);
 
 extern void read_frame_register_value (struct value *value,
 				       struct frame_info *frame);
-- 
2.5.0