public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Make some functions independent of current inferior
@ 2022-11-24 16:04 Simon Marchi
  2022-11-24 16:04 ` [PATCH 1/5] gdb: add inferior parameter to target_current_description Simon Marchi
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Here's a little series making some functions independent from the
current inferior, taking an inferior parameter instead.  This is in
preparation for some other work I'm doing, but I thought it could go in
as its own.

Simon Marchi (5):
  gdb: add inferior parameter to target_current_description
  gdb: add inferior parameter to set_target_gdbarch, rename to
    set_inferior_gdbarch
  gdb: add inferior parameter to gdbarch_update_p
  gdb: add inferior parameter to target_find_description
  gdb: add inferior parameter to target_clear_description

 gdb/arch-utils.c          | 45 ++++++++++++++++++------------------
 gdb/arm-tdep.c            |  2 +-
 gdb/cris-tdep.c           |  6 ++---
 gdb/gdbarch.h             |  8 +++----
 gdb/i386-darwin-nat.c     |  2 +-
 gdb/infcmd.c              |  2 +-
 gdb/infrun.c              |  4 ++--
 gdb/mips-tdep.c           | 10 ++++----
 gdb/osabi.c               |  3 ++-
 gdb/remote.c              |  8 +++----
 gdb/rs6000-aix-nat.c      |  2 +-
 gdb/rs6000-tdep.c         |  4 ++--
 gdb/target-descriptions.c | 48 +++++++++++++++++++++------------------
 gdb/target-descriptions.h | 10 ++++----
 gdb/target.c              |  2 +-
 gdb/tracefile-tfile.c     |  2 +-
 16 files changed, 82 insertions(+), 76 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] gdb: add inferior parameter to target_current_description
  2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
@ 2022-11-24 16:04 ` Simon Marchi
  2022-11-24 16:04 ` [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch Simon Marchi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Make target_current_description not dependent on the current inferior on
entry.  Update all callers to pass the current inferior, so no change in
behavior is expected.

Change-Id: Ic3c501bc83eb6950db077001a96a5c70dc8ae942
---
 gdb/arch-utils.c          | 4 ++--
 gdb/target-descriptions.c | 4 ++--
 gdb/target-descriptions.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 7b84daf046e..dc67c632155 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -586,7 +586,7 @@ gdbarch_update_p (struct gdbarch_info info)
 
   /* Check for the current target description.  */
   if (info.target_desc == NULL)
-    info.target_desc = target_current_description ();
+    info.target_desc = target_current_description (current_inferior ());
 
   new_gdbarch = gdbarch_find_by_info (info);
 
@@ -644,7 +644,7 @@ set_gdbarch_from_file (bfd *abfd)
   struct gdbarch *gdbarch;
 
   info.abfd = abfd;
-  info.target_desc = target_current_description ();
+  info.target_desc = target_current_description (current_inferior ());
   gdbarch = gdbarch_find_by_info (info);
 
   if (gdbarch == NULL)
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 44dea711a39..0d50aadddb8 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -607,9 +607,9 @@ target_clear_description (void)
    an existing gdbarch.  */
 
 const struct target_desc *
-target_current_description (void)
+target_current_description (inferior *inf)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = get_tdesc_info (inf);
 
   if (tdesc_info->fetched)
     return tdesc_info->tdesc;
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 3ab0ae2542d..3049b783e2f 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -46,7 +46,7 @@ void target_clear_description (void);
    be used by gdbarch initialization code; most access should be
    through an existing gdbarch.  */
 
-const struct target_desc *target_current_description (void);
+const struct target_desc *target_current_description (inferior *inf);
 
 /* Copy inferior target description data.  Used for example when
    handling (v)forks, where child's description is the same as the
-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch
  2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
  2022-11-24 16:04 ` [PATCH 1/5] gdb: add inferior parameter to target_current_description Simon Marchi
