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 9/9] gdb: make value::allocate_register_lazy store id of next non-inline frame
Date: Thu, 21 Dec 2023 14:16:30 -0500	[thread overview]
Message-ID: <20231221191716.257256-10-simon.marchi@efficios.com> (raw)
In-Reply-To: <20231221191716.257256-1-simon.marchi@efficios.com>

Some spots loop on the frame chain to find the first next non-inline
frame, and pass that as the "next frame" to
value::allocate_register_lazy / value::allocate_register.  This is
necessary if the value is used in the process of computing the id of
"this frame".  If the frame next to "this frame" is inlined into "this
frame", then you that next frame won't have a computed id yet.  You have
to go past that to find the next non-inline frame, which will have a
computed id.

In other cases, it's fine to store the id of an inline frame as the
"next frame id" in a register struct value.  When trying to unwind a
register from it, it will just call inline_frame_prev_register, which
will forward the request to the next next frame, until we hit the next
physical frame.

I think it would make things simpler to just never store the id of an
inline frame as the next frame id of register struct values, and go with
the first next non-inline frame directly.  This way, we don't have to
wonder which code paths have to skip inline frames when creating
register values and which don't.

So, change value::allocate_register_lazy to do that work, and remove the
loops for the callers that did it.

Change-Id: Ic88115dac49dc14e3053c95f92050062b24b7310
---
 gdb/findvar.c     | 19 +++----------------
 gdb/rs6000-tdep.c |  7 ++-----
 gdb/value.c       | 12 ++++++++++++
 gdb/value.h       |  8 ++++----
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 7c360eb37ff9..90d3385b36d8 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -270,17 +270,6 @@ value_of_register_lazy (frame_info_ptr next_frame, int regnum)
   gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch));
   gdb_assert (next_frame != nullptr);
 
-  /* In some cases NEXT_FRAME may not have a valid frame-id yet.  This can
-     happen if we end up trying to unwind a register as part of the frame
-     sniffer.  The only time that we get here without a valid frame-id is
-     if NEXT_FRAME is an inline frame.  If this is the case then we can
-     avoid getting into trouble here by skipping past the inline frames.  */
-  while (get_frame_type (next_frame) == INLINE_FRAME)
-    next_frame = get_next_frame_sentinel_okay (next_frame);
-
-  /* We should have a valid next frame.  */
-  gdb_assert (frame_id_p (get_frame_id (next_frame)));
-
   return value::allocate_register_lazy (next_frame, regnum);
 }
 
@@ -746,11 +735,9 @@ value *
 default_value_from_register (gdbarch *gdbarch, type *type, int regnum,
 			     frame_info_ptr 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 *value = value::allocate_register (next_frame, regnum, type);
+  value *value
+    = value::allocate_register (get_next_frame_sentinel_okay (this_frame),
+				regnum, type);
 
   /* Any structure stored in more than one register will always be
      an integral number of registers.  Otherwise, you need to do
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index edf776853e20..1c9bb3e5f04f 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -2755,12 +2755,9 @@ rs6000_value_from_register (gdbarch *gdbarch, type *type, int regnum,
      fpr to vsr.  */
   regnum = ieee_128_float_regnum_adjust (gdbarch, type, regnum);
 
-  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 *value
-    = value::allocate_register (next_frame, regnum, type);
+    = value::allocate_register (get_next_frame_sentinel_okay (this_frame),
+				regnum, type);
 
   /* Any structure stored in more than one register will always be
      an integral number of registers.  Otherwise, you need to do
diff --git a/gdb/value.c b/gdb/value.c
index a69bc348167a..92e62d655d0c 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -972,8 +972,20 @@ value::allocate_register_lazy (frame_info_ptr next_frame, int regnum,
 
   result->set_lval (lval_register);
   result->m_location.reg.regnum = regnum;
+
+  /* If this register value is created during unwind (while computing a frame
+     id), and NEXT_FRAME is a frame inlined in the frame being unwound, then
+     NEXT_FRAME will not have a valid frame id yet.  Find the next non-inline
+     frame (possibly the sentinel frame).  This is where registers are unwound
+     from anyway.  */
+  while (get_frame_type (next_frame) == INLINE_FRAME)
+    next_frame = get_next_frame_sentinel_okay (next_frame);
+
   result->m_location.reg.next_frame_id = get_frame_id (next_frame);
 
+  /* We should have a next frame with a valid id.  */
+  gdb_assert (frame_id_p (result->m_location.reg.next_frame_id));
+
   return result;
 }
 
diff --git a/gdb/value.h b/gdb/value.h
index f1202007bb4f..22b3a09e5a5b 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -680,10 +680,10 @@ struct value
     {
       /* Register number.  */
       int regnum;
-      /* Frame ID of "next" frame to which a register value is relative.
-	 If the register value is found relative to frame F, then the
-	 frame id of F->next will be stored in next_frame_id.  */
-      struct frame_id next_frame_id;
+
+      /* Frame ID of the next physical (non-inline) frame to which a register
+	 value is relative.  */
+      frame_id next_frame_id;
     } reg;
 
     /* Pointer to internal variable.  */
-- 
2.43.0


  parent reply	other threads:[~2023-12-21 19:19 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 ` [PATCH 3/9] gdb: pass non-nullptr frame to gdbarch_value_from_register in address_from_register Simon Marchi
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 ` Simon Marchi [this message]
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-10-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).