public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame
Date: Wed,  8 Nov 2023 00:00:52 -0500	[thread overview]
Message-ID: <20231108051222.1275306-9-simon.marchi@polymtl.ca> (raw)
In-Reply-To: <20231108051222.1275306-1-simon.marchi@polymtl.ca>

From: Simon Marchi <simon.marchi@efficios.com>

Some functions related to the handling of registers in frames accept
"this frame", for which we want to read or write the register values,
while other functions accept "the next frame", which is the frame next
to that.  The later is needed because we sometimes need to read register
values for a frame that does not exist yet (usually when trying to
unwind that frame-to-be).

value_of_register and value_of_register_lazy both take "this frame",
even if what they ultimately want internally is "the next frame".  This
is annoying if you are in a spot that currently has "the next frame" and
need to call one of these functions (which happens later in this
series).  You need to get the previous frame only for those functions to
get the next frame again.  This is more manipulations, more chances of
mistake.

I propose to change these functions (and a few more functions in the
subsequent patches) to operate on "the next frame".  Things become a bit
less awkward when all these functions agree on which frame they take.

So, in this patch, change value_of_register_lazy and value_of_register
to take "the next frame" instead of "this frame".  This adds a lot of
get_next_frame_sentinel_okay, but if we convert the user registers API
to also use "the next frame" instead of "this frame", it will get simple
again.

Change-Id: Iaa24815e648fbe5ae3c214c738758890a91819cd
---
 gdb/aarch64-tdep.c     |  3 +--
 gdb/arm-tdep.c         |  4 ++--
 gdb/eval.c             |  3 ++-
 gdb/findvar.c          | 34 ++++++++++++----------------------
 gdb/frame-unwind.c     |  3 ++-
 gdb/guile/scm-frame.c  |  3 ++-
 gdb/infcmd.c           |  6 +++---
 gdb/loongarch-tdep.c   |  3 ++-
 gdb/mi/mi-main.c       |  3 ++-
 gdb/mips-tdep.c        |  2 +-
 gdb/nds32-tdep.c       |  5 +++--
 gdb/python/py-frame.c  |  3 ++-
 gdb/python/py-unwind.c |  4 ++--
 gdb/riscv-tdep.c       |  4 ++--
 gdb/s12z-tdep.c        |  2 +-
 gdb/std-regs.c         | 11 +++++++----
 gdb/value.h            |  9 +++++++--
 17 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index eaae2d91047b..50bfaa41eee2 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3449,9 +3449,8 @@ value_of_aarch64_user_reg (frame_info_ptr frame, const void *baton)
 {
   const int *reg_p = (const int *) baton;
 
-  return value_of_register (*reg_p, frame);
+  return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame));
 }
-\f
 
 /* Implement the "software_single_step" gdbarch method, needed to
    single step through atomic sequences on AArch64.  */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 2018f4ed0edd..5c8c4cd4e94b 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9954,9 +9954,9 @@ static struct value *
 value_of_arm_user_reg (frame_info_ptr frame, const void *baton)
 {
   const int *reg_p = (const int *) baton;
-  return value_of_register (*reg_p, frame);
+  return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame));
 }
-\f
+
 static enum gdb_osabi
 arm_elf_osabi_sniffer (bfd *abfd)
 {
diff --git a/gdb/eval.c b/gdb/eval.c
index b859e825925b..16206d92702a 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1134,7 +1134,8 @@ eval_op_register (struct type *expect_type, struct expression *exp,
       && regno < gdbarch_num_cooked_regs (exp->gdbarch))
     val = value::zero (register_type (exp->gdbarch, regno), not_lval);
   else
-    val = value_of_register (regno, get_selected_frame (NULL));
+    val = value_of_register
+      (regno, get_next_frame_sentinel_okay (get_selected_frame ()));
   if (val == NULL)
     error (_("Value of register %s not available."), name);
   else
diff --git a/gdb/findvar.c b/gdb/findvar.c
index edb61fb27317..6ee31e780932 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -244,42 +244,32 @@ copy_integer_to_size (gdb_byte *dest, int dest_size, const gdb_byte *source,
     }
 }
 
-/* Return a `value' with the contents of (virtual or cooked) register
-   REGNUM as found in the specified FRAME.  The register's type is
-   determined by register_type ().  */
+/* See value.h.  */
 
-struct value *
-value_of_register (int regnum, frame_info_ptr frame)
+value *
+value_of_register (int regnum, frame_info_ptr next_frame)
 {
-  struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct value *reg_val;
+  gdbarch *gdbarch = frame_unwind_arch (next_frame);
 
   /* User registers lie completely outside of the range of normal
      registers.  Catch them early so that the target never sees them.  */
   if (regnum >= gdbarch_num_cooked_regs (gdbarch))
-    return value_of_user_reg (regnum, frame);
+    return value_of_user_reg (regnum, get_prev_frame_always (next_frame));
 
-  reg_val = value_of_register_lazy (frame, regnum);
+  value *reg_val = value_of_register_lazy (next_frame, regnum);
   reg_val->fetch_lazy ();
   return reg_val;
 }
 