@ 2022-11-24 16:04 ` Simon Marchi
  2022-11-24 16:42   ` Lancelot SIX
  2022-11-24 16:04 ` [PATCH 3/5] gdb: add inferior parameter to gdbarch_update_p Simon Marchi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Add an inferior parameter, so it doesn't depend on the current
inferior.  While at it, rename to set_inferior_gdbarch, I think that's a
better name than "target".

The sole observer of the architecture_changed observable,
pyuw_on_new_gdbarch, doesn't seem to depend on the current inferior.
Neither does registers_changed.

Update callers to pass the current inferior, no change in behavior is
expected.

Change-Id: I276e28eafd4740c94bc5233c81a86c01b4a6ae90
---
 gdb/arch-utils.c | 16 ++++++++--------
 gdb/gdbarch.h    |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index dc67c632155..92caa5c3c4a 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -617,7 +617,7 @@ gdbarch_update_p (struct gdbarch_info info)
 		"New architecture %s (%s) selected\n",
 		host_address_to_string (new_gdbarch),
 		gdbarch_bfd_arch_info (new_gdbarch)->printable_name);
-  set_target_gdbarch (new_gdbarch);
+  set_inferior_gdbarch (current_inferior (), new_gdbarch);
 
   return 1;
 }
@@ -649,7 +649,7 @@ set_gdbarch_from_file (bfd *abfd)
 
   if (gdbarch == NULL)
     error (_("Architecture of file not recognized."));
-  set_target_gdbarch (gdbarch);
+  set_inferior_gdbarch (current_inferior (), gdbarch);
 }
 
 /* Initialize the current architecture.  Update the ``set
@@ -1427,15 +1427,15 @@ gdbarch_find_by_info (struct gdbarch_info info)
   return new_gdbarch;
 }
 
-/* Make the specified architecture current.  */
+/* See gdbarch.h.  */
 
 void
-set_target_gdbarch (struct gdbarch *new_gdbarch)
+set_inferior_gdbarch (inferior *inf, gdbarch *gdbarch)
 {
-  gdb_assert (new_gdbarch != NULL);
-  gdb_assert (new_gdbarch->initialized_p);
-  current_inferior ()->gdbarch = new_gdbarch;
-  gdb::observers::architecture_changed.notify (new_gdbarch);
+  gdb_assert (gdbarch != nullptr);
+  gdb_assert (gdbarch->initialized_p);
+  inf->gdbarch = gdbarch;
+  gdb::observers::architecture_changed.notify (gdbarch);
   registers_changed ();
 }
 
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index f2ba5f97765..76ffddfe0ff 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -342,9 +342,9 @@ extern int gdbarch_update_p (struct gdbarch_info info);
 extern struct gdbarch *gdbarch_find_by_info (struct gdbarch_info info);
 
 
-/* Helper function.  Set the target gdbarch to "gdbarch".  */
+/* Set INF's the target gdbarch to "gdbarch".  */
 
-extern void set_target_gdbarch (struct gdbarch *gdbarch);
+extern void set_inferior_gdbarch (inferior *inf, gdbarch *gdbarch);
 
 
 /* A registry adaptor for gdbarch.  This arranges to store the
-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] gdb: add inferior parameter to gdbarch_update_p
  2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
  2022-11-24 16:04 ` [PATCH 1/5] gdb: add inferior parameter to target_current_description Simon Marchi
  2022-11-24 16:04 ` [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch Simon Marchi
@ 2022-11-24 16:04 ` Simon Marchi
  2022-11-24 16:04 ` [PATCH 4/5] gdb: add inferior parameter to target_find_description Simon Marchi
  2022-11-24 16:04 ` [PATCH 5/5] gdb: add inferior parameter to target_clear_description Simon Marchi
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

gdbarch_update_p sets the current inferior's gdbarch, based on the
passed gdbarch_info.  Add an inferior parameter, so it doesn't depend on
the current inferior.

The difficult thing is to ensure that all gdbarch and osabi init
functions are independent of the current inferior.  There are so many,
it's not realistic to read them all to be sure.  However, I added
temporarily a little hack to ensure current_inferior isn't called during
`rego->init`, and ran "maint selftest" on an all targets build, and it
worked fine.  Given that this instantiates many variants of all
architectures, it gives some confidence that no init function relies on
the current inferior.

Update callers to pass the current inferior, no change in behavior is
expected.

