[v2] gdbarch: Use an anonymous union for target data in `gdbarch_info'

Message ID alpine.DEB.2.00.1707291508210.29991@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki Aug. 1, 2017, 3:52 p.m. UTC
  As an update to commit ede5f15146ae ("gdbarch.h: Change 
gdbarch_info::tdep_info's type to void *") replace the definition of the 
`tdep_info' member in `struct gdbarch_info' with an anonymous union, 
comprising the original member, with its type reverted to `struct 
gdbarch_tdep_info *', a `tdesc_data' member of a `struct tdesc_arch_data 
*' type and an `id' member of an `int *' type.  Remove now unnecessary 
casts throughout use places then, making code easier to read an less 
prone to errors, which may happen with casting.

	gdb/
	* gdbarch.sh (gdbarch_info): Replace the `tdep_info' member with
	a union of `tdep_info', `tdesc_data' and `id'.
	* aarch64-tdep.c (aarch64_gdbarch_init): Use `info.tdesc_data' 
	rather than `info.tdep_info'.
	* amd64-linux-tdep.c (amd64_linux_init_abi): Likewise.
	* i386-linux-tdep.c (i386_linux_init_abi): Likewise.
	* i386-tdep.c (i386_gdbarch_init): Likewise.
	* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
	* mips-tdep.c (mips_gdbarch_init): Likewise.
	* nds32-tdep.c (nds32_gdbarch_init): Likewise.
	* rs6000-tdep.c (rs6000_gdbarch_init): Likewise.
	* ppc-linux-tdep.c (ppu2spu_sniffer): Use `info.id' rather than 
	`info.tdep_info'.
	(ppc_linux_init_abi): Use `info.tdesc_data' rather than 
	`info.tdep_info'.
	* sparc-tdep.c (sparc32_gdbarch_init): Likewise.
	* spu-multiarch.c (spu_gdbarch): Use `info.id' rather than
	`info.tdep_info'.
	* spu-tdep.c (spu_gdbarch_init): Likewise.
	* gdbarch.h: Regenerate.
---
Hi,

 Rebased and updated as requested, see the change log below and: 
<https://patchwork.sourceware.org/patch/16622/> for the earlier 
discussion.  Verified to build with `--enable-targets=all' and no 
regressions with `mips-linux-gnu' and `x86_64-linux-gnu' targets.

 OK to apply?

  Maciej

Changes from v1:

- Descriptions for new union members added.

- SPARC backend modification added according to changes made on master 
  since the original submission.

---
 gdb/aarch64-tdep.c     |    2 +-
 gdb/amd64-linux-tdep.c |    6 ++----
 gdb/gdbarch.h          |   16 +++++++++++++++-
 gdb/gdbarch.sh         |   16 +++++++++++++++-
 gdb/i386-linux-tdep.c  |    3 +--
 gdb/i386-tdep.c        |    2 +-
 gdb/mips-linux-tdep.c  |    3 +--
 gdb/mips-tdep.c        |    2 +-
 gdb/nds32-tdep.c       |    2 +-
 gdb/ppc-linux-tdep.c   |    5 ++---
 gdb/rs6000-tdep.c      |    2 +-
 gdb/sparc-tdep.c       |    2 +-
 gdb/spu-multiarch.c    |    2 +-
 gdb/spu-tdep.c         |    4 ++--
 14 files changed, 45 insertions(+), 22 deletions(-)

gdb-gdbarch-tdep-info-union.diff
  

Comments