-/* Return a `value' with the contents of (virtual or cooked) register
-   REGNUM as found in the specified FRAME.  The register's type is
-   determined by register_type ().  The value is not fetched.  */
+/* See value.h.  */
 
-struct value *
-value_of_register_lazy (frame_info_ptr frame, int regnum)
+value *
+value_of_register_lazy (frame_info_ptr next_frame, int regnum)
 {
-  struct gdbarch *gdbarch = get_frame_arch (frame);
-  struct value *reg_val;
-  frame_info_ptr next_frame;
+  gdbarch *gdbarch = frame_unwind_arch (next_frame);
 
   gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch));
-
-  gdb_assert (frame != NULL);
-
-  next_frame = get_next_frame_sentinel_okay (frame);
+  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
@@ -292,7 +282,7 @@ value_of_register_lazy (frame_info_ptr frame, int regnum)
   /* We should have a valid next frame.  */
   gdb_assert (frame_id_p (get_frame_id (next_frame)));
 
-  reg_val = value::allocate_lazy (register_type (gdbarch, regnum));
+  value *reg_val = value::allocate_lazy (register_type (gdbarch, regnum));
   reg_val->set_lval (lval_register);
   VALUE_REGNUM (reg_val) = regnum;
   VALUE_NEXT_FRAME_ID (reg_val) = get_frame_id (next_frame);
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index 927b6256b0b3..89ae3d70b200 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -278,7 +278,8 @@ struct value *
 frame_unwind_got_register (frame_info_ptr frame,
 			   int regnum, int new_regnum)
 {
-  return value_of_register_lazy (frame, new_regnum);
+  return value_of_register_lazy (get_next_frame_sentinel_okay (frame),
+				 new_regnum);
 }
 
 /* Return a value which indicates that FRAME saved REGNUM in memory at
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index 45863c587c1d..830acb8b53ed 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -835,7 +835,8 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
 						register_str,
 						strlen (register_str));
 	  if (regnum >= 0)
-	    value = value_of_register (regnum, frame);
+	    value = value_of_register (regnum,
+				       get_next_frame_sentinel_okay (frame));
 	}
     }
   catch (const gdb_exception &ex)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index cf8cd5279558..63b700c1e669 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2318,9 +2318,9 @@ default_print_registers_info (struct gdbarch *gdbarch,
       if (*(gdbarch_register_name (gdbarch, i)) == '\0')
 	continue;
 
-      default_print_one_register_info (file,
-				       gdbarch_register_name (gdbarch, i),
-				       value_of_register (i, frame));
+      default_print_one_register_info
+	(file, gdbarch_register_name (gdbarch, i),
+	 value_of_register (i, get_next_frame_sentinel_okay (frame)));
     }
 }
 
diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c
index c65e2414bf8a..6d862dcec044 100644
--- a/gdb/loongarch-tdep.c
+++ b/gdb/loongarch-tdep.c
@@ -392,7 +392,8 @@ loongarch_software_single_step (struct regcache *regcache)
 static struct value *
 value_of_loongarch_user_reg (frame_info_ptr frame, const void *baton)
 {
-  return value_of_register ((long long) baton, frame);
+  return value_of_register ((long long) baton,
+			    get_next_frame_sentinel_okay (frame));
 }
 
 /* Implement the frame_align gdbarch method.  */
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 944c9116731f..dd5079f87c6c 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1101,7 +1101,8 @@ output_register (frame_info_ptr frame, int regnum, int format,
 		 int skip_unavailable)
 {
   struct ui_out *uiout = current_uiout;
-  struct value *val = value_of_register (regnum, frame);
+  value *val
+    = value_of_register (regnum, get_next_frame_sentinel_okay (frame));
   struct value_print_options opts;
 
   if (skip_unavailable && !val->entirely_available ())
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6bd0a66a6284..ac90aa1f2c5f 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -8074,7 +8074,7 @@ static struct value *
 value_of_mips_user_reg (frame_info_ptr frame, const void *baton)
 {
   const int *reg_p = (const int *) baton;
-  return value_of_register (*reg_p, frame);
+  return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame));
 }
 
 static struct gdbarch *
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index a81b5fd1fae1..7618a81fc256 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -267,9 +267,10 @@ static const struct
 static struct value *
 value_of_nds32_reg (frame_info_ptr frame, const void *baton)
 {
-  return value_of_register ((int) (intptr_t) baton, frame);
+  return value_of_register ((int) (intptr_t) baton,
+			    get_next_frame_sentinel_okay (frame));
 }