Change-Id: I011e40e74a0e4e2dd5eeb0b3b59eb39f4813ab9e
---
 gdb/arch-utils.c          | 27 ++++++++++++++-------------
 gdb/arm-tdep.c            |  2 +-
 gdb/cris-tdep.c           |  6 +++---
 gdb/gdbarch.h             |  4 ++--
 gdb/i386-darwin-nat.c     |  2 +-
 gdb/mips-tdep.c           | 10 +++++-----
 gdb/osabi.c               |  3 ++-
 gdb/rs6000-aix-nat.c      |  2 +-
 gdb/rs6000-tdep.c         |  4 ++--
 gdb/target-descriptions.c |  4 ++--
 10 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 92caa5c3c4a..878c1849e8a 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -396,13 +396,13 @@ set_endian (const char *ignore_args, int from_tty, struct cmd_list_element *c)
   if (set_endian_string == endian_auto)
     {
       target_byte_order_user = BFD_ENDIAN_UNKNOWN;
-      if (! gdbarch_update_p (info))
+      if (! gdbarch_update_p (current_inferior (), info))
 	internal_error (_("set_endian: architecture update failed"));
     }
   else if (set_endian_string == endian_little)
     {
       info.byte_order = BFD_ENDIAN_LITTLE;
-      if (! gdbarch_update_p (info))
+      if (! gdbarch_update_p (current_inferior (), info))
 	gdb_printf (gdb_stderr,
 		    _("Little endian target not supported by GDB\n"));
       else
@@ -411,7 +411,7 @@ set_endian (const char *ignore_args, int from_tty, struct cmd_list_element *c)
   else if (set_endian_string == endian_big)
     {
       info.byte_order = BFD_ENDIAN_BIG;
-      if (! gdbarch_update_p (info))
+      if (! gdbarch_update_p (current_inferior (), info))
 	gdb_printf (gdb_stderr,
 		    _("Big endian target not supported by GDB\n"));
       else
@@ -553,7 +553,7 @@ set_architecture (const char *ignore_args,
   if (strcmp (set_architecture_string, "auto") == 0)
     {
       target_architecture_user = NULL;
-      if (!gdbarch_update_p (info))
+      if (!gdbarch_update_p (current_inferior (), info))
 	internal_error (_("could not select an architecture automatically"));
     }
   else
@@ -561,7 +561,7 @@ set_architecture (const char *ignore_args,
       info.bfd_arch_info = bfd_scan_arch (set_architecture_string);
       if (info.bfd_arch_info == NULL)
 	internal_error (_("set_architecture: bfd_scan_arch failed"));
-      if (gdbarch_update_p (info))
+      if (gdbarch_update_p (current_inferior (), info))
 	target_architecture_user = info.bfd_arch_info;
       else
 	gdb_printf (gdb_stderr,
@@ -571,22 +571,23 @@ set_architecture (const char *ignore_args,
   show_architecture (gdb_stdout, from_tty, NULL, NULL);
 }
 
-/* Try to select a global architecture that matches "info".  Return
-   non-zero if the attempt succeeds.  */
+/* See gdbarch.h.  */
+
 int
-gdbarch_update_p (struct gdbarch_info info)
+gdbarch_update_p (inferior *inf, struct gdbarch_info info)
 {
   struct gdbarch *new_gdbarch;
 
   /* Check for the current file.  */
   if (info.abfd == NULL)
-    info.abfd = current_program_space->exec_bfd ();
+    info.abfd = inf->pspace->exec_bfd ();
+
   if (info.abfd == NULL)
-    info.abfd = core_bfd;
+    info.abfd = inf->pspace->cbfd.get ();
 
   /* Check for the current target description.  */
   if (info.target_desc == NULL)
-    info.target_desc = target_current_description (current_inferior ());
+    info.target_desc = target_current_description (inf);
 
   new_gdbarch = gdbarch_find_by_info (info);
 
@@ -601,7 +602,7 @@ gdbarch_update_p (struct gdbarch_info info)
 
   /* If it is the same old architecture, accept the request (but don't
      swap anything).  */
-  if (new_gdbarch == target_gdbarch ())
+  if (new_gdbarch == inf->gdbarch)
     {
       if (gdbarch_debug)
 	gdb_printf (gdb_stdlog, "gdbarch_update_p: "
@@ -741,7 +742,7 @@ initialize_current_architecture (void)
   info.byte_order = default_byte_order;
   info.byte_order_for_code = info.byte_order;
 
-  if (! gdbarch_update_p (info))
+  if (! gdbarch_update_p (current_inferior (), info))
     internal_error (_("initialize_current_architecture: Selection of "
 		      "initial architecture failed"));
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 6f02f04b5cb..0badf3f2b1a 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9393,7 +9393,7 @@ arm_update_current_architecture (void)
 
   /* Update the architecture.  */
   gdbarch_info info;
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("could not update architecture"));
 }
 
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index d38850aa1af..7d2641a4eb0 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -3883,7 +3883,7 @@ set_cris_version (const char *ignore_args, int from_tty,
   usr_cmd_cris_version_valid = 1;
   
   /* Update the current architecture, if needed.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("cris_gdbarch_update: failed to update architecture."));
 }
 
@@ -3894,7 +3894,7 @@ set_cris_mode (const char *ignore_args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the current architecture, if needed.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error ("cris_gdbarch_update: failed to update architecture.");
 }
 
@@ -3905,7 +3905,7 @@ set_cris_dwarf2_cfi (const char *ignore_args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the current architecture, if needed.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("cris_gdbarch_update: failed to update architecture."));
 }
 
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 76ffddfe0ff..55e4a9b4743 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -319,7 +319,7 @@ extern obstack *gdbarch_obstack (gdbarch *arch);
 
 extern char *gdbarch_obstack_strdup (struct gdbarch *arch, const char *string);
 
-/* Helper function.  Force an update of the current architecture.
+/* Helper function.  Force an update of INF's current architecture.
 
    The actual architecture selected is determined by INFO, ``(gdb) set
    architecture'' et.al., the existing architecture and BFD's default
@@ -328,7 +328,7 @@ extern char *gdbarch_obstack_strdup (struct gdbarch *arch, const char *string);
 
    Returns non-zero if the update succeeds.  */
 
-extern int gdbarch_update_p (struct gdbarch_info info);
+extern int gdbarch_update_p (inferior *inf, struct gdbarch_info info);
 
 
 /* Helper function.  Find an architecture matching info.
diff --git a/gdb/i386-darwin-nat.c b/gdb/i386-darwin-nat.c
index b64b2b3e81b..b06d0007740 100644
--- a/gdb/i386-darwin-nat.c
+++ b/gdb/i386-darwin-nat.c
@@ -500,7 +500,7 @@ darwin_check_osabi (darwin_inferior *inf, thread_t thread)
       else
 	info.bfd_arch_info = bfd_lookup_arch (bfd_arch_i386,
 					      bfd_mach_i386_i386);
-      gdbarch_update_p (info);
+      gdbarch_update_p (current_inferior (), info);
     }
 }
 
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 8c1643585f4..6474b8dc26e 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -894,7 +894,7 @@ set_mips64_transfers_32bit_regs (const char *args, int from_tty,
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
      instead of relying on globals.  Doing that would let generic code
      handle the search for this specific architecture.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     {
       mips64_transfers_32bit_regs_p = 0;
       error (_("32-bit compatibility mode not supported"));
@@ -6965,7 +6965,7 @@ set_mipsfpu_single_command (const char *args, int from_tty)
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
      instead of relying on globals.  Doing that would let generic code
      handle the search for this specific architecture.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("set mipsfpu failed"));
 }
 
@@ -6978,7 +6978,7 @@ set_mipsfpu_double_command (const char *args, int from_tty)
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
      instead of relying on globals.  Doing that would let generic code
      handle the search for this specific architecture.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("set mipsfpu failed"));
 }
 
@@ -6991,7 +6991,7 @@ set_mipsfpu_none_command (const char *args, int from_tty)
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
      instead of relying on globals.  Doing that would let generic code
      handle the search for this specific architecture.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("set mipsfpu failed"));
 }
 
@@ -8836,7 +8836,7 @@ mips_abi_update (const char *ignore_args,
 
   /* Force the architecture to update, and (if it's a MIPS architecture)
      mips_gdbarch_init will take care of the rest.  */
-  gdbarch_update_p (info);
+  gdbarch_update_p (current_inferior (), info);
 }
 
 /* Print out which MIPS ABI is in use.  */
diff --git a/gdb/osabi.c b/gdb/osabi.c
index 57e2df6b25c..9c98c182fd7 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -24,6 +24,7 @@
 #include "gdbcmd.h"
 #include "command.h"
 #include "gdb_bfd.h"
+#include "inferior.h"
 
 #include "elf-bfd.h"
 
@@ -647,7 +648,7 @@ set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
   /* NOTE: At some point (true multiple architectures) we'll need to be more
      graceful here.  */
   gdbarch_info info;
-  if (! gdbarch_update_p (info))
+  if (! gdbarch_update_p (current_inferior (), info))
     internal_error (_("Updating OS ABI failed."));
 }
 
diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index 2ac1f6e70b6..b9ae6ea405c 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -765,7 +765,7 @@ rs6000_nat_target::create_inferior (const char *exec_file,
   info.bfd_arch_info = bfd_get_arch_info (&abfd);
   info.abfd = current_program_space->exec_bfd ();
 
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("rs6000_create_inferior: failed "
 		      "to select architecture"));
 }
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index cbd84514795..5991effa03e 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -8479,7 +8479,7 @@ powerpc_set_soft_float (const char *args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the architecture.  */
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("could not update architecture"));
 }
 
@@ -8505,7 +8505,7 @@ powerpc_set_vector_abi (const char *args, int from_tty,
 
   /* Update the architecture.  */
   gdbarch_info info;
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("could not update architecture"));
 }
 
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 0d50aadddb8..57d23747f26 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -564,7 +564,7 @@ target_find_description (void)
       struct gdbarch_info info;
 
       info.target_desc = tdesc_info->tdesc;
-      if (!gdbarch_update_p (info))
+      if (!gdbarch_update_p (current_inferior (), info))
 	warning (_("Architecture rejected target-supplied description"));
       else
 	{
@@ -598,7 +598,7 @@ target_clear_description (void)
   tdesc_info->tdesc = nullptr;
 
   gdbarch_info info;
-  if (!gdbarch_update_p (info))
+  if (!gdbarch_update_p (current_inferior (), info))
     internal_error (_("Could not remove target-supplied description"));
 }
 
-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] gdb: add inferior parameter to target_find_description
  2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
                   ` (2 preceding siblings ...)
  2022-11-24 16:04 ` [PATCH 3/5] gdb: add inferior parameter to gdbarch_update_p Simon Marchi
@ 2022-11-24 16:04 ` Simon Marchi
  2022-11-24 16:25   ` Simon Marchi
  2022-11-24 16:04 ` [PATCH 5/5] gdb: add inferior parameter to target_clear_description Simon Marchi
  4 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Make target_find_description not dependent on the current inferior on
entry.  Add an inferior parameter, and make it switch the current
inferior temporarily where needed.

Make callers pass the current inferior, no change in behavior is
expected.

Change-Id: I6fbe03a91e812c003b7946ba7ccc807a8b7369e6
---
 gdb/infcmd.c              |  2 +-
 gdb/infrun.c              |  2 +-
 gdb/remote.c              |  6 +++---
 gdb/target-descriptions.c | 31 ++++++++++++++++++-------------
 gdb/target-descriptions.h |  6 +++---
 gdb/tracefile-tfile.c     |  2 +-
 6 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f7bce0d0399..f281914ce54 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -251,7 +251,7 @@ post_create_inferior (int from_tty)
      Targets which need to access registers during to_open,
      to_create_inferior, or to_attach should do it earlier; but many
      don't need to.  */
-  target_find_description ();
+  target_find_description (current_inferior ());
 
   /* Now that we know the register layout, retrieve current PC.  But
      if the PC is unavailable (e.g., we're opening a core file with
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 96346e1f25b..c678d5accce 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1272,7 +1272,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
      architecture, and the old executable may e.g., be 32-bit, while
      the new one 64-bit), and before anything involving memory or
      registers.  */
-  target_find_description ();
+  target_find_description (inf);
 
   gdb::observers::inferior_execd.notify (inf);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 5118ecd0a31..ca6fd535a54 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4826,7 +4826,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 
   /* Next, if the target can specify a description, read it.  We do
      this before anything involving memory or registers.  */
-  target_find_description ();
+  target_find_description (current_inferior ());
 
   /* Next, now that we know something about the target, update the
      address spaces in the program spaces.  */
@@ -4963,7 +4963,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 	  && gdbarch_target_desc (target_gdbarch ()) == NULL)
 	{
 	  target_clear_description ();
-	  target_find_description ();
+	  target_find_description (current_inferior ());
 	}
 
       /* Use the previously fetched status.  */
@@ -6187,7 +6187,7 @@ extended_remote_target::attach (const char *args, int from_tty)
 
   /* Next, if the target can specify a description, read it.  We do
      this before anything involving memory or registers.  */
-  target_find_description ();
+  target_find_description (current_inferior ());
 
   if (!target_is_non_stop_p ())
     {
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 57d23747f26..40c04e0770f 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -520,13 +520,12 @@ target_desc_info_free (struct target_desc_info *tdesc_info)
 
 static std::string tdesc_filename_cmd_string;
 
-/* Fetch the current target's description, and switch the current
-   architecture to one which incorporates that description.  */
+/* See target-descriptions.h.  */
 
 void
-target_find_description (void)
+target_find_description (inferior *inf)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = get_tdesc_info (inf);
 
   /* If we've already fetched a description from the target, don't do
      it again.  This allows a target to fetch the description early,
@@ -538,7 +537,7 @@ target_find_description (void)
   /* The current architecture should not have any target description
      specified.  It should have been cleared, e.g. when we
      disconnected from the previous target.  */
-  gdb_assert (gdbarch_target_desc (target_gdbarch ()) == NULL);
+  gdb_assert (gdbarch_target_desc (inf->gdbarch) == nullptr);
 
   /* First try to fetch an XML description from the user-specified
      file.  */
@@ -546,16 +545,22 @@ target_find_description (void)
   if (!tdesc_info->filename.empty ())
     tdesc_info->tdesc = file_read_description_xml (tdesc_info->filename.data ());
 
+  /* The calls that get the target description from the target depend on INF
+     being the current inferior, and some targets need a specific thread to
+     be selected.  */
+  scoped_restore_current_thread restore_thread;
+  thread_info *thread = any_thread_of_inferior (inf);
+  gdb_assert (thread != nullptr);
+  switch_to_thread (thread);
+
   /* Next try to read the description from the current target using
      target objects.  */
   if (tdesc_info->tdesc == nullptr)
-    tdesc_info->tdesc = target_read_description_xml
-      (current_inferior ()->top_target ());
+    tdesc_info->tdesc = target_read_description_xml (inf->top_target ());
 
   /* If that failed try a target-specific hook.  */
   if (tdesc_info->tdesc == nullptr)
-    tdesc_info->tdesc = target_read_description
-      (current_inferior ()->top_target ());
+    tdesc_info->tdesc = target_read_description (inf->top_target ());
 
   /* If a non-NULL description was returned, then update the current
      architecture.  */
@@ -564,13 +569,13 @@ target_find_description (void)
       struct gdbarch_info info;
 
       info.target_desc = tdesc_info->tdesc;
-      if (!gdbarch_update_p (current_inferior (), info))
+      if (!gdbarch_update_p (inf, info))
 	warning (_("Architecture rejected target-supplied description"));
       else
 	{
 	  struct tdesc_arch_data *data;
 
-	  data = get_arch_data (target_gdbarch ());
+	  data = get_arch_data (inf->gdbarch);
 	  if (tdesc_has_registers (tdesc_info->tdesc)
 	      && data->arch_regs.empty ())
 	    warning (_("Target-supplied registers are not supported "
@@ -1289,7 +1294,7 @@ set_tdesc_filename_cmd (const char *args, int from_tty,
   tdesc_info->filename = tdesc_filename_cmd_string;
 
   target_clear_description ();
-  target_find_description ();
+  target_find_description (current_inferior ());
 }
 
 static void
@@ -1316,7 +1321,7 @@ unset_tdesc_filename_cmd (const char *args, int from_tty)
 
   tdesc_info->filename.clear ();
   target_clear_description ();
-  target_find_description ();
+  target_find_description (current_inferior ());
 }
 
 /* Print target description in C.  */
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 3049b783e2f..ab534488d65 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -31,10 +31,10 @@ struct target_ops;
 struct target_desc_info;
 struct inferior;
 
-/* Fetch the current inferior's description, and switch its current
-   architecture to one which incorporates that description.  */
+/* Fetch INFERIOR's target description, and switch its architecture to one which
+   incorporates that description.  */
 
-void target_find_description (void);
+void target_find_description (inferior *inf);
 
 /* Discard any description fetched from the target for the current
    inferior, and switch the current architecture to one with no target
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 3266f357a27..b8302902af4 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -538,7 +538,7 @@ tfile_target_open (const char *arg, int from_tty)
 	}
 
       /* By now, tdesc lines have been read from tfile - let's parse them.  */
-      target_find_description ();
+      target_find_description (current_inferior ());
 
       /* Record the starting offset of the binary trace data.  */
       trace_frames_offset = bytes;
-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] gdb: add inferior parameter to target_clear_description
  2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
                   ` (3 preceding siblings ...)
  2022-11-24 16:04 ` [PATCH 4/5] gdb: add inferior parameter to target_find_description Simon Marchi
@ 2022-11-24 16:04 ` Simon Marchi
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@polymtl.ca>

Make target_clear_description not dependent on the current inferior on
entry.  Add an inferior parameter, and make it switch the current
inferior temporarily where needed.

Mkae the callers pass the current inferior, no change in behavior is
expected.

Change-Id: I85e4c500fceee9fc037f209d188e608536ed3f13
---
 gdb/infrun.c              |  2 +-
 gdb/remote.c              |  2 +-
 gdb/target-descriptions.c | 13 ++++++-------
 gdb/target-descriptions.h |  2 +-
 gdb/target.c              |  2 +-
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c678d5accce..02ca137f545 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1252,7 +1252,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
 	 this on "follow-exec-mode new", as the old inferior stays
 	 around (its description is later cleared/refetched on
 	 restart).  */
-      target_clear_description ();
+      target_clear_description (inf);
       target_follow_exec (inf, ptid, exec_file_target);
     }
 
