diff mbox series

[3/9] gdb: pass non-nullptr frame to gdbarch_value_from_register in address_from_register

Message ID 20231221191716.257256-4-simon.marchi@efficios.com
State New
Headers
Series Some register value cleanups |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Simon Marchi Dec. 21, 2023, 7:16 p.m. UTC 
  address_from_register used to pass null_frame_id to
gdbarch_value_from_register as "this frame"'s id, because it's possible
for it to be called during unwind, when "this frame"'s id is not yet
known.  This create an oddity where those register struct values are
created without a valid next frame id.  I would much prefer for things
to be consistent and have all register struct values to have a valid
next frame id.

Since gdbarch_value_from_register takes a frame_info_ptr now, rather
than a frame_id, we can pass down "this frame", even if it doesn't have
a valid id.  gdbarch_value_from_register implementations can obtain the
next frame from it.

However, it's possible for the "this frame"'s next frame to be an
inline frame, inlined in "this frame", in which case that next frame's
id is also not known.  So, loop until we get to the next non-inline
frame (which is actually the frame where registers for "this frame" are
unwound from).  This is the same thing that we do in
value_of_register_lazy, for the same reason.  A later patch will factor
out this "while next frame is inline" loop to apply it to all register
struct values, so this is somewhat temporary.

Change-Id: If487c82620cc5a4a4ea5807f0a0bad80ab984078
---
 gdb/findvar.c     | 19 +++++--------------
 gdb/rs6000-tdep.c | 10 ++++------
 2 files changed, 9 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 25737c4e3159..852f42dc176d 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -755,13 +755,11 @@  default_value_from_register (gdbarch *gdbarch, type *type, int regnum,
   struct value *value = value::allocate (type);
   value->set_lval (lval_register);
 
-  frame_id next_frame_id;
-  if (this_frame == nullptr)
-    next_frame_id = null_frame_id;
-  else
-    next_frame_id = get_frame_id (get_next_frame_sentinel_okay (this_frame));
+  frame_info_ptr next_frame = get_next_frame_sentinel_okay (this_frame);
+  while (get_frame_type (next_frame) == INLINE_FRAME)
+    next_frame = get_next_frame_sentinel_okay (next_frame);
 
-  VALUE_NEXT_FRAME_ID (value) = next_frame_id;
+  VALUE_NEXT_FRAME_ID (value) = get_frame_id (next_frame);
   VALUE_REGNUM (value) = regnum;
 
   /* Any structure stored in more than one register will always be
@@ -887,13 +885,6 @@  address_from_register (int regnum, frame_info_ptr frame)
     error (_("Invalid register #%d, expecting 0 <= # < %d"), regnum,
 	   regnum_max_excl);
 
-  /* 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_NEXT_FRAME_ID.  Therefore, we re-implement
-     the core of value_from_register, but use the null_frame_id.  */
-
   /* Some targets require a special conversion routine even for plain
      pointer types.  Avoid constructing a value object in those cases.  */
   if (gdbarch_convert_register_p (gdbarch, regnum, type))
@@ -915,7 +906,7 @@  address_from_register (int regnum, frame_info_ptr frame)
       return unpack_long (type, buf);
     }
 
-  value *value = gdbarch_value_from_register (gdbarch, type, regnum, nullptr);
+  value *value = gdbarch_value_from_register (gdbarch, type, regnum, frame);
   read_frame_register_value (value, frame);
 
   if (value->optimized_out ())
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index a0f3d94f3a11..478e9078de6f 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -2760,13 +2760,11 @@  rs6000_value_from_register (gdbarch *gdbarch, type *type, int regnum,
 
   value->set_lval (lval_register);
 
-  frame_id next_frame_id;
-  if (this_frame == nullptr)
-    next_frame_id = null_frame_id;
-  else
-    next_frame_id = get_frame_id (get_next_frame_sentinel_okay (this_frame));
+  frame_info_ptr next_frame = get_next_frame_sentinel_okay (this_frame);
+  while (get_frame_type (next_frame) == INLINE_FRAME)
+    next_frame = get_next_frame_sentinel_okay (next_frame);
 
-  VALUE_NEXT_FRAME_ID (value) = next_frame_id;
+  VALUE_NEXT_FRAME_ID (value) = get_frame_id (next_frame);
   VALUE_REGNUM (value) = regnum;
 
   /* Any structure stored in more than one register will always be