-\f
+
 /* Implement the "frame_align" gdbarch method.  */
 
 static CORE_ADDR
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 1a55e514e396..e6e245520323 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -260,7 +260,8 @@ frapy_read_register (PyObject *self, PyObject *args, PyObject *kw)
 	return nullptr;
 
       gdb_assert (regnum >= 0);
-      struct value *val = value_of_register (regnum, frame);
+      value *val
+	= value_of_register (regnum, get_next_frame_sentinel_okay (frame));
 
       if (val == NULL)
 	PyErr_SetString (PyExc_ValueError, _("Can't read register."));
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index ee50c51b531d..f1162f22290f 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -467,8 +467,8 @@ pending_framepy_read_register (PyObject *self, PyObject *args, PyObject *kw)
 	 which maps to a real register.  In the past,
 	 get_frame_register_value() was used here, which did not
 	 handle the user register case.  */
-      struct value *val = value_of_register (regnum,
-					     pending_frame->frame_info);
+      value *val = value_of_register
+        (regnum, get_next_frame_sentinel_okay (pending_frame->frame_info));
       if (val == NULL)
 	PyErr_Format (PyExc_ValueError,
 		      "Cannot read register %d from frame.",
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 3a2891c2c920..48f5a9e35e67 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -168,7 +168,7 @@ static struct value *
 value_of_riscv_user_reg (frame_info_ptr frame, const void *baton)
 {
   const int *reg_p = (const int *) baton;
-  return value_of_register (*reg_p, frame);
+  return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame));
 }
 
 /* Information about a register alias that needs to be set up for this
@@ -1149,7 +1149,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch,
 
   try
     {
-      val = value_of_register (regnum, frame);
+      val = value_of_register (regnum, get_next_frame_sentinel_okay (frame));
       regtype = val->type ();
     }
   catch (const gdb_exception_error &ex)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index 4781eab083eb..42fd1fb1d6ca 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -494,7 +494,7 @@ s12z_print_ccw_info (struct gdbarch *gdbarch,
 		     frame_info_ptr frame,
 		     int reg)
 {
-  struct value *v = value_of_register (reg, frame);
+  value *v = value_of_register (reg, get_next_frame_sentinel_okay (frame));
   const char *name = gdbarch_register_name (gdbarch, reg);
   uint32_t ccw = value_as_long (v);
   gdb_puts (name, file);
diff --git a/gdb/std-regs.c b/gdb/std-regs.c
index 54cf9018e426..221b15f1de9f 100644
--- a/gdb/std-regs.c
+++ b/gdb/std-regs.c
@@ -39,7 +39,7 @@ value_of_builtin_frame_fp_reg (frame_info_ptr frame, const void *baton)
        register can do so by adding "fp" to register name table (mind
        you, doing this is probably a dangerous thing).  */
     return value_of_register (gdbarch_deprecated_fp_regnum (gdbarch),
-			      frame);
+			      get_next_frame_sentinel_okay (frame));
   else
     {
       struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
@@ -58,7 +58,8 @@ value_of_builtin_frame_pc_reg (frame_info_ptr frame, const void *baton)
   struct gdbarch *gdbarch = get_frame_arch (frame);
 
   if (gdbarch_pc_regnum (gdbarch) >= 0)
-    return value_of_register (gdbarch_pc_regnum (gdbarch), frame);
+    return value_of_register (gdbarch_pc_regnum (gdbarch),
+			      get_next_frame_sentinel_okay (frame));
   else
     {
       struct type *func_ptr_type = builtin_type (gdbarch)->builtin_func_ptr;
@@ -77,7 +78,8 @@ value_of_builtin_frame_sp_reg (frame_info_ptr frame, const void *baton)
   struct gdbarch *gdbarch = get_frame_arch (frame);
 
   if (gdbarch_sp_regnum (gdbarch) >= 0)
-    return value_of_register (gdbarch_sp_regnum (gdbarch), frame);
+    return value_of_register (gdbarch_sp_regnum (gdbarch),
+			      get_next_frame_sentinel_okay (frame));
   error (_("Standard register ``$sp'' is not available for this target"));
 }
 
@@ -87,7 +89,8 @@ value_of_builtin_frame_ps_reg (frame_info_ptr frame, const void *baton)
   struct gdbarch *gdbarch = get_frame_arch (frame);
 
   if (gdbarch_ps_regnum (gdbarch) >= 0)
-    return value_of_register (gdbarch_ps_regnum (gdbarch), frame);
+    return value_of_register (gdbarch_ps_regnum (gdbarch),
+			      get_next_frame_sentinel_okay (frame));
   error (_("Standard register ``$ps'' is not available for this target"));
 }
 