diff --git a/gdb/remote.c b/gdb/remote.c
index ca6fd535a54..cf3b2c92d36 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4962,7 +4962,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
       if (remote_read_description_p (this)
 	  && gdbarch_target_desc (target_gdbarch ()) == NULL)
 	{
-	  target_clear_description ();
+	  target_clear_description (current_inferior ());
 	  target_find_description (current_inferior ());
 	}
 
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 40c04e0770f..1abe5c8ead6 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -588,13 +588,12 @@ target_find_description (inferior *inf)
   tdesc_info->fetched = true;
 }
 
-/* Discard any description fetched from the current target, and switch
-   the current architecture to one with no target description.  */
+/* See target-descriptions.h.  */
 
 void
-target_clear_description (void)
+target_clear_description (inferior *inf)
 {
-  target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
+  target_desc_info *tdesc_info = get_tdesc_info (inf);
 
   if (!tdesc_info->fetched)
     return;
@@ -603,7 +602,7 @@ target_clear_description (void)
   tdesc_info->tdesc = nullptr;
 
   gdbarch_info info;
-  if (!gdbarch_update_p (current_inferior (), info))
+  if (!gdbarch_update_p (inf, info))
     internal_error (_("Could not remove target-supplied description"));
 }
 
@@ -1293,7 +1292,7 @@ set_tdesc_filename_cmd (const char *args, int from_tty,
 
   tdesc_info->filename = tdesc_filename_cmd_string;
 
-  target_clear_description ();
+  target_clear_description (current_inferior ());
   target_find_description (current_inferior ());
 }
 