Simon Marchi Aug. 1, 2017, 6:34 p.m. UTC | #1
On 2017-08-01 05:52 PM, Maciej W. Rozycki wrote:
> As an update to commit ede5f15146ae ("gdbarch.h: Change 
> gdbarch_info::tdep_info's type to void *") replace the definition of the 
> `tdep_info' member in `struct gdbarch_info' with an anonymous union, 
> comprising the original member, with its type reverted to `struct 
> gdbarch_tdep_info *', a `tdesc_data' member of a `struct tdesc_arch_data 
> *' type and an `id' member of an `int *' type.  Remove now unnecessary 
> casts throughout use places then, making code easier to read an less 
> prone to errors, which may happen with casting.
> 
> 	gdb/
> 	* gdbarch.sh (gdbarch_info): Replace the `tdep_info' member with
> 	a union of `tdep_info', `tdesc_data' and `id'.
> 	* aarch64-tdep.c (aarch64_gdbarch_init): Use `info.tdesc_data' 
> 	rather than `info.tdep_info'.
> 	* amd64-linux-tdep.c (amd64_linux_init_abi): Likewise.
> 	* i386-linux-tdep.c (i386_linux_init_abi): Likewise.
> 	* i386-tdep.c (i386_gdbarch_init): Likewise.
> 	* mips-linux-tdep.c (mips_linux_init_abi): Likewise.
> 	* mips-tdep.c (mips_gdbarch_init): Likewise.
> 	* nds32-tdep.c (nds32_gdbarch_init): Likewise.
> 	* rs6000-tdep.c (rs6000_gdbarch_init): Likewise.
> 	* ppc-linux-tdep.c (ppu2spu_sniffer): Use `info.id' rather than 
> 	`info.tdep_info'.
> 	(ppc_linux_init_abi): Use `info.tdesc_data' rather than 
> 	`info.tdep_info'.
> 	* sparc-tdep.c (sparc32_gdbarch_init): Likewise.
> 	* spu-multiarch.c (spu_gdbarch): Use `info.id' rather than
> 	`info.tdep_info'.
> 	* spu-tdep.c (spu_gdbarch_init): Likewise.
> 	* gdbarch.h: Regenerate.
> ---
> Hi,
> 
>  Rebased and updated as requested, see the change log below and: 
> <https://patchwork.sourceware.org/patch/16622/> for the earlier 
> discussion.  Verified to build with `--enable-targets=all' and no 
> regressions with `mips-linux-gnu' and `x86_64-linux-gnu' targets.
> 
>  OK to apply?
> 
>   Maciej
> 
> Changes from v1:
> 
> - Descriptions for new union members added.
> 
> - SPARC backend modification added according to changes made on master 
>   since the original submission.

Hi Maciej,

This LGTM.

Simon
  
Maciej W. Rozycki Aug. 7, 2017, 2:55 p.m. UTC | #2
On Tue, 1 Aug 2017, Simon Marchi wrote:

> This LGTM.

 Applied now, thanks for your review.

  Maciej
  

Patch

Index: binutils/gdb/aarch64-tdep.c
===================================================================
--- binutils.orig/gdb/aarch64-tdep.c	2017-07-31 14:52:02.599875688 +0100
+++ binutils/gdb/aarch64-tdep.c	2017-08-01 14:23:17.929876233 +0100
@@ -2997,7 +2997,7 @@  aarch64_gdbarch_init (struct gdbarch_inf
 
   /* Hook in the ABI-specific overrides, if they have been registered.  */
   info.target_desc = tdesc;
-  info.tdep_info = (void *) tdesc_data;
+  info.tdesc_data = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
   dwarf2_frame_set_init_reg (gdbarch, aarch64_dwarf2_frame_init_reg);
Index: binutils/gdb/amd64-linux-tdep.c
===================================================================
--- binutils.orig/gdb/amd64-linux-tdep.c	2017-07-31 21:03:10.055131614 +0100
+++ binutils/gdb/amd64-linux-tdep.c	2017-08-01 14:23:17.944140958 +0100
@@ -1863,8 +1863,7 @@  static void
 amd64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  struct tdesc_arch_data *tdesc_data
-    = (struct tdesc_arch_data *) info.tdep_info;
+  struct tdesc_arch_data *tdesc_data = info.tdesc_data;
   const struct tdesc_feature *feature;
   int valid_p;
 
@@ -2077,8 +2076,7 @@  static void
 amd64_x32_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  struct tdesc_arch_data *tdesc_data
-    = (struct tdesc_arch_data *) info.tdep_info;
+  struct tdesc_arch_data *tdesc_data = info.tdesc_data;
   const struct tdesc_feature *feature;
   int valid_p;
 
Index: binutils/gdb/gdbarch.h
===================================================================
--- binutils.orig/gdb/gdbarch.h	2017-07-31 21:03:13.557252461 +0100
+++ binutils/gdb/gdbarch.h	2017-08-01 16:46:56.895811012 +0100
@@ -1647,7 +1647,21 @@  struct gdbarch_info
   bfd *abfd;
 
   /* Use default: NULL (ZERO).  */
-  void *tdep_info;
+  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;
+
+      /* SPU file system ID.  This is a single integer, so using the
+	 generic form would only complicate code.  Other targets may
+	 reuse this member if suitable.  */
+      int *id;
+    };
 
   /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
   enum gdb_osabi osabi;
Index: binutils/gdb/gdbarch.sh
===================================================================
--- binutils.orig/gdb/gdbarch.sh	2017-07-31 21:03:13.569331250 +0100
+++ binutils/gdb/gdbarch.sh	2017-08-01 16:45:30.547572596 +0100
@@ -1474,7 +1474,21 @@  struct gdbarch_info
   bfd *abfd;
 
   /* Use default: NULL (ZERO).  */
-  void *tdep_info;
+  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;
+
+      /* SPU file system ID.  This is a single integer, so using the
+	 generic form would only complicate code.  Other targets may
+	 reuse this member if suitable.  */
+      int *id;
+    };
 
   /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
   enum gdb_osabi osabi;
