public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 3/9] gdb: pass non-nullptr frame to gdbarch_value_from_register in address_from_register
Date: Thu, 21 Dec 2023 14:16:24 -0500	[thread overview]
Message-ID: <20231221191716.257256-4-simon.marchi@efficios.com> (raw)
In-Reply-To: <20231221191716.257256-1-simon.marchi@efficios.com>

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 --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
-- 
2.43.0


  parent reply	other threads:[~2023-12-21 19:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 19:16 [PATCH 0/9] Some register value cleanups Simon Marchi
2023-12-21 19:16 ` [PATCH 1/9] gdb: don't set frame id after calling cooked_read_value Simon Marchi
2023-12-21 19:16 ` [PATCH 2/9] gdb: pass frame_info_ptr to gdbarch_value_from_register Simon Marchi
2023-12-22 16:40   ` Tom Tromey
2023-12-22 16:53     ` Simon Marchi
2023-12-22 16:56       ` Tom Tromey
2023-12-21 19:16 ` Simon Marchi [this message]
2023-12-21 19:16 ` [PATCH 4/9] gdb: add type parameter to value::allocate_register and add value::allocate_register_lazy Simon Marchi
2023-12-21 19:16 ` [PATCH 5/9] gdb: remove read_frame_register_value's frame parameter Simon Marchi
2023-12-21 19:16 ` [PATCH 6/9] gdb: implement address_from_register using value_from_register Simon Marchi
2023-12-21 19:16 ` [PATCH 7/9] gdb: remove VALUE_NEXT_FRAME_ID, add value::next_frame_id Simon Marchi
2023-12-22 16:51   ` Tom Tromey
2023-12-22 16:56     ` Simon Marchi
2023-12-22 17:02       ` Tom Tromey
2023-12-22 17:06         ` Simon Marchi
2023-12-24 15:35           ` Simon Marchi
2023-12-21 19:16 ` [PATCH 8/9] gdb: remove VALUE_REGNUM, add value::regnum Simon Marchi
2023-12-22 16:52   ` Tom Tromey
2023-12-22 16:57     ` Simon Marchi
2023-12-21 19:16 ` [PATCH 9/9] gdb: make value::allocate_register_lazy store id of next non-inline frame Simon Marchi
2023-12-22 16:53 ` [PATCH 0/9] Some register value cleanups Tom Tromey
2023-12-22 16:58   ` Simon Marchi
2023-12-22 17:02     ` Tom Tromey
2023-12-24 18:28       ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231221191716.257256-4-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).