@@ -1320,7 +1319,7 @@ unset_tdesc_filename_cmd (const char *args, int from_tty)
   target_desc_info *tdesc_info = get_tdesc_info (current_inferior ());
 
   tdesc_info->filename.clear ();
-  target_clear_description ();
+  target_clear_description (current_inferior ());
   target_find_description (current_inferior ());
 }
 
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index ab534488d65..5f89e933b6d 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -40,7 +40,7 @@ void target_find_description (inferior *inf);
    inferior, and switch the current architecture to one with no target
    description.  */
 
-void target_clear_description (void);
+void target_clear_description (inferior *inf);
 
 /* Return the current inferior's target description.  This should only
    be used by gdbarch initialization code; most access should be
diff --git a/gdb/target.c b/gdb/target.c
index 74925e139dc..bd3b6c98a57 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2484,7 +2484,7 @@ target_pre_inferior (int from_tty)
 
       invalidate_target_mem_regions ();
 
-      target_clear_description ();
+      target_clear_description (current_inferior ());
     }
 
   /* attach_flag may be set if the previous process associated with
-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/5] gdb: add inferior parameter to target_find_description
  2022-11-24 16:04 ` [PATCH 4/5] gdb: add inferior parameter to target_find_description Simon Marchi
@ 2022-11-24 16:25   ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:25 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

> @@ -546,16 +545,22 @@ target_find_description (void)
>    if (!tdesc_info->filename.empty ())
>      tdesc_info->tdesc = file_read_description_xml (tdesc_info->filename.data ());
>  
> +  /* The calls that get the target description from the target depend on INF
> +     being the current inferior, and some targets need a specific thread to
> +     be selected.  */
> +  scoped_restore_current_thread restore_thread;
> +  thread_info *thread = any_thread_of_inferior (inf);
> +  gdb_assert (thread != nullptr);
> +  switch_to_thread (thread);
Sorry, looks like I messed up my testing, this doesn't work when using
the "set tdesc filename" command while there are no threads:


 37 (gdb) set tdesc filename /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/arc-tdesc-cpu/trivial.xml^M
 38 /home/smarchi/src/binutils-gdb/gdb/thread.c:631: internal-error: any_thread_of_inferior: Assertion `inf->pid != 0' failed.^M
 39 A problem internal to GDB has been detected,^M
 40 further debugging may prove unreliable.^M
 41 ----- Backtrace -----^M
 42 FAIL: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.arch/arc-tdesc-cpu/trivial.xml (GDB internal error)

I made it this way because the remote target uses inferior_ptid to set
the general thread in remote_target::xfer_partial, regardless of the
object.  For TARGET_OBJECT_AVAILABLE_FEATURES, since target descriptions
are a per-inferior thing (for now), it could look at the current
inferior instead, it doesn't really need a current thread.  This is part
of my larger work that I picked these patches from, so I will have to
re-check the order of the changes.

Simon


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch
  2022-11-24 16:04 ` [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch Simon Marchi
@ 2022-11-24 16:42   ` Lancelot SIX
  2022-11-24 16:47     ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Lancelot SIX @ 2022-11-24 16:42 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Simon Marchi

On Thu, Nov 24, 2022 at 11:04:25AM -0500, Simon Marchi via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> Add an inferior parameter, so it doesn't depend on the current
> inferior.  While at it, rename to set_inferior_gdbarch, I think that's a
> better name than "target".
> 
> The sole observer of the architecture_changed observable,
> pyuw_on_new_gdbarch, doesn't seem to depend on the current inferior.
> Neither does registers_changed.

Hi,

I kind of feel that it would make sense to also change the type of the
architecture_changed observable so the observers receive the inferior *
and the gdbarch *.  It will not be used at the moment, but makes it
clearer that this observable gets triggerd for a given inferior.

WDYT?

Best,
Lancelot.

> 
> Update callers to pass the current inferior, no change in behavior is
> expected.
> 
> Change-Id: I276e28eafd4740c94bc5233c81a86c01b4a6ae90
> ---
>  gdb/arch-utils.c | 16 ++++++++--------
>  gdb/gdbarch.h    |  4 ++--
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index dc67c632155..92caa5c3c4a 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -617,7 +617,7 @@ gdbarch_update_p (struct gdbarch_info info)
>  		"New architecture %s (%s) selected\n",
>  		host_address_to_string (new_gdbarch),
>  		gdbarch_bfd_arch_info (new_gdbarch)->printable_name);
> -  set_target_gdbarch (new_gdbarch);
> +  set_inferior_gdbarch (current_inferior (), new_gdbarch);
>  
>    return 1;
>  }
> @@ -649,7 +649,7 @@ set_gdbarch_from_file (bfd *abfd)
>  
>    if (gdbarch == NULL)
>      error (_("Architecture of file not recognized."));
> -  set_target_gdbarch (gdbarch);
> +  set_inferior_gdbarch (current_inferior (), gdbarch);
>  }
>  
>  /* Initialize the current architecture.  Update the ``set
> @@ -1427,15 +1427,15 @@ gdbarch_find_by_info (struct gdbarch_info info)
>    return new_gdbarch;
>  }
>  
> -/* Make the specified architecture current.  */
> +/* See gdbarch.h.  */
>  
>  void
> -set_target_gdbarch (struct gdbarch *new_gdbarch)
> +set_inferior_gdbarch (inferior *inf, gdbarch *gdbarch)
>  {
> -  gdb_assert (new_gdbarch != NULL);
> -  gdb_assert (new_gdbarch->initialized_p);
> -  current_inferior ()->gdbarch = new_gdbarch;
> -  gdb::observers::architecture_changed.notify (new_gdbarch);
> +  gdb_assert (gdbarch != nullptr);
> +  gdb_assert (gdbarch->initialized_p);
> +  inf->gdbarch = gdbarch;
> +  gdb::observers::architecture_changed.notify (gdbarch);
>    registers_changed ();
>  }
>  
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index f2ba5f97765..76ffddfe0ff 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -342,9 +342,9 @@ extern int gdbarch_update_p (struct gdbarch_info info);
>  extern struct gdbarch *gdbarch_find_by_info (struct gdbarch_info info);
>  
>  
> -/* Helper function.  Set the target gdbarch to "gdbarch".  */
> +/* Set INF's the target gdbarch to "gdbarch".  */
>  
> -extern void set_target_gdbarch (struct gdbarch *gdbarch);
> +extern void set_inferior_gdbarch (inferior *inf, gdbarch *gdbarch);
>  
>  
>  /* A registry adaptor for gdbarch.  This arranges to store the
> -- 
> 2.37.3
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch
  2022-11-24 16:42   ` Lancelot SIX
@ 2022-11-24 16:47     ` Simon Marchi
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2022-11-24 16:47 UTC (permalink / raw)
  To: Lancelot SIX, Simon Marchi; +Cc: gdb-patches

On 11/24/22 11:42, Lancelot SIX wrote:
> On Thu, Nov 24, 2022 at 11:04:25AM -0500, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>>
>> Add an inferior parameter, so it doesn't depend on the current
>> inferior.  While at it, rename to set_inferior_gdbarch, I think that's a
>> better name than "target".
>>
>> The sole observer of the architecture_changed observable,
>> pyuw_on_new_gdbarch, doesn't seem to depend on the current inferior.
>> Neither does registers_changed.
> 
> Hi,
> 
> I kind of feel that it would make sense to also change the type of the
> architecture_changed observable so the observers receive the inferior *
> and the gdbarch *.  It will not be used at the moment, but makes it
> clearer that this observable gets triggerd for a given inferior.
> 
> WDYT?

That's a good idea, I'll add a patch that does that to my next version.

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-11-24 16:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24 16:04 [PATCH 0/5] Make some functions independent of current inferior Simon Marchi
2022-11-24 16:04 ` [PATCH 1/5] gdb: add inferior parameter to target_current_description Simon Marchi
2022-11-24 16:04 ` [PATCH 2/5] gdb: add inferior parameter to set_target_gdbarch, rename to set_inferior_gdbarch Simon Marchi
2022-11-24 16:42   ` Lancelot SIX
2022-11-24 16:47     ` Simon Marchi
2022-11-24 16:04 ` [PATCH 3/5] gdb: add inferior parameter to gdbarch_update_p Simon Marchi
2022-11-24 16:04 ` [PATCH 4/5] gdb: add inferior parameter to target_find_description Simon Marchi
2022-11-24 16:25   ` Simon Marchi
2022-11-24 16:04 ` [PATCH 5/5] gdb: add inferior parameter to target_clear_description Simon Marchi

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).