Index: binutils/gdb/i386-linux-tdep.c
===================================================================
--- binutils.orig/gdb/i386-linux-tdep.c	2017-07-31 21:03:14.233751092 +0100
+++ binutils/gdb/i386-linux-tdep.c	2017-08-01 14:23:18.052865360 +0100
@@ -855,8 +855,7 @@  i386_linux_init_abi (struct gdbarch_info
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   const struct target_desc *tdesc = info.target_desc;
-  struct tdesc_arch_data *tdesc_data
-    = (struct tdesc_arch_data *) info.tdep_info;
+  struct tdesc_arch_data *tdesc_data = info.tdesc_data;
   const struct tdesc_feature *feature;
   int valid_p;
 
Index: binutils/gdb/i386-tdep.c
===================================================================
--- binutils.orig/gdb/i386-tdep.c	2017-07-31 14:52:02.635555559 +0100
+++ binutils/gdb/i386-tdep.c	2017-08-01 14:23:18.068046742 +0100
@@ -8604,7 +8604,7 @@  i386_gdbarch_init (struct gdbarch_info i
   /* Hook in ABI-specific overrides, if they have been registered.
      Note: If INFO specifies a 64 bit arch, this is where we turn
      a 32-bit i386 into a 64-bit amd64.  */
-  info.tdep_info = tdesc_data;
+  info.tdesc_data = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
   if (!i386_validate_tdesc_p (tdep, tdesc_data))
Index: binutils/gdb/mips-linux-tdep.c
===================================================================
--- binutils.orig/gdb/mips-linux-tdep.c	2017-07-31 11:12:00.990250331 +0100
+++ binutils/gdb/mips-linux-tdep.c	2017-08-01 14:23:18.120974780 +0100
@@ -1616,8 +1616,7 @@  mips_linux_init_abi (struct gdbarch_info
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum mips_abi abi = mips_abi (gdbarch);
-  struct tdesc_arch_data *tdesc_data
-    = (struct tdesc_arch_data *) info.tdep_info;
+  struct tdesc_arch_data *tdesc_data = info.tdesc_data;
 
   linux_init_abi (info, gdbarch);
 
Index: binutils/gdb/mips-tdep.c
===================================================================
--- binutils.orig/gdb/mips-tdep.c	2017-08-01 11:31:16.838764652 +0100
+++ binutils/gdb/mips-tdep.c	2017-08-01 14:23:18.134275124 +0100
@@ -8785,7 +8785,7 @@  mips_gdbarch_init (struct gdbarch_info i
   mips_register_g_packet_guesses (gdbarch);
 
   /* Hook in OS ABI-specific overrides, if they have been registered.  */
-  info.tdep_info = tdesc_data;
+  info.tdesc_data = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
   /* The hook may have adjusted num_regs, fetch the final value and
Index: binutils/gdb/nds32-tdep.c
===================================================================
--- binutils.orig/gdb/nds32-tdep.c	2017-07-31 09:19:35.824226786 +0100
+++ binutils/gdb/nds32-tdep.c	2017-08-01 14:23:18.156720737 +0100
@@ -2134,7 +2134,7 @@  nds32_gdbarch_init (struct gdbarch_info 
   nds32_add_reggroups (gdbarch);
 
   /* Hook in ABI-specific overrides, if they have been registered.  */
-  info.tdep_info = (void *) tdesc_data;
+  info.tdesc_data = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
   /* Override tdesc_register callbacks for system registers.  */
Index: binutils/gdb/ppc-linux-tdep.c
===================================================================
--- binutils.orig/gdb/ppc-linux-tdep.c	2017-07-31 09:19:36.292744792 +0100
+++ binutils/gdb/ppc-linux-tdep.c	2017-08-01 14:23:18.180982711 +0100
@@ -1350,7 +1350,7 @@  ppu2spu_sniffer (const struct frame_unwi
       info.bfd_arch_info = bfd_lookup_arch (bfd_arch_spu, bfd_mach_spu);
       info.byte_order = BFD_ENDIAN_BIG;
       info.osabi = GDB_OSABI_LINUX;
-      info.tdep_info = &data.id;
+      info.id = &data.id;
       data.gdbarch = gdbarch_find_by_info (info);
       if (!data.gdbarch)
 	return 0;
@@ -1650,8 +1650,7 @@  ppc_linux_init_abi (struct gdbarch_info 
                     struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  struct tdesc_arch_data *tdesc_data
-    = (struct tdesc_arch_data *) info.tdep_info;
+  struct tdesc_arch_data *tdesc_data = info.tdesc_data;
   static const char *const stap_integer_prefixes[] = { "i", NULL };
   static const char *const stap_register_indirection_prefixes[] = { "(",
 								    NULL };
Index: binutils/gdb/rs6000-tdep.c
===================================================================
--- binutils.orig/gdb/rs6000-tdep.c	2017-07-31 11:12:01.583503710 +0100
+++ binutils/gdb/rs6000-tdep.c	2017-08-01 14:23:18.195238425 +0100
@@ -6529,7 +6529,7 @@  rs6000_gdbarch_init (struct gdbarch_info
 
   /* Hook in ABI-specific overrides, if they have been registered.  */
   info.target_desc = tdesc;
-  info.tdep_info = tdesc_data;
+  info.tdesc_data = tdesc_data;
   gdbarch_init_osabi (info, gdbarch);
 
   switch (info.osabi)
Index: binutils/gdb/sparc-tdep.c
===================================================================
--- binutils.orig/gdb/sparc-tdep.c	2017-07-31 09:19:40.159610319 +0100
+++ binutils/gdb/sparc-tdep.c	2017-08-01 14:23:18.201296672 +0100
@@ -1906,7 +1906,7 @@  sparc32_gdbarch_init (struct gdbarch_inf
         }
 
       /* Target description may have changed. */
-      info.tdep_info = tdesc_data;
+      info.tdesc_data = tdesc_data;
       tdesc_use_registers (gdbarch, tdesc, tdesc_data);
     }
 
Index: binutils/gdb/spu-multiarch.c
===================================================================
--- binutils.orig/gdb/spu-multiarch.c	2017-07-31 09:19:40.344558366 +0100
+++ binutils/gdb/spu-multiarch.c	2017-08-01 14:23:18.210497307 +0100
@@ -107,7 +107,7 @@  spu_gdbarch (int spufs_fd)
   info.bfd_arch_info = bfd_lookup_arch (bfd_arch_spu, bfd_mach_spu);
   info.byte_order = BFD_ENDIAN_BIG;
   info.osabi = GDB_OSABI_LINUX;
-  info.tdep_info = &spufs_fd;
+  info.id = &spufs_fd;
   return gdbarch_find_by_info (info);
 }
 
Index: binutils/gdb/spu-tdep.c
===================================================================
--- binutils.orig/gdb/spu-tdep.c	2017-07-31 14:52:02.701013577 +0100
+++ binutils/gdb/spu-tdep.c	2017-08-01 14:23:18.271183468 +0100
@@ -2668,8 +2668,8 @@  spu_gdbarch_init (struct gdbarch_info in
   int id = -1;
 
   /* Which spufs ID was requested as address space?  */
-  if (info.tdep_info)
-    id = *(int *)info.tdep_info;
+  if (info.id)
+    id = *info.id;
   /* For objfile architectures of SPU solibs, decode the ID from the name.
      This assumes the filename convention employed by solib-spu.c.  */
   else if (info.abfd)