Use address_from_register in dwarf2-frame.c:read_addr_from_reg

This patch fixes a problem that prevented use of the Dwarf unwinders on SPU,
because dwarf2-frame.c common code did not support the situation where the
stack and/or frame pointer is maintained in a *vector* register.  This is
because read_addr_from_reg is hard-coded to assume that such pointers can
be read from registers via a simple get_frame_register / unpack_pointer
operation.

Now, there *is* a routine address_from_register that calls into the
appropriate tdep routines to handle pointer values in "weird" registers
like on SPU, but it turns out I cannot simply change dwarf2-frame.c to
use address_from_register.  This is because address_from_register uses
value_from_register to create a (temporary) value, and that routine
at some point calls get_frame_id in order to set up that value's
VALUE_FRAME_ID entry.

However, the dwarf2-frame.c read_addr_from_reg routine will be called
during early unwinding (to unwind the frame's CFA), at which point the
frame's ID is not actually known yet!  This would cause an assert.

On the other hand, we may notice that VALUE_FRAME_ID is only needed in the
value returned by value_from_register if that value is later used as an
lvalue.  But this is obviously never done to the temporary value used in
address_from_register.  So, if we could change address_from_register to
not call value_from_register but instead accept constructing a value
that doesn't have VALUE_FRAME_ID set, things should be fine.

To do that, we can change the value_from_register callback to accept
a FRAME_ID instead of a FRAME; the only existing uses of the FRAME
argument were either to extract its frame ID, or its gdbarch.  (To
keep a way of getting at the latter, we also change the callback's
type from "f" to "m".)  Together with the required follow-on changes
in the existing value_from_register implementations (including the
default one), this seems to fix the problem.

As another minor interface cleanup, I've removed the explicit TYPE
argument from address_from_register.  This routine really always
uses a default pointer type, and in the new implementation it -to
some extent- relies on that fact, in that it will now no longer
handle types that require gdbarch_convert_register_p handling.

gdb:
2014-04-17  Ulrich Weigand  <uweigand@de.ibm.com>

	* gdbarch.sh (value_from_register): Make class "m" instead of "f".
	Replace FRAME argument with FRAME_ID.
	* gdbarch.c, gdbarch.h: Regenerate.
	* findvar.c (default_value_from_register): Add GDBARCH argument;
	replace FRAME by FRAME_ID.  No longer call get_frame_id.
	(value_from_register): Update call to gdbarch_value_from_register.
	* value.h (default_value_from_register): Update prototype.
	* s390-linux-tdep.c (s390_value_from_register): Update interface
	and call to default_value_from_register.
	* spu-tdep.c (spu_value_from_register): Likewise.

	* findvar.c (address_from_register): Remove TYPE argument.
	Do not call value_from_register; use gdbarch_value_from_register
	with null_frame_id instead.
	* value.h (address_from_register): Update prototype.
	* dwarf2-frame.c (read_addr_from_reg): Use address_from_register.
	* dwarf2loc.c (dwarf_expr_read_addr_from_reg): Update for
	address_from_register interface change.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8cdd1423a4b77d99cca468c52ab909bacdcec0b..f0a77be1b4e45db8430679e40cf64be2888c56c2 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,24 @@
+2014-04-17  Ulrich Weigand  <uweigand@de.ibm.com>
+
+       * gdbarch.sh (value_from_register): Make class "m" instead of "f".
+       Replace FRAME argument with FRAME_ID.
+       * gdbarch.c, gdbarch.h: Regenerate.
+       * findvar.c (default_value_from_register): Add GDBARCH argument;
+       replace FRAME by FRAME_ID.  No longer call get_frame_id.
+       (value_from_register): Update call to gdbarch_value_from_register.
+       * value.h (default_value_from_register): Update prototype.
+       * s390-linux-tdep.c (s390_value_from_register): Update interface
+       and call to default_value_from_register.
+       * spu-tdep.c (spu_value_from_register): Likewise.
+
+       * findvar.c (address_from_register): Remove TYPE argument.
+       Do not call value_from_register; use gdbarch_value_from_register
+       with null_frame_id instead.
+       * value.h (address_from_register): Update prototype.
+       * dwarf2-frame.c (read_addr_from_reg): Use address_from_register.
+       * dwarf2loc.c (dwarf_expr_read_addr_from_reg): Update for
+       address_from_register interface change.
+
 2014-04-17  Yao Qi  <yao@codesourcery.com>
 
        * gdbtypes.h: Update comments to link to types and macros'

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index ce21112ba7620ee6a025d00b1dad7cadc0856a05..3e3ba23d262e4b76cb21cd5525eaef531c5dddc0 100644 (file)
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -291,15 +291,9 @@ read_addr_from_reg (void *baton, int reg)
 {
   struct frame_info *this_frame = (struct frame_info *) baton;
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  int regnum;
-  gdb_byte *buf;
-
-  regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, reg);
-
-  buf = alloca (register_size (gdbarch, regnum));
-  get_frame_register (this_frame, regnum, buf);
+  int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, reg);
 