diff --git a/gdb/value.h b/gdb/value.h
index e4912717684b..37017c9dd9f7 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1123,9 +1123,14 @@ extern struct value *value_of_variable (struct symbol *var,
 extern struct value *address_of_variable (struct symbol *var,
 					  const struct block *b);
 
-extern struct value *value_of_register (int regnum, frame_info_ptr frame);
+/* Return a value with the contents of register REGNUM as found in the frame
+   previous to NEXT_FRAME.  */
 
-struct value *value_of_register_lazy (frame_info_ptr frame, int regnum);
+extern value *value_of_register (int regnum, frame_info_ptr next_frame);
+
+/* Same as the above, but the value is not fetched.  */
+
+extern value *value_of_register_lazy (frame_info_ptr next_frame, int regnum);
 
 /* Return the symbol's reading requirement.  */
 
-- 
2.42.1


  parent reply	other threads:[~2023-11-08  5:14 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-08  5:00 [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-08  5:00 ` [PATCH 01/24] gdb: don't handle i386 k registers as pseudo registers Simon Marchi
2023-11-11 19:29   ` John Baldwin
2023-11-08  5:00 ` [PATCH 02/24] gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h Simon Marchi
2023-11-11 19:42   ` John Baldwin
2023-11-08  5:00 ` [PATCH 03/24] gdb: make store_integer take an array_view Simon Marchi
2023-11-08  5:00 ` [PATCH 04/24] gdb: simplify conditions in regcache::{read,write,raw_collect,raw_supply}_part Simon Marchi
2023-11-08  5:00 ` [PATCH 05/24] gdb: change regcache interface to use array_view Simon Marchi
2023-11-13 13:43   ` Andrew Burgess
2023-11-13 14:00     ` Andrew Burgess
2023-11-13 16:47       ` Simon Marchi
2023-11-08  5:00 ` [PATCH 06/24] gdb: fix bugs in {get,put}_frame_register_bytes Simon Marchi
2023-11-13 15:00   ` Andrew Burgess
2023-11-13 19:51     ` Simon Marchi
2023-11-08  5:00 ` [PATCH 07/24] gdb: make put_frame_register take an array_view Simon Marchi
2023-11-08  5:00 ` Simon Marchi [this message]
2023-11-08  5:00 ` [PATCH 09/24] gdb: remove frame_register Simon Marchi
2023-11-08  5:00 ` [PATCH 10/24] gdb: make put_frame_register take the next frame Simon Marchi
2023-11-08  5:00 ` [PATCH 11/24] gdb: make put_frame_register_bytes " Simon Marchi
2023-11-08  5:00 ` [PATCH 12/24] gdb: make get_frame_register_bytes " Simon Marchi
2023-11-08  5:00 ` [PATCH 13/24] gdb: add value::allocate_register Simon Marchi
2023-11-08  5:00 ` [PATCH 14/24] gdb: read pseudo register through frame Simon Marchi
2023-11-11 20:11   ` John Baldwin
2023-11-08  5:00 ` [PATCH 15/24] gdb: change parameter name in frame_unwind_register_unsigned declaration Simon Marchi
2023-11-08  5:01 ` [PATCH 16/24] gdb: rename gdbarch_pseudo_register_write to gdbarch_deprecated_pseudo_register_write Simon Marchi
2023-11-14 12:12   ` Andrew Burgess
2023-11-14 15:16     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame Simon Marchi
2023-11-14 12:20   ` Andrew Burgess
2023-11-14 15:20     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 18/24] gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write Simon Marchi
2023-11-11 20:16   ` John Baldwin
2023-11-13  2:59     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 19/24] gdb: make aarch64_za_offsets_from_regnum return za_offsets Simon Marchi
2023-11-08  5:01 ` [PATCH 20/24] gdb: add missing raw register read in aarch64_sme_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 21/24] gdb: migrate aarch64 to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 22/24] gdb: migrate arm to gdbarch_pseudo_register_read_value Simon Marchi
2023-11-08  5:01 ` [PATCH 23/24] gdb: migrate arm to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 24/24] gdb/testsuite: add tests for unwinding of pseudo registers Simon Marchi
2023-11-08  5:16 ` [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-09  3:05   ` Simon Marchi
2023-11-08 11:57 ` Luis Machado
2023-11-08 15:47   ` Simon Marchi
2023-11-08 17:08     ` Luis Machado
2023-11-08 19:34       ` Simon Marchi
2023-11-09 19:04         ` Simon Marchi
2023-11-13 13:10           ` Luis Machado
2023-11-13 15:08             ` Luis Machado
2023-11-11 20:26 ` John Baldwin
2023-11-13  3:03   ` Simon Marchi
2023-12-01 16:27 Simon Marchi
2023-12-01 16:27 ` [PATCH 08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame 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=20231108051222.1275306-9-simon.marchi@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@efficios.com \
    /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).