[PATCH 1/2] gdb: remove gdbarch_info::tdep_info

[PATCH 1/2] gdb: remove gdbarch_info::tdep_info

[Simon Marchi]

This field is not actually used, remove it.

gdb/ChangeLog:

	* gdbarch.sh (struct gdbarch_info) <tdep_info>: Remove.
	(gdbarch_find_by_info): Remove print.

Change-Id: I00af4681b8e1a27727441cbadc3827f5914bd8eb
---
 gdb/gdbarch.c  | 3 ---
 gdb/gdbarch.h  | 4 ----
 gdb/gdbarch.sh | 7 -------
 3 files changed, 14 deletions(-)

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 208cf4b5aaab..830a86df89f3 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -5695,9 +5695,6 @@ gdbarch_find_by_info (struct gdbarch_info info)
       fprintf_unfiltered (gdb_stdlog,
 			  "gdbarch_find_by_info: info.abfd %s\n",
 			  host_address_to_string (info.abfd));
-      fprintf_unfiltered (gdb_stdlog,
-			  "gdbarch_find_by_info: info.tdep_info %s\n",
-			  host_address_to_string (info.tdep_info));
     }
 
   /* Find the tdep code that knows about this architecture.  */
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 7157e5596fd3..36ea4de17c8c 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1795,10 +1795,6 @@ struct gdbarch_info
   /* Use default: NULL (ZERO).  */
   union
     {
-      /* Architecture-specific information.  The generic form for targets
-	 that have extra requirements.  */
-      struct gdbarch_tdep_info *tdep_info;
-
       /* Architecture-specific target description data.  Numerous targets
 	 need only this, so give them an easy way to hold it.  */
       struct tdesc_arch_data *tdesc_data;
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 43e51341f97b..01a5ac88219d 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1547,10 +1547,6 @@ struct gdbarch_info
   /* Use default: NULL (ZERO).  */
   union
     {
-      /* Architecture-specific information.  The generic form for targets
-	 that have extra requirements.  */
-      struct gdbarch_tdep_info *tdep_info;
-
       /* Architecture-specific target description data.  Numerous targets
 	 need only this, so give them an easy way to hold it.  */
       struct tdesc_arch_data *tdesc_data;
@@ -2479,9 +2475,6 @@ gdbarch_find_by_info (struct gdbarch_info info)
       fprintf_unfiltered (gdb_stdlog,
 			  "gdbarch_find_by_info: info.abfd %s\n",
 			  host_address_to_string (info.abfd));
-      fprintf_unfiltered (gdb_stdlog,
-			  "gdbarch_find_by_info: info.tdep_info %s\n",
-			  host_address_to_string (info.tdep_info));
     }
 
   /* Find the tdep code that knows about this architecture.  */
-- 
2.32.0

[PATCH 2/2] gdb: remove gdbarch_info_init

[Simon Marchi]

While reviewing another patch, I realized that gdbarch_info_init could
easily be removed in favor of initializing gdbarch_info fields directly
in the struct declaration.  The only odd part is the union.  I don't
know if it's actually important for it to be zero-initialized, but I
presume it is.  I added a constructor to gdbarch_info to take care of
that.  A proper solution would be to use std::variant.  Or, these could
also be separate fields, the little extra space required wouldn't
matter.

gdb/ChangeLog:

	* gdbarch.sh (struct gdbarch_info): Initialize fields, add
	constructor.
	* gdbarch.h: Re-generate.
	* arch-utils.h (gdbarch_info_init): Remove, delete all usages.
	* arch-utils.c (gdbarch_info_init): Remove.

Change-Id: I7502e08fe0f278d84eef1667a072e8a97bda5ab5
---
 gdb/aarch64-linux-nat.c   |  1 -
 gdb/aarch64-tdep.c        |  2 --
 gdb/arch-utils.c          | 26 +-------------------------
 gdb/arch-utils.h          |  6 ------
 gdb/arm-tdep.c            |  7 +------
 gdb/cris-tdep.c           |  3 ---
 gdb/gdbarch.h             | 28 ++++++++++++++--------------
 gdb/gdbarch.sh            | 28 ++++++++++++++--------------
 gdb/i386-darwin-nat.c     |  3 +--
 gdb/inferior.c            |  3 +--
 gdb/mips-tdep.c           |  5 -----
 gdb/osabi.c               |  4 +---
 gdb/rs6000-nat.c          |  3 +--
 gdb/rs6000-tdep.c         |  4 +---
 gdb/selftest-arch.c       |  1 -
 gdb/target-descriptions.c |  2 --
 16 files changed, 35 insertions(+), 91 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 61224022f6aa..c7cbebbc3517 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -1060,7 +1060,6 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
      new one), stashing the vector length inside id.  Use -1 for when SVE
      unavailable, to distinguish from an unset value of 0.  */
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);
   info.id = (int *) (vq == 0 ? -1 : vq);
   return gdbarch_find_by_info (info);
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 202b02108744..dea16e729c03 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -627,7 +627,6 @@ aarch64_analyze_prologue_test (void)
 {
   struct gdbarch_info info;
 
-  gdbarch_info_init (&info);
   info.bfd_arch_info = bfd_scan_arch ("aarch64");
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
@@ -4559,7 +4558,6 @@ aarch64_process_record_test (void)
   struct gdbarch_info info;
   uint32_t ret;
 
-  gdbarch_info_init (&info);
   info.bfd_arch_info = bfd_scan_arch ("aarch64");
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index c75a79757ea2..4290d637ce16 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -389,8 +389,6 @@ set_endian (const char *ignore_args, int from_tty, struct cmd_list_element *c)
 {
   struct gdbarch_info info;
 
-  gdbarch_info_init (&info);
-
   if (set_endian_string == endian_auto)
     {
       target_byte_order_user = BFD_ENDIAN_UNKNOWN;
@@ -548,8 +546,6 @@ set_architecture (const char *ignore_args,
 {
   struct gdbarch_info info;
 
-  gdbarch_info_init (&info);
-
   if (strcmp (set_architecture_string, "auto") == 0)
     {
       target_architecture_user = NULL;
@@ -630,7 +626,6 @@ struct gdbarch *
 gdbarch_from_bfd (bfd *abfd)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
 
   info.abfd = abfd;
   return gdbarch_find_by_info (info);
@@ -645,7 +640,6 @@ set_gdbarch_from_file (bfd *abfd)
   struct gdbarch_info info;
   struct gdbarch *gdbarch;
 
-  gdbarch_info_init (&info);
   info.abfd = abfd;
   info.target_desc = target_current_description ();
   gdbarch = gdbarch_find_by_info (info);
@@ -679,10 +673,6 @@ void
 initialize_current_architecture (void)
 {
   const char **arches = gdbarch_printable_names ();
-  struct gdbarch_info info;
-
-  /* determine a default architecture and byte order.  */
-  gdbarch_info_init (&info);
   
   /* Find a default architecture.  */
   if (default_bfd_arch == NULL)
@@ -705,6 +695,7 @@ initialize_current_architecture (void)
 			_("initialize_current_architecture: Arch not found"));
     }
 
+  gdbarch_info info;
   info.bfd_arch_info = default_bfd_arch;
 
   /* Take several guesses at a byte order.  */
@@ -769,21 +760,6 @@ initialize_current_architecture (void)
   }
 }
 
-
-/* Initialize a gdbarch info to values that will be automatically
-   overridden.  Note: Originally, this ``struct info'' was initialized
-   using memset(0).  Unfortunately, that ran into problems, namely
-   BFD_ENDIAN_BIG is zero.  An explicit initialization function that
-   can explicitly set each field to a well defined value is used.  */
-
-void
-gdbarch_info_init (struct gdbarch_info *info)
-{
-  memset (info, 0, sizeof (struct gdbarch_info));
-  info->byte_order = BFD_ENDIAN_UNKNOWN;
-  info->byte_order_for_code = info->byte_order;
-}
-
 /* Similar to init, but this time fill in the blanks.  Information is
    obtained from the global "set ..." options and explicitly
    initialized INFO fields.  */
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index a5b40ad8ff1b..03e9082f6d77 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -196,12 +196,6 @@ extern enum bfd_endian selected_byte_order (void);
    was explicitly selected.  */
 extern const char *selected_architecture_name (void);
 
-/* Initialize a ``struct info''.  Can't use memset(0) since some
-   default values are not zero.  "fill" takes all available
-   information and fills in any unspecified fields.  */
-
-extern void gdbarch_info_init (struct gdbarch_info *info);
-
 /* Similar to init, but this time fill in the blanks.  Information is
    obtained from the global "set ..." options and explicitly
    initialized INFO fields.  */
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 857a52a9a513..339e03271e7e 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8403,15 +8403,12 @@ arm_skip_stub (struct frame_info *frame, CORE_ADDR pc)
 static void
 arm_update_current_architecture (void)
 {
-  struct gdbarch_info info;
-
   /* If the current architecture is not ARM, we have nothing to do.  */
   if (gdbarch_bfd_arch_info (target_gdbarch ())->arch != bfd_arch_arm)
     return;
 
   /* Update the architecture.  */
-  gdbarch_info_init (&info);
-
+  gdbarch_info info;
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, _("could not update architecture"));
 }
@@ -13237,7 +13234,6 @@ static void
 arm_record_test (void)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   info.bfd_arch_info = bfd_scan_arch ("arm");
 
   struct gdbarch *gdbarch = gdbarch_find_by_info (info);
@@ -13329,7 +13325,6 @@ arm_analyze_prologue_test ()
   for (bfd_endian endianness : {BFD_ENDIAN_LITTLE, BFD_ENDIAN_BIG})
     {
       struct gdbarch_info info;
-      gdbarch_info_init (&info);
       info.byte_order = endianness;
       info.byte_order_for_code = endianness;
       info.bfd_arch_info = bfd_scan_arch ("arm");
diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index e1c141865cc6..01ef1604bffa 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -3879,7 +3879,6 @@ set_cris_version (const char *ignore_args, int from_tty,
   usr_cmd_cris_version_valid = 1;
   
   /* Update the current architecture, if needed.  */
-  gdbarch_info_init (&info);
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, 
 		    _("cris_gdbarch_update: failed to update architecture."));
@@ -3892,7 +3891,6 @@ set_cris_mode (const char *ignore_args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the current architecture, if needed.  */
-  gdbarch_info_init (&info);
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, 
 		    "cris_gdbarch_update: failed to update architecture.");
@@ -3905,7 +3903,6 @@ set_cris_dwarf2_cfi (const char *ignore_args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the current architecture, if needed.  */
-  gdbarch_info_init (&info);
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, 
 		    _("cris_gdbarch_update: failed to update architecture."));
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 36ea4de17c8c..f94695adaa08 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1781,18 +1781,20 @@ struct gdbarch_list
 
 struct gdbarch_info
 {
-  /* Use default: NULL (ZERO).  */
-  const struct bfd_arch_info *bfd_arch_info;
+  gdbarch_info ()
+    /* Ensure the union is zero-initialized.  Relies on the fact that there's
+       no member larget than TDESC_DATA.  */
+    : tdesc_data ()
+  {}
 
-  /* Use default: BFD_ENDIAN_UNKNOWN (NB: is not ZERO).  */
-  enum bfd_endian byte_order;
+  const struct bfd_arch_info *bfd_arch_info = nullptr;
 
-  enum bfd_endian byte_order_for_code;
+  enum bfd_endian byte_order = BFD_ENDIAN_UNKNOWN;
 
-  /* Use default: NULL (ZERO).  */
-  bfd *abfd;
+  enum bfd_endian byte_order_for_code = BFD_ENDIAN_UNKNOWN;
+
+  bfd *abfd = nullptr;
 
-  /* Use default: NULL (ZERO).  */
   union
     {
       /* Architecture-specific target description data.  Numerous targets
@@ -1805,11 +1807,9 @@ struct gdbarch_info
       int *id;
     };
 
-  /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
-  enum gdb_osabi osabi;
+  enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
 
-  /* Use default: NULL (ZERO).  */
-  const struct target_desc *target_desc;
+  const struct target_desc *target_desc = nullptr;
 };
 
 typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
@@ -1883,8 +1883,8 @@ extern int gdbarch_update_p (struct gdbarch_info info);
 
 /* Helper function.  Find an architecture matching info.
 
-   INFO should be initialized using gdbarch_info_init, relevant fields
-   set, and then finished using gdbarch_info_fill.
+   INFO should have relevant fields set, and then finished using
+   gdbarch_info_fill.
 
    Returns the corresponding architecture, or NULL if no matching
    architecture was found.  */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 01a5ac88219d..e82d711ff2c3 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1533,18 +1533,20 @@ struct gdbarch_list
 
 struct gdbarch_info
 {
-  /* Use default: NULL (ZERO).  */
-  const struct bfd_arch_info *bfd_arch_info;
+  gdbarch_info ()
+    /* Ensure the union is zero-initialized.  Relies on the fact that there's
+       no member larget than TDESC_DATA.  */
+    : tdesc_data ()
+  {}
+
+  const struct bfd_arch_info *bfd_arch_info = nullptr;
 
-  /* Use default: BFD_ENDIAN_UNKNOWN (NB: is not ZERO).  */
-  enum bfd_endian byte_order;
+  enum bfd_endian byte_order = BFD_ENDIAN_UNKNOWN;
 
-  enum bfd_endian byte_order_for_code;
+  enum bfd_endian byte_order_for_code = BFD_ENDIAN_UNKNOWN;
 
-  /* Use default: NULL (ZERO).  */
-  bfd *abfd;
+  bfd *abfd = nullptr;
 
-  /* Use default: NULL (ZERO).  */
   union
     {
       /* Architecture-specific target description data.  Numerous targets
@@ -1557,11 +1559,9 @@ struct gdbarch_info
       int *id;
     };
 
-  /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
-  enum gdb_osabi osabi;
+  enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
 
-  /* Use default: NULL (ZERO).  */
-  const struct target_desc *target_desc;
+  const struct target_desc *target_desc = nullptr;
 };
 
 typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
@@ -1637,8 +1637,8 @@ extern int gdbarch_update_p (struct gdbarch_info info);
 
 /* Helper function.  Find an architecture matching info.
 
-   INFO should be initialized using gdbarch_info_init, relevant fields
-   set, and then finished using gdbarch_info_fill.
+   INFO should have relevant fields set, and then finished using
+   gdbarch_info_fill.
 
    Returns the corresponding architecture, or NULL if no matching
    architecture was found.  */
diff --git a/gdb/i386-darwin-nat.c b/gdb/i386-darwin-nat.c
index 922d7a6df0ca..c1d6139490d8 100644
--- a/gdb/i386-darwin-nat.c
+++ b/gdb/i386-darwin-nat.c
@@ -479,7 +479,6 @@ darwin_check_osabi (darwin_inferior *inf, thread_t thread)
     {
       /* Attaching to a process.  Let's figure out what kind it is.  */
       x86_thread_state_t gp_regs;
-      struct gdbarch_info info;
       unsigned int gp_count = x86_THREAD_STATE_COUNT;
       kern_return_t ret;
 
@@ -491,7 +490,7 @@ darwin_check_osabi (darwin_inferior *inf, thread_t thread)
 	  return;
 	}
 
-      gdbarch_info_init (&info);
+      gdbarch_info info;
       gdbarch_info_fill (&info);
       info.byte_order = gdbarch_byte_order (target_gdbarch ());
       info.osabi = GDB_OSABI_DARWIN;
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 059839ec9626..e1c70d553fde 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -747,7 +747,6 @@ add_inferior_with_spaces (void)
   struct address_space *aspace;
   struct program_space *pspace;
   struct inferior *inf;
-  struct gdbarch_info info;
 
   /* If all inferiors share an address space on this system, this
      doesn't really return a new address space; otherwise, it
@@ -760,7 +759,7 @@ add_inferior_with_spaces (void)
 
   /* Setup the inferior's initial arch, based on information obtained
      from the global "set ..." options.  */
-  gdbarch_info_init (&info);
+  gdbarch_info info;
   inf->gdbarch = gdbarch_find_by_info (info);
   /* The "set ..." options reject invalid settings, so we should
      always have a valid arch by now.  */
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index dbace1212508..2aee4f10b316 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -865,7 +865,6 @@ set_mips64_transfers_32bit_regs (const char *args, int from_tty,
 				 struct cmd_list_element *c)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   /* 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.  */
@@ -6918,7 +6917,6 @@ static void
 set_mipsfpu_single_command (const char *args, int from_tty)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   mips_fpu_type = MIPS_FPU_SINGLE;
   mips_fpu_type_auto = 0;
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
@@ -6932,7 +6930,6 @@ static void
 set_mipsfpu_double_command (const char *args, int from_tty)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   mips_fpu_type = MIPS_FPU_DOUBLE;
   mips_fpu_type_auto = 0;
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
@@ -6946,7 +6943,6 @@ static void
 set_mipsfpu_none_command (const char *args, int from_tty)
 {
   struct gdbarch_info info;
-  gdbarch_info_init (&info);
   mips_fpu_type = MIPS_FPU_NONE;
   mips_fpu_type_auto = 0;
   /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
@@ -8790,7 +8786,6 @@ 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_info_init (&info);
   gdbarch_update_p (info);
 }
 
diff --git a/gdb/osabi.c b/gdb/osabi.c
index b1603c09c855..aabf895c0451 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -598,8 +598,6 @@ generic_elf_osabi_sniffer (bfd *abfd)
 static void
 set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
 {
-  struct gdbarch_info info;
-
   if (strcmp (set_osabi_string, "auto") == 0)
     user_osabi_state = osabi_auto;
   else if (strcmp (set_osabi_string, "default") == 0)
@@ -630,7 +628,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_init (&info);
+  gdbarch_info info;
   if (! gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, _("Updating OS ABI failed."));
 }
diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
index 50c96e281c64..ee00a600343a 100644
--- a/gdb/rs6000-nat.c
+++ b/gdb/rs6000-nat.c
@@ -558,7 +558,6 @@ rs6000_nat_target::create_inferior (const char *exec_file,
   enum bfd_architecture arch;
   unsigned long mach;
   bfd abfd;
-  struct gdbarch_info info;
 
   inf_ptrace_target::create_inferior (exec_file, allargs, env, from_tty);
 
@@ -593,7 +592,7 @@ rs6000_nat_target::create_inferior (const char *exec_file,
 
   bfd_default_set_arch_mach (&abfd, arch, mach);
 
-  gdbarch_info_init (&info);
+  gdbarch_info info;
   info.bfd_arch_info = bfd_get_arch_info (&abfd);
   info.abfd = current_program_space->exec_bfd ();
 
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 5bcf5fca2c3b..a629450d746f 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -7260,7 +7260,6 @@ powerpc_set_soft_float (const char *args, int from_tty,
   struct gdbarch_info info;
 
   /* Update the architecture.  */
-  gdbarch_info_init (&info);
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, _("could not update architecture"));
 }
@@ -7269,7 +7268,6 @@ static void
 powerpc_set_vector_abi (const char *args, int from_tty,
 			struct cmd_list_element *c)
 {
-  struct gdbarch_info info;
   int vector_abi;
 
   for (vector_abi = POWERPC_VEC_AUTO;
@@ -7287,7 +7285,7 @@ powerpc_set_vector_abi (const char *args, int from_tty,
 		    powerpc_vector_abi_string);
 
   /* Update the architecture.  */
-  gdbarch_info_init (&info);
+  gdbarch_info info;
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__, _("could not update architecture"));
 }
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index ab1215c848c3..0eef134d4841 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -64,7 +64,6 @@ struct gdbarch_selftest : public selftest
 	  {
 	    struct gdbarch_info info;
 
-	    gdbarch_info_init (&info);
 	    info.bfd_arch_info = bfd_scan_arch (arches[i]);
 
 	    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index f7d0a688c62f..f94ace756d4d 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -555,7 +555,6 @@ target_find_description (void)
     {
       struct gdbarch_info info;
 
-      gdbarch_info_init (&info);
       info.target_desc = tdesc_info->tdesc;
       if (!gdbarch_update_p (info))
 	warning (_("Architecture rejected target-supplied description"));
@@ -592,7 +591,6 @@ target_clear_description (void)
   tdesc_info->tdesc = nullptr;
 
   gdbarch_info info;
-  gdbarch_info_init (&info);
   if (!gdbarch_update_p (info))
     internal_error (__FILE__, __LINE__,
 		    _("Could not remove target-supplied description"));
-- 
2.32.0

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Luis Machado]

On 6/23/21 4:27 PM, Simon Marchi via Gdb-patches wrote:
> While reviewing another patch, I realized that gdbarch_info_init could
> easily be removed in favor of initializing gdbarch_info fields directly
> in the struct declaration.  The only odd part is the union.  I don't
> know if it's actually important for it to be zero-initialized, but I
> presume it is.  I added a constructor to gdbarch_info to take care of
> that.  A proper solution would be to use std::variant.  Or, these could
> also be separate fields, the little extra space required wouldn't
> matter.
> 
> gdb/ChangeLog:
> 
> 	* gdbarch.sh (struct gdbarch_info): Initialize fields, add
> 	constructor.
> 	* gdbarch.h: Re-generate.
> 	* arch-utils.h (gdbarch_info_init): Remove, delete all usages.
> 	* arch-utils.c (gdbarch_info_init): Remove.
> 
> Change-Id: I7502e08fe0f278d84eef1667a072e8a97bda5ab5
> ---
>   gdb/aarch64-linux-nat.c   |  1 -
>   gdb/aarch64-tdep.c        |  2 --
>   gdb/arch-utils.c          | 26 +-------------------------
>   gdb/arch-utils.h          |  6 ------
>   gdb/arm-tdep.c            |  7 +------
>   gdb/cris-tdep.c           |  3 ---
>   gdb/gdbarch.h             | 28 ++++++++++++++--------------
>   gdb/gdbarch.sh            | 28 ++++++++++++++--------------
>   gdb/i386-darwin-nat.c     |  3 +--
>   gdb/inferior.c            |  3 +--
>   gdb/mips-tdep.c           |  5 -----
>   gdb/osabi.c               |  4 +---
>   gdb/rs6000-nat.c          |  3 +--
>   gdb/rs6000-tdep.c         |  4 +---
>   gdb/selftest-arch.c       |  1 -
>   gdb/target-descriptions.c |  2 --
>   16 files changed, 35 insertions(+), 91 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 61224022f6aa..c7cbebbc3517 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -1060,7 +1060,6 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
>        new one), stashing the vector length inside id.  Use -1 for when SVE
>        unavailable, to distinguish from an unset value of 0.  */
>     struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>     info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);
>     info.id = (int *) (vq == 0 ? -1 : vq);
>     return gdbarch_find_by_info (info);
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 202b02108744..dea16e729c03 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -627,7 +627,6 @@ aarch64_analyze_prologue_test (void)
>   {
>     struct gdbarch_info info;
>   
> -  gdbarch_info_init (&info);
>     info.bfd_arch_info = bfd_scan_arch ("aarch64");
>   
>     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> @@ -4559,7 +4558,6 @@ aarch64_process_record_test (void)
>     struct gdbarch_info info;
>     uint32_t ret;
>   
> -  gdbarch_info_init (&info);
>     info.bfd_arch_info = bfd_scan_arch ("aarch64");
>   
>     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index c75a79757ea2..4290d637ce16 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -389,8 +389,6 @@ set_endian (const char *ignore_args, int from_tty, struct cmd_list_element *c)
>   {
>     struct gdbarch_info info;
>   
> -  gdbarch_info_init (&info);
> -
>     if (set_endian_string == endian_auto)
>       {
>         target_byte_order_user = BFD_ENDIAN_UNKNOWN;
> @@ -548,8 +546,6 @@ set_architecture (const char *ignore_args,
>   {
>     struct gdbarch_info info;
>   
> -  gdbarch_info_init (&info);
> -
>     if (strcmp (set_architecture_string, "auto") == 0)
>       {
>         target_architecture_user = NULL;
> @@ -630,7 +626,6 @@ struct gdbarch *
>   gdbarch_from_bfd (bfd *abfd)
>   {
>     struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>   
>     info.abfd = abfd;
>     return gdbarch_find_by_info (info);
> @@ -645,7 +640,6 @@ set_gdbarch_from_file (bfd *abfd)
>     struct gdbarch_info info;
>     struct gdbarch *gdbarch;
>   
> -  gdbarch_info_init (&info);
>     info.abfd = abfd;
>     info.target_desc = target_current_description ();
>     gdbarch = gdbarch_find_by_info (info);
> @@ -679,10 +673,6 @@ void
>   initialize_current_architecture (void)
>   {
>     const char **arches = gdbarch_printable_names ();
> -  struct gdbarch_info info;
> -
> -  /* determine a default architecture and byte order.  */
> -  gdbarch_info_init (&info);
>     
>     /* Find a default architecture.  */
>     if (default_bfd_arch == NULL)
> @@ -705,6 +695,7 @@ initialize_current_architecture (void)
>   			_("initialize_current_architecture: Arch not found"));
>       }
>   
> +  gdbarch_info info;
>     info.bfd_arch_info = default_bfd_arch;
>   
>     /* Take several guesses at a byte order.  */
> @@ -769,21 +760,6 @@ initialize_current_architecture (void)
>     }
>   }
>   
> -
> -/* Initialize a gdbarch info to values that will be automatically
> -   overridden.  Note: Originally, this ``struct info'' was initialized
> -   using memset(0).  Unfortunately, that ran into problems, namely
> -   BFD_ENDIAN_BIG is zero.  An explicit initialization function that
> -   can explicitly set each field to a well defined value is used.  */
> -
> -void
> -gdbarch_info_init (struct gdbarch_info *info)
> -{
> -  memset (info, 0, sizeof (struct gdbarch_info));
> -  info->byte_order = BFD_ENDIAN_UNKNOWN;
> -  info->byte_order_for_code = info->byte_order;
> -}
> -
>   /* Similar to init, but this time fill in the blanks.  Information is
>      obtained from the global "set ..." options and explicitly
>      initialized INFO fields.  */
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index a5b40ad8ff1b..03e9082f6d77 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -196,12 +196,6 @@ extern enum bfd_endian selected_byte_order (void);
>      was explicitly selected.  */
>   extern const char *selected_architecture_name (void);
>   
> -/* Initialize a ``struct info''.  Can't use memset(0) since some
> -   default values are not zero.  "fill" takes all available
> -   information and fills in any unspecified fields.  */
> -
> -extern void gdbarch_info_init (struct gdbarch_info *info);
> -
>   /* Similar to init, but this time fill in the blanks.  Information is
>      obtained from the global "set ..." options and explicitly
>      initialized INFO fields.  */
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 857a52a9a513..339e03271e7e 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -8403,15 +8403,12 @@ arm_skip_stub (struct frame_info *frame, CORE_ADDR pc)
>   static void
>   arm_update_current_architecture (void)
>   {
> -  struct gdbarch_info info;
> -
>     /* If the current architecture is not ARM, we have nothing to do.  */
>     if (gdbarch_bfd_arch_info (target_gdbarch ())->arch != bfd_arch_arm)
>       return;
>   
>     /* Update the architecture.  */
> -  gdbarch_info_init (&info);
> -
> +  gdbarch_info info;
>     if (!gdbarch_update_p (info))
>       internal_error (__FILE__, __LINE__, _("could not update architecture"));
>   }
> @@ -13237,7 +13234,6 @@ static void
>   arm_record_test (void)
>   {
>     struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>     info.bfd_arch_info = bfd_scan_arch ("arm");
>   
>     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> @@ -13329,7 +13325,6 @@ arm_analyze_prologue_test ()
>     for (bfd_endian endianness : {BFD_ENDIAN_LITTLE, BFD_ENDIAN_BIG})
>       {
>         struct gdbarch_info info;
> -      gdbarch_info_init (&info);
>         info.byte_order = endianness;
>         info.byte_order_for_code = endianness;
>         info.bfd_arch_info = bfd_scan_arch ("arm");
> diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
> index e1c141865cc6..01ef1604bffa 100644
> --- a/gdb/cris-tdep.c
> +++ b/gdb/cris-tdep.c
> @@ -3879,7 +3879,6 @@ set_cris_version (const char *ignore_args, int from_tty,
>     usr_cmd_cris_version_valid = 1;
>     
>     /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>     if (!gdbarch_update_p (info))
>       internal_error (__FILE__, __LINE__,
>   		    _("cris_gdbarch_update: failed to update architecture."));
> @@ -3892,7 +3891,6 @@ set_cris_mode (const char *ignore_args, int from_tty,
>     struct gdbarch_info info;
>   
>     /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>     if (!gdbarch_update_p (info))
>       internal_error (__FILE__, __LINE__,
>   		    "cris_gdbarch_update: failed to update architecture.");
> @@ -3905,7 +3903,6 @@ set_cris_dwarf2_cfi (const char *ignore_args, int from_tty,
>     struct gdbarch_info info;
>   
>     /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>     if (!gdbarch_update_p (info))
>       internal_error (__FILE__, __LINE__,
>   		    _("cris_gdbarch_update: failed to update architecture."));
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 36ea4de17c8c..f94695adaa08 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1781,18 +1781,20 @@ struct gdbarch_list
>   
>   struct gdbarch_info
>   {
> -  /* Use default: NULL (ZERO).  */
> -  const struct bfd_arch_info *bfd_arch_info;
> +  gdbarch_info ()
> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
> +       no member larget than TDESC_DATA.  */
> +    : tdesc_data ()
> +  {}
>   
> -  /* Use default: BFD_ENDIAN_UNKNOWN (NB: is not ZERO).  */
> -  enum bfd_endian byte_order;
> +  const struct bfd_arch_info *bfd_arch_info = nullptr;
>   
> -  enum bfd_endian byte_order_for_code;
> +  enum bfd_endian byte_order = BFD_ENDIAN_UNKNOWN;
>   
> -  /* Use default: NULL (ZERO).  */
> -  bfd *abfd;
> +  enum bfd_endian byte_order_for_code = BFD_ENDIAN_UNKNOWN;
> +
> +  bfd *abfd = nullptr;
>   
> -  /* Use default: NULL (ZERO).  */
>     union
>       {
>         /* Architecture-specific target description data.  Numerous targets
> @@ -1805,11 +1807,9 @@ struct gdbarch_info
>         int *id;
>       };
>   
> -  /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
> -  enum gdb_osabi osabi;
> +  enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
>   
> -  /* Use default: NULL (ZERO).  */
> -  const struct target_desc *target_desc;
> +  const struct target_desc *target_desc = nullptr;
>   };
>   
>   typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
> @@ -1883,8 +1883,8 @@ extern int gdbarch_update_p (struct gdbarch_info info);
>   
>   /* Helper function.  Find an architecture matching info.
>   
> -   INFO should be initialized using gdbarch_info_init, relevant fields
> -   set, and then finished using gdbarch_info_fill.
> +   INFO should have relevant fields set, and then finished using
> +   gdbarch_info_fill.
>   
>      Returns the corresponding architecture, or NULL if no matching
>      architecture was found.  */
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 01a5ac88219d..e82d711ff2c3 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1533,18 +1533,20 @@ struct gdbarch_list
>   
>   struct gdbarch_info
>   {
> -  /* Use default: NULL (ZERO).  */
> -  const struct bfd_arch_info *bfd_arch_info;
> +  gdbarch_info ()
> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
> +       no member larget than TDESC_DATA.  */

Typo, larger?

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Simon Marchi]

>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index 01a5ac88219d..e82d711ff2c3 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -1533,18 +1533,20 @@ struct gdbarch_list
>>     struct gdbarch_info
>>   {
>> -  /* Use default: NULL (ZERO).  */
>> -  const struct bfd_arch_info *bfd_arch_info;
>> +  gdbarch_info ()
>> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
>> +       no member larget than TDESC_DATA.  */
> 
> Typo, larger?

Indeed!  Will fix locally, thanks.

Simon

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Weimin Pan]

On 6/23/2021 1:35 PM, Simon Marchi via Gdb-patches wrote:
>>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>>> index 01a5ac88219d..e82d711ff2c3 100755
>>> --- a/gdb/gdbarch.sh
>>> +++ b/gdb/gdbarch.sh
>>> @@ -1533,18 +1533,20 @@ struct gdbarch_list
>>>      struct gdbarch_info
>>>    {
>>> -  /* Use default: NULL (ZERO).  */
>>> -  const struct bfd_arch_info *bfd_arch_info;
>>> +  gdbarch_info ()
>>> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
>>> +       no member larget than TDESC_DATA.  */
>> Typo, larger?
> Indeed!  Will fix locally, thanks.

The same typo is also in gdbarch.h.

>
> Simon

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Simon Marchi]

On 2021-06-23 5:40 p.m., Weimin Pan wrote:
> 
> On 6/23/2021 1:35 PM, Simon Marchi via Gdb-patches wrote:
>>>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>>>> index 01a5ac88219d..e82d711ff2c3 100755
>>>> --- a/gdb/gdbarch.sh
>>>> +++ b/gdb/gdbarch.sh
>>>> @@ -1533,18 +1533,20 @@ struct gdbarch_list
>>>>      struct gdbarch_info
>>>>    {
>>>> -  /* Use default: NULL (ZERO).  */
>>>> -  const struct bfd_arch_info *bfd_arch_info;
>>>> +  gdbarch_info ()
>>>> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
>>>> +       no member larget than TDESC_DATA.  */
>>> Typo, larger?
>> Indeed!  Will fix locally, thanks.
> 
> The same typo is also in gdbarch.h.

This is generated from gdbarch.sh, so is fixed when re-running gdbarch.h (which I must not forget to run!).  Thanks for noticing.

Simon

Re: [PATCH 1/2] gdb: remove gdbarch_info::tdep_info

[Andrew Burgess]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-23 15:27:16 -0400]:

> This field is not actually used, remove it.
> 
> gdb/ChangeLog:
> 
> 	* gdbarch.sh (struct gdbarch_info) <tdep_info>: Remove.
> 	(gdbarch_find_by_info): Remove print.

Shouldn't this ChangeLog have 'gdbarch.{ch}: Regenerate.' too?

Otherwise, LGTM.

thanks,
Andrew


> 
> Change-Id: I00af4681b8e1a27727441cbadc3827f5914bd8eb
> ---
>  gdb/gdbarch.c  | 3 ---
>  gdb/gdbarch.h  | 4 ----
>  gdb/gdbarch.sh | 7 -------
>  3 files changed, 14 deletions(-)
> 
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 208cf4b5aaab..830a86df89f3 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -5695,9 +5695,6 @@ gdbarch_find_by_info (struct gdbarch_info info)
>        fprintf_unfiltered (gdb_stdlog,
>  			  "gdbarch_find_by_info: info.abfd %s\n",
>  			  host_address_to_string (info.abfd));
> -      fprintf_unfiltered (gdb_stdlog,
> -			  "gdbarch_find_by_info: info.tdep_info %s\n",
> -			  host_address_to_string (info.tdep_info));
>      }
>  
>    /* Find the tdep code that knows about this architecture.  */
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 7157e5596fd3..36ea4de17c8c 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1795,10 +1795,6 @@ struct gdbarch_info
>    /* Use default: NULL (ZERO).  */
>    union
>      {
> -      /* Architecture-specific information.  The generic form for targets
> -	 that have extra requirements.  */
> -      struct gdbarch_tdep_info *tdep_info;
> -
>        /* Architecture-specific target description data.  Numerous targets
>  	 need only this, so give them an easy way to hold it.  */
>        struct tdesc_arch_data *tdesc_data;
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 43e51341f97b..01a5ac88219d 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1547,10 +1547,6 @@ struct gdbarch_info
>    /* Use default: NULL (ZERO).  */
>    union
>      {
> -      /* Architecture-specific information.  The generic form for targets
> -	 that have extra requirements.  */
> -      struct gdbarch_tdep_info *tdep_info;
> -
>        /* Architecture-specific target description data.  Numerous targets
>  	 need only this, so give them an easy way to hold it.  */
>        struct tdesc_arch_data *tdesc_data;
> @@ -2479,9 +2475,6 @@ gdbarch_find_by_info (struct gdbarch_info info)
>        fprintf_unfiltered (gdb_stdlog,
>  			  "gdbarch_find_by_info: info.abfd %s\n",
>  			  host_address_to_string (info.abfd));
> -      fprintf_unfiltered (gdb_stdlog,
> -			  "gdbarch_find_by_info: info.tdep_info %s\n",
> -			  host_address_to_string (info.tdep_info));
>      }
>  
>    /* Find the tdep code that knows about this architecture.  */
> -- 
> 2.32.0
> 

Re: [PATCH 1/2] gdb: remove gdbarch_info::tdep_info

[Simon Marchi]

On 2021-06-25 4:24 p.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-23 15:27:16 -0400]:
> 
>> This field is not actually used, remove it.
>>
>> gdb/ChangeLog:
>>
>> 	* gdbarch.sh (struct gdbarch_info) <tdep_info>: Remove.
>> 	(gdbarch_find_by_info): Remove print.
> 
> Shouldn't this ChangeLog have 'gdbarch.{ch}: Regenerate.' too?
> 
> Otherwise, LGTM.
> 
> thanks,
> Andrew

Oh, yes, will add.  Thanks!

Simon

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Andrew Burgess]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-23 15:27:17 -0400]:

> While reviewing another patch, I realized that gdbarch_info_init could
> easily be removed in favor of initializing gdbarch_info fields directly
> in the struct declaration.  The only odd part is the union.  I don't
> know if it's actually important for it to be zero-initialized, but I
> presume it is.  I added a constructor to gdbarch_info to take care of
> that.  A proper solution would be to use std::variant.  Or, these could
> also be separate fields, the little extra space required wouldn't
> matter.
> 
> gdb/ChangeLog:
> 
> 	* gdbarch.sh (struct gdbarch_info): Initialize fields, add
> 	constructor.
> 	* gdbarch.h: Re-generate.
> 	* arch-utils.h (gdbarch_info_init): Remove, delete all usages.

I'm still seeing a use of gdbarch_info_init in corelow.c
(core_target::core_target) that prevents GDB compiling.

I was not previously aware of the 'id' field within gdbarch_info, and
I was wondering what it was for.  As far as I can tell it's only used
for aarch64 with it being set in:

  aarch64_linux_nat_target::thread_architecture

and used in:

  aarch64_gdbarch_init

like this:

  uint64_t vq = 0;
  if (info.id == (int *) -1)
    vq = 0;
  else if (info.id != 0)
    vq = (uint64_t) info.id;
  else
    vq = aarch64_get_tdesc_vq (info.target_desc);

Now, maybe I'm missing some cleverness here, but isn't the final use
of 'info.target_desc' meaningless?  It shares space with the id field
that we're checking?

Given that only ::thread_architecture seems to set the id before
calling gdbarch_find_by_info, I assume that if we, for example,
connected to a remote target, the first pass through this code would
end up treating the target_desc point like id?

I guess, I'm suggesting that this aarch64 code looks broken to me, and
I wonder if the fix is to split the target_desc and id out from the
union?

Thanks,
Andrew

> 	* arch-utils.c (gdbarch_info_init): Remove.
> 
> Change-Id: I7502e08fe0f278d84eef1667a072e8a97bda5ab5
> ---
>  gdb/aarch64-linux-nat.c   |  1 -
>  gdb/aarch64-tdep.c        |  2 --
>  gdb/arch-utils.c          | 26 +-------------------------
>  gdb/arch-utils.h          |  6 ------
>  gdb/arm-tdep.c            |  7 +------
>  gdb/cris-tdep.c           |  3 ---
>  gdb/gdbarch.h             | 28 ++++++++++++++--------------
>  gdb/gdbarch.sh            | 28 ++++++++++++++--------------
>  gdb/i386-darwin-nat.c     |  3 +--
>  gdb/inferior.c            |  3 +--
>  gdb/mips-tdep.c           |  5 -----
>  gdb/osabi.c               |  4 +---
>  gdb/rs6000-nat.c          |  3 +--
>  gdb/rs6000-tdep.c         |  4 +---
>  gdb/selftest-arch.c       |  1 -
>  gdb/target-descriptions.c |  2 --
>  16 files changed, 35 insertions(+), 91 deletions(-)
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 61224022f6aa..c7cbebbc3517 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -1060,7 +1060,6 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
>       new one), stashing the vector length inside id.  Use -1 for when SVE
>       unavailable, to distinguish from an unset value of 0.  */
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);
>    info.id = (int *) (vq == 0 ? -1 : vq);
>    return gdbarch_find_by_info (info);
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 202b02108744..dea16e729c03 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -627,7 +627,6 @@ aarch64_analyze_prologue_test (void)
>  {
>    struct gdbarch_info info;
>  
> -  gdbarch_info_init (&info);
>    info.bfd_arch_info = bfd_scan_arch ("aarch64");
>  
>    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> @@ -4559,7 +4558,6 @@ aarch64_process_record_test (void)
>    struct gdbarch_info info;
>    uint32_t ret;
>  
> -  gdbarch_info_init (&info);
>    info.bfd_arch_info = bfd_scan_arch ("aarch64");
>  
>    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index c75a79757ea2..4290d637ce16 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -389,8 +389,6 @@ set_endian (const char *ignore_args, int from_tty, struct cmd_list_element *c)
>  {
>    struct gdbarch_info info;
>  
> -  gdbarch_info_init (&info);
> -
>    if (set_endian_string == endian_auto)
>      {
>        target_byte_order_user = BFD_ENDIAN_UNKNOWN;
> @@ -548,8 +546,6 @@ set_architecture (const char *ignore_args,
>  {
>    struct gdbarch_info info;
>  
> -  gdbarch_info_init (&info);
> -
>    if (strcmp (set_architecture_string, "auto") == 0)
>      {
>        target_architecture_user = NULL;
> @@ -630,7 +626,6 @@ struct gdbarch *
>  gdbarch_from_bfd (bfd *abfd)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>  
>    info.abfd = abfd;
>    return gdbarch_find_by_info (info);
> @@ -645,7 +640,6 @@ set_gdbarch_from_file (bfd *abfd)
>    struct gdbarch_info info;
>    struct gdbarch *gdbarch;
>  
> -  gdbarch_info_init (&info);
>    info.abfd = abfd;
>    info.target_desc = target_current_description ();
>    gdbarch = gdbarch_find_by_info (info);
> @@ -679,10 +673,6 @@ void
>  initialize_current_architecture (void)
>  {
>    const char **arches = gdbarch_printable_names ();
> -  struct gdbarch_info info;
> -
> -  /* determine a default architecture and byte order.  */
> -  gdbarch_info_init (&info);
>    
>    /* Find a default architecture.  */
>    if (default_bfd_arch == NULL)
> @@ -705,6 +695,7 @@ initialize_current_architecture (void)
>  			_("initialize_current_architecture: Arch not found"));
>      }
>  
> +  gdbarch_info info;
>    info.bfd_arch_info = default_bfd_arch;
>  
>    /* Take several guesses at a byte order.  */
> @@ -769,21 +760,6 @@ initialize_current_architecture (void)
>    }
>  }
>  
> -
> -/* Initialize a gdbarch info to values that will be automatically
> -   overridden.  Note: Originally, this ``struct info'' was initialized
> -   using memset(0).  Unfortunately, that ran into problems, namely
> -   BFD_ENDIAN_BIG is zero.  An explicit initialization function that
> -   can explicitly set each field to a well defined value is used.  */
> -
> -void
> -gdbarch_info_init (struct gdbarch_info *info)
> -{
> -  memset (info, 0, sizeof (struct gdbarch_info));
> -  info->byte_order = BFD_ENDIAN_UNKNOWN;
> -  info->byte_order_for_code = info->byte_order;
> -}
> -
>  /* Similar to init, but this time fill in the blanks.  Information is
>     obtained from the global "set ..." options and explicitly
>     initialized INFO fields.  */
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index a5b40ad8ff1b..03e9082f6d77 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -196,12 +196,6 @@ extern enum bfd_endian selected_byte_order (void);
>     was explicitly selected.  */
>  extern const char *selected_architecture_name (void);
>  
> -/* Initialize a ``struct info''.  Can't use memset(0) since some
> -   default values are not zero.  "fill" takes all available
> -   information and fills in any unspecified fields.  */
> -
> -extern void gdbarch_info_init (struct gdbarch_info *info);
> -
>  /* Similar to init, but this time fill in the blanks.  Information is
>     obtained from the global "set ..." options and explicitly
>     initialized INFO fields.  */
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 857a52a9a513..339e03271e7e 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -8403,15 +8403,12 @@ arm_skip_stub (struct frame_info *frame, CORE_ADDR pc)
>  static void
>  arm_update_current_architecture (void)
>  {
> -  struct gdbarch_info info;
> -
>    /* If the current architecture is not ARM, we have nothing to do.  */
>    if (gdbarch_bfd_arch_info (target_gdbarch ())->arch != bfd_arch_arm)
>      return;
>  
>    /* Update the architecture.  */
> -  gdbarch_info_init (&info);
> -
> +  gdbarch_info info;
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, _("could not update architecture"));
>  }
> @@ -13237,7 +13234,6 @@ static void
>  arm_record_test (void)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    info.bfd_arch_info = bfd_scan_arch ("arm");
>  
>    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> @@ -13329,7 +13325,6 @@ arm_analyze_prologue_test ()
>    for (bfd_endian endianness : {BFD_ENDIAN_LITTLE, BFD_ENDIAN_BIG})
>      {
>        struct gdbarch_info info;
> -      gdbarch_info_init (&info);
>        info.byte_order = endianness;
>        info.byte_order_for_code = endianness;
>        info.bfd_arch_info = bfd_scan_arch ("arm");
> diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
> index e1c141865cc6..01ef1604bffa 100644
> --- a/gdb/cris-tdep.c
> +++ b/gdb/cris-tdep.c
> @@ -3879,7 +3879,6 @@ set_cris_version (const char *ignore_args, int from_tty,
>    usr_cmd_cris_version_valid = 1;
>    
>    /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, 
>  		    _("cris_gdbarch_update: failed to update architecture."));
> @@ -3892,7 +3891,6 @@ set_cris_mode (const char *ignore_args, int from_tty,
>    struct gdbarch_info info;
>  
>    /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, 
>  		    "cris_gdbarch_update: failed to update architecture.");
> @@ -3905,7 +3903,6 @@ set_cris_dwarf2_cfi (const char *ignore_args, int from_tty,
>    struct gdbarch_info info;
>  
>    /* Update the current architecture, if needed.  */
> -  gdbarch_info_init (&info);
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, 
>  		    _("cris_gdbarch_update: failed to update architecture."));
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index 36ea4de17c8c..f94695adaa08 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -1781,18 +1781,20 @@ struct gdbarch_list
>  
>  struct gdbarch_info
>  {
> -  /* Use default: NULL (ZERO).  */
> -  const struct bfd_arch_info *bfd_arch_info;
> +  gdbarch_info ()
> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
> +       no member larget than TDESC_DATA.  */
> +    : tdesc_data ()
> +  {}
>  
> -  /* Use default: BFD_ENDIAN_UNKNOWN (NB: is not ZERO).  */
> -  enum bfd_endian byte_order;
> +  const struct bfd_arch_info *bfd_arch_info = nullptr;
>  
> -  enum bfd_endian byte_order_for_code;
> +  enum bfd_endian byte_order = BFD_ENDIAN_UNKNOWN;
>  
> -  /* Use default: NULL (ZERO).  */
> -  bfd *abfd;
> +  enum bfd_endian byte_order_for_code = BFD_ENDIAN_UNKNOWN;
> +
> +  bfd *abfd = nullptr;
>  
> -  /* Use default: NULL (ZERO).  */
>    union
>      {
>        /* Architecture-specific target description data.  Numerous targets
> @@ -1805,11 +1807,9 @@ struct gdbarch_info
>        int *id;
>      };
>  
> -  /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
> -  enum gdb_osabi osabi;
> +  enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
>  
> -  /* Use default: NULL (ZERO).  */
> -  const struct target_desc *target_desc;
> +  const struct target_desc *target_desc = nullptr;
>  };
>  
>  typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
> @@ -1883,8 +1883,8 @@ extern int gdbarch_update_p (struct gdbarch_info info);
>  
>  /* Helper function.  Find an architecture matching info.
>  
> -   INFO should be initialized using gdbarch_info_init, relevant fields
> -   set, and then finished using gdbarch_info_fill.
> +   INFO should have relevant fields set, and then finished using
> +   gdbarch_info_fill.
>  
>     Returns the corresponding architecture, or NULL if no matching
>     architecture was found.  */
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 01a5ac88219d..e82d711ff2c3 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -1533,18 +1533,20 @@ struct gdbarch_list
>  
>  struct gdbarch_info
>  {
> -  /* Use default: NULL (ZERO).  */
> -  const struct bfd_arch_info *bfd_arch_info;
> +  gdbarch_info ()
> +    /* Ensure the union is zero-initialized.  Relies on the fact that there's
> +       no member larget than TDESC_DATA.  */
> +    : tdesc_data ()
> +  {}
> +
> +  const struct bfd_arch_info *bfd_arch_info = nullptr;
>  
> -  /* Use default: BFD_ENDIAN_UNKNOWN (NB: is not ZERO).  */
> -  enum bfd_endian byte_order;
> +  enum bfd_endian byte_order = BFD_ENDIAN_UNKNOWN;
>  
> -  enum bfd_endian byte_order_for_code;
> +  enum bfd_endian byte_order_for_code = BFD_ENDIAN_UNKNOWN;
>  
> -  /* Use default: NULL (ZERO).  */
> -  bfd *abfd;
> +  bfd *abfd = nullptr;
>  
> -  /* Use default: NULL (ZERO).  */
>    union
>      {
>        /* Architecture-specific target description data.  Numerous targets
> @@ -1557,11 +1559,9 @@ struct gdbarch_info
>        int *id;
>      };
>  
> -  /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
> -  enum gdb_osabi osabi;
> +  enum gdb_osabi osabi = GDB_OSABI_UNKNOWN;
>  
> -  /* Use default: NULL (ZERO).  */
> -  const struct target_desc *target_desc;
> +  const struct target_desc *target_desc = nullptr;
>  };
>  
>  typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
> @@ -1637,8 +1637,8 @@ extern int gdbarch_update_p (struct gdbarch_info info);
>  
>  /* Helper function.  Find an architecture matching info.
>  
> -   INFO should be initialized using gdbarch_info_init, relevant fields
> -   set, and then finished using gdbarch_info_fill.
> +   INFO should have relevant fields set, and then finished using
> +   gdbarch_info_fill.
>  
>     Returns the corresponding architecture, or NULL if no matching
>     architecture was found.  */
> diff --git a/gdb/i386-darwin-nat.c b/gdb/i386-darwin-nat.c
> index 922d7a6df0ca..c1d6139490d8 100644
> --- a/gdb/i386-darwin-nat.c
> +++ b/gdb/i386-darwin-nat.c
> @@ -479,7 +479,6 @@ darwin_check_osabi (darwin_inferior *inf, thread_t thread)
>      {
>        /* Attaching to a process.  Let's figure out what kind it is.  */
>        x86_thread_state_t gp_regs;
> -      struct gdbarch_info info;
>        unsigned int gp_count = x86_THREAD_STATE_COUNT;
>        kern_return_t ret;
>  
> @@ -491,7 +490,7 @@ darwin_check_osabi (darwin_inferior *inf, thread_t thread)
>  	  return;
>  	}
>  
> -      gdbarch_info_init (&info);
> +      gdbarch_info info;
>        gdbarch_info_fill (&info);
>        info.byte_order = gdbarch_byte_order (target_gdbarch ());
>        info.osabi = GDB_OSABI_DARWIN;
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 059839ec9626..e1c70d553fde 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -747,7 +747,6 @@ add_inferior_with_spaces (void)
>    struct address_space *aspace;
>    struct program_space *pspace;
>    struct inferior *inf;
> -  struct gdbarch_info info;
>  
>    /* If all inferiors share an address space on this system, this
>       doesn't really return a new address space; otherwise, it
> @@ -760,7 +759,7 @@ add_inferior_with_spaces (void)
>  
>    /* Setup the inferior's initial arch, based on information obtained
>       from the global "set ..." options.  */
> -  gdbarch_info_init (&info);
> +  gdbarch_info info;
>    inf->gdbarch = gdbarch_find_by_info (info);
>    /* The "set ..." options reject invalid settings, so we should
>       always have a valid arch by now.  */
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index dbace1212508..2aee4f10b316 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -865,7 +865,6 @@ set_mips64_transfers_32bit_regs (const char *args, int from_tty,
>  				 struct cmd_list_element *c)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    /* 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.  */
> @@ -6918,7 +6917,6 @@ static void
>  set_mipsfpu_single_command (const char *args, int from_tty)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    mips_fpu_type = MIPS_FPU_SINGLE;
>    mips_fpu_type_auto = 0;
>    /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
> @@ -6932,7 +6930,6 @@ static void
>  set_mipsfpu_double_command (const char *args, int from_tty)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    mips_fpu_type = MIPS_FPU_DOUBLE;
>    mips_fpu_type_auto = 0;
>    /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
> @@ -6946,7 +6943,6 @@ static void
>  set_mipsfpu_none_command (const char *args, int from_tty)
>  {
>    struct gdbarch_info info;
> -  gdbarch_info_init (&info);
>    mips_fpu_type = MIPS_FPU_NONE;
>    mips_fpu_type_auto = 0;
>    /* FIXME: cagney/2003-11-15: Should be setting a field in "info"
> @@ -8790,7 +8786,6 @@ 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_info_init (&info);
>    gdbarch_update_p (info);
>  }
>  
> diff --git a/gdb/osabi.c b/gdb/osabi.c
> index b1603c09c855..aabf895c0451 100644
> --- a/gdb/osabi.c
> +++ b/gdb/osabi.c
> @@ -598,8 +598,6 @@ generic_elf_osabi_sniffer (bfd *abfd)
>  static void
>  set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
>  {
> -  struct gdbarch_info info;
> -
>    if (strcmp (set_osabi_string, "auto") == 0)
>      user_osabi_state = osabi_auto;
>    else if (strcmp (set_osabi_string, "default") == 0)
> @@ -630,7 +628,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_init (&info);
> +  gdbarch_info info;
>    if (! gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, _("Updating OS ABI failed."));
>  }
> diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c
> index 50c96e281c64..ee00a600343a 100644
> --- a/gdb/rs6000-nat.c
> +++ b/gdb/rs6000-nat.c
> @@ -558,7 +558,6 @@ rs6000_nat_target::create_inferior (const char *exec_file,
>    enum bfd_architecture arch;
>    unsigned long mach;
>    bfd abfd;
> -  struct gdbarch_info info;
>  
>    inf_ptrace_target::create_inferior (exec_file, allargs, env, from_tty);
>  
> @@ -593,7 +592,7 @@ rs6000_nat_target::create_inferior (const char *exec_file,
>  
>    bfd_default_set_arch_mach (&abfd, arch, mach);
>  
> -  gdbarch_info_init (&info);
> +  gdbarch_info info;
>    info.bfd_arch_info = bfd_get_arch_info (&abfd);
>    info.abfd = current_program_space->exec_bfd ();
>  
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index 5bcf5fca2c3b..a629450d746f 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -7260,7 +7260,6 @@ powerpc_set_soft_float (const char *args, int from_tty,
>    struct gdbarch_info info;
>  
>    /* Update the architecture.  */
> -  gdbarch_info_init (&info);
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, _("could not update architecture"));
>  }
> @@ -7269,7 +7268,6 @@ static void
>  powerpc_set_vector_abi (const char *args, int from_tty,
>  			struct cmd_list_element *c)
>  {
> -  struct gdbarch_info info;
>    int vector_abi;
>  
>    for (vector_abi = POWERPC_VEC_AUTO;
> @@ -7287,7 +7285,7 @@ powerpc_set_vector_abi (const char *args, int from_tty,
>  		    powerpc_vector_abi_string);
>  
>    /* Update the architecture.  */
> -  gdbarch_info_init (&info);
> +  gdbarch_info info;
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__, _("could not update architecture"));
>  }
> diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
> index ab1215c848c3..0eef134d4841 100644
> --- a/gdb/selftest-arch.c
> +++ b/gdb/selftest-arch.c
> @@ -64,7 +64,6 @@ struct gdbarch_selftest : public selftest
>  	  {
>  	    struct gdbarch_info info;
>  
> -	    gdbarch_info_init (&info);
>  	    info.bfd_arch_info = bfd_scan_arch (arches[i]);
>  
>  	    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index f7d0a688c62f..f94ace756d4d 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -555,7 +555,6 @@ target_find_description (void)
>      {
>        struct gdbarch_info info;
>  
> -      gdbarch_info_init (&info);
>        info.target_desc = tdesc_info->tdesc;
>        if (!gdbarch_update_p (info))
>  	warning (_("Architecture rejected target-supplied description"));
> @@ -592,7 +591,6 @@ target_clear_description (void)
>    tdesc_info->tdesc = nullptr;
>  
>    gdbarch_info info;
> -  gdbarch_info_init (&info);
>    if (!gdbarch_update_p (info))
>      internal_error (__FILE__, __LINE__,
>  		    _("Could not remove target-supplied description"));
> -- 
> 2.32.0
> 

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Simon Marchi]

On 2021-06-25 4:55 p.m., Andrew Burgess wrote:
> * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-23 15:27:17 -0400]:
> 
>> While reviewing another patch, I realized that gdbarch_info_init could
>> easily be removed in favor of initializing gdbarch_info fields directly
>> in the struct declaration.  The only odd part is the union.  I don't
>> know if it's actually important for it to be zero-initialized, but I
>> presume it is.  I added a constructor to gdbarch_info to take care of
>> that.  A proper solution would be to use std::variant.  Or, these could
>> also be separate fields, the little extra space required wouldn't
>> matter.
>>
>> gdb/ChangeLog:
>>
>> 	* gdbarch.sh (struct gdbarch_info): Initialize fields, add
>> 	constructor.
>> 	* gdbarch.h: Re-generate.
>> 	* arch-utils.h (gdbarch_info_init): Remove, delete all usages.
> 
> I'm still seeing a use of gdbarch_info_init in corelow.c
> (core_target::core_target) that prevents GDB compiling.

Yes, that's the call I saw in review that prompted me to write this
patch.  Luis merged his patch, so after rebasing I need to get rid of
that call too, thanks.

> I was not previously aware of the 'id' field within gdbarch_info, and
> I was wondering what it was for.  As far as I can tell it's only used
> for aarch64 with it being set in:

It was originally used for SPU, as the comment implies, but AArch64
started using it because... it's an int field, and AArch64 needed an int
field to pass the vq value.

> 
>   aarch64_linux_nat_target::thread_architecture
> 
> and used in:
> 
>   aarch64_gdbarch_init
> 
> like this:
> 
>   uint64_t vq = 0;
>   if (info.id == (int *) -1)
>     vq = 0;
>   else if (info.id != 0)
>     vq = (uint64_t) info.id;
>   else
>     vq = aarch64_get_tdesc_vq (info.target_desc);
> 
> Now, maybe I'm missing some cleverness here, but isn't the final use
> of 'info.target_desc' meaningless?  It shares space with the id field
> that we're checking?
> 
> Given that only ::thread_architecture seems to set the id before
> calling gdbarch_find_by_info, I assume that if we, for example,
> connected to a remote target, the first pass through this code would
> end up treating the target_desc point like id?
> 
> I guess, I'm suggesting that this aarch64 code looks broken to me, and
> I wonder if the fix is to split the target_desc and id out from the
> union?

I think you are confusing tdesc_data and target_desc.  The id field
shares its space with:

   struct tdesc_arch_data *tdesc_data;

not

  const struct target_desc *target_desc = nullptr;

I can't explain really explain why this is designed this way, with a
union, but it looks like "id" and "tdesc_data" are not used
simultaneously.  "id" is used between
aarch64_linux_nat_target::thread_architecture and aarch64_gdbarch_init
(or it is zero, the default value).  "tdesc_data" is then used between
aarch64_gdbarch_init and gdbarch_osabi_init and the osabi init handler.

Simon

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-06-27 22:11:24 -0400]:

> 
> 
> On 2021-06-25 4:55 p.m., Andrew Burgess wrote:
> > * Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-06-23 15:27:17 -0400]:
> > 
> >> While reviewing another patch, I realized that gdbarch_info_init could
> >> easily be removed in favor of initializing gdbarch_info fields directly
> >> in the struct declaration.  The only odd part is the union.  I don't
> >> know if it's actually important for it to be zero-initialized, but I
> >> presume it is.  I added a constructor to gdbarch_info to take care of
> >> that.  A proper solution would be to use std::variant.  Or, these could
> >> also be separate fields, the little extra space required wouldn't
> >> matter.
> >>
> >> gdb/ChangeLog:
> >>
> >> 	* gdbarch.sh (struct gdbarch_info): Initialize fields, add
> >> 	constructor.
> >> 	* gdbarch.h: Re-generate.
> >> 	* arch-utils.h (gdbarch_info_init): Remove, delete all usages.
> > 
> > I'm still seeing a use of gdbarch_info_init in corelow.c
> > (core_target::core_target) that prevents GDB compiling.
> 
> Yes, that's the call I saw in review that prompted me to write this
> patch.  Luis merged his patch, so after rebasing I need to get rid of
> that call too, thanks.
> 
> > I was not previously aware of the 'id' field within gdbarch_info, and
> > I was wondering what it was for.  As far as I can tell it's only used
> > for aarch64 with it being set in:
> 
> It was originally used for SPU, as the comment implies, but AArch64
> started using it because... it's an int field, and AArch64 needed an int
> field to pass the vq value.
> 
> > 
> >   aarch64_linux_nat_target::thread_architecture
> > 
> > and used in:
> > 
> >   aarch64_gdbarch_init
> > 
> > like this:
> > 
> >   uint64_t vq = 0;
> >   if (info.id == (int *) -1)
> >     vq = 0;
> >   else if (info.id != 0)
> >     vq = (uint64_t) info.id;
> >   else
> >     vq = aarch64_get_tdesc_vq (info.target_desc);
> > 
> > Now, maybe I'm missing some cleverness here, but isn't the final use
> > of 'info.target_desc' meaningless?  It shares space with the id field
> > that we're checking?
> > 
> > Given that only ::thread_architecture seems to set the id before
> > calling gdbarch_find_by_info, I assume that if we, for example,
> > connected to a remote target, the first pass through this code would
> > end up treating the target_desc point like id?
> > 
> > I guess, I'm suggesting that this aarch64 code looks broken to me, and
> > I wonder if the fix is to split the target_desc and id out from the
> > union?
> 
> I think you are confusing tdesc_data and target_desc.  The id field
> shares its space with:
> 
>    struct tdesc_arch_data *tdesc_data;
> 
> not
> 
>   const struct target_desc *target_desc = nullptr;
> 
> I can't explain really explain why this is designed this way, with a
> union, but it looks like "id" and "tdesc_data" are not used
> simultaneously.  "id" is used between
> aarch64_linux_nat_target::thread_architecture and aarch64_gdbarch_init
> (or it is zero, the default value).  "tdesc_data" is then used between
> aarch64_gdbarch_init and gdbarch_osabi_init and the osabi init handler.

Thanks for pointing that out.  In that case the patch LGTM (with the
extra call removed once this is rebased).

Thanks,
Andrew

Re: [PATCH 2/2] gdb: remove gdbarch_info_init

[Simon Marchi]

On 2021-06-28 5:16 a.m., Andrew Burgess wrote:
> Thanks for pointing that out.  In that case the patch LGTM (with the
> extra call removed once this is rebased).
> 
> Thanks,
> Andrew

Thanks, will push shortly.

Simon