-  return unpack_pointer (register_type (gdbarch, regnum), buf);
+  return address_from_register (regnum, this_frame);
 }
 
 /* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback.  */

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index addae1382b40cfc2f6a0d74f533a3f698e2f7757..fa17ea63f4cedbfc25433a5925ea38d14fccafb0 100644 (file)
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -317,13 +317,9 @@ dwarf_expr_read_addr_from_reg (void *baton, int dwarf_regnum)
 {
   struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton;
   struct gdbarch *gdbarch = get_frame_arch (debaton->frame);
-  CORE_ADDR result;
-  int regnum;
+  int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, dwarf_regnum);
 
-  regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, dwarf_regnum);
-  result = address_from_register (builtin_type (gdbarch)->builtin_data_ptr,
-                                 regnum, debaton->frame);
-  return result;
+  return address_from_register (regnum, debaton->frame);
 }
 
 /* Implement struct dwarf_expr_context_funcs' "get_reg_value" callback.  */

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 998a799e3bed6147ee61611a76735bfd22687dc7..9390c8a7b88d15952db73948c6724dbfeb6413cb 100644 (file)
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -625,15 +625,14 @@ read_var_value (struct symbol *var, struct frame_info *frame)
 /* Install default attributes for register values.  */
 
 struct value *
-default_value_from_register (struct type *type, int regnum,
-                            struct frame_info *frame)
+default_value_from_register (struct gdbarch *gdbarch, struct type *type,
+                             int regnum, struct frame_id frame_id)
 {
-  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_FRAME_ID (value) = frame_id;
   VALUE_REGNUM (value) = regnum;
 
   /* Any structure stored in more than one register will always be
@@ -741,7 +740,8 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   else
     {
       /* Construct the value.  */
-      v = gdbarch_value_from_register (gdbarch, type, regnum, frame);
+      v = gdbarch_value_from_register (gdbarch, type,
+                                      regnum, get_frame_id (frame));
 
       /* Get the data.  */
       read_frame_register_value (v, frame);
@@ -750,18 +750,30 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   return v;
 }
 
-/* Return contents of register REGNUM in frame FRAME as address,
-   interpreted as value of type TYPE.   Will abort if register
-   value is not available.  */
+/* Return contents of register REGNUM in frame FRAME as address.
+   Will abort if register value is not available.  */
 
 CORE_ADDR
-address_from_register (struct type *type, int regnum, struct frame_info *frame)
+address_from_register (int regnum, struct frame_info *frame)
 {
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  struct type *type = builtin_type (gdbarch)->builtin_data_ptr;
   struct value *value;
   CORE_ADDR result;
 
-  value = value_from_register (type, regnum, frame);
-  gdb_assert (value);
+  /* This routine may be called during early unwinding, at a time
+     where the ID of FRAME is not yet known.  Calling value_from_register
+     would therefore abort in get_frame_id.  However, since we only need
+     a temporary value that is never used as lvalue, we actually do not
+     really need to set its VALUE_FRAME_ID.  Therefore, we re-implement
+     the core of value_from_register, but use the null_frame_id.
+
+     This works only if we do not require a special conversion routine,
+     which is true for plain pointer types for all current targets.  */
+  gdb_assert (!gdbarch_convert_register_p (gdbarch, regnum, type));
+
+  value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id);
+  read_frame_register_value (value, frame);
 
   if (value_optimized_out (value))
     {

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 9ec586597a059a687e50eb983528658780062e49..4f8209510ab43e45dd3a49c59464e7c3d428e2da 100644 (file)
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -2381,13 +2381,13 @@ set_gdbarch_value_to_register (struct gdbarch *gdbarch,
 }
 
 struct value *
-gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_info *frame)
+gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id)
 {
   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);
+  return gdbarch->value_from_register (gdbarch, type, regnum, frame_id);
 }
 
 void

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 9fb27d4412502c9bb66ff934355a324151cf6628..c621091aa4a283998fb54fc2d9bff79e216f1673 100644 (file)
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -422,12 +422,12 @@ extern void gdbarch_value_to_register (struct gdbarch *gdbarch, struct frame_inf
 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, interpreted as type TYPE.  The routine needs to
+   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 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);
+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);
 
 typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);

diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 6a47f85331c67f840fbee6fe5610c5564397ed70..9f357b67191a6a447edb1dd80421ebfe56adbd8e 100755 (executable)
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -500,10 +500,10 @@ m:int:convert_register_p:int regnum, struct type *type:regnum, type:0:generic_co
 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, interpreted as type TYPE.  The routine needs to
+# 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.
-f:struct value *:value_from_register:struct type *type, int regnum, struct frame_info *frame:type, regnum, frame::default_value_from_register::0
+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
 #
 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 3084b326ebb4457136d9b27065d0a2022c265ca5..9ab5c703f61a04dbc79e2614dc1c53157f1991a9 100644 (file)
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -380,11 +380,11 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
    registers, even though we are otherwise a big-endian platform.  */
 
 static struct value *
-s390_value_from_register (struct type *type, int regnum,
-                         struct frame_info *frame)
+s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
+                         int regnum, struct frame_id frame_id)
 {
-  struct value *value = default_value_from_register (type, regnum, frame);
-
+  struct value *value = default_value_from_register (gdbarch, type,
+                                                    regnum, frame_id);
   check_typedef (type);
 
   if (regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM

diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 4967bcd1d7231edf9993f2234c0fe3d42aca0df8..0f23b0f1100631b2de2b1505e661525c1ff27fd1 100644 (file)
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -314,10 +314,11 @@ spu_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
 /* Value conversion -- access scalar values at the preferred slot.  */
 
 static struct value *
-spu_value_from_register (struct type *type, int regnum,
-                        struct frame_info *frame)
+spu_value_from_register (struct gdbarch *gdbarch, struct type *type,
+                        int regnum, struct frame_id frame_id)
 {
-  struct value *value = default_value_from_register (type, regnum, frame);
+  struct value *value = default_value_from_register (gdbarch, type,
+                                                    regnum, frame_id);
   int len = TYPE_LENGTH (type);
 
   if (regnum < SPU_NUM_GPRS && len < 16)

diff --git a/gdb/value.h b/gdb/value.h
index 9425a0ef11f68da2b6711b43a7304cb7c0118e32..144e18264a2d288daf0259baf48cd88f462260c3 100644 (file)
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -579,9 +579,10 @@ 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 type *type,
+extern struct value *default_value_from_register (struct gdbarch *gdbarch,
+                                                 struct type *type,
                                                  int regnum,
-                                                 struct frame_info *frame);
+                                                 struct frame_id frame_id);
 
 extern void read_frame_register_value (struct value *value,
                                       struct frame_info *frame);
@@ -589,7 +590,7 @@ extern void read_frame_register_value (struct value *value,
 extern struct value *value_from_register (struct type *type, int regnum,
                                          struct frame_info *frame);
 
-extern CORE_ADDR address_from_register (struct type *type, int regnum,
+extern CORE_ADDR address_from_register (int regnum,
                                        struct frame_info *frame);
 
 extern struct value *value_of_variable (struct symbol *var,