From: "Maciej W. Rozycki" <macro@imgtec.com>
To: <gdb-patches@sourceware.org>
Cc: Simon Marchi <simon.marchi@ericsson.com>
Subject: [PATCH] gdbarch: Use an anonymous union for target data in `gdbarch_info'
Date: Tue, 18 Oct 2016 16:20:00 -0000 [thread overview]
Message-ID: <alpine.DEB.2.00.1610181615410.31859@tp.orcam.me.uk> (raw)
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'.
* spu-multiarch.c (spu_gdbarch): Use `info.id' rather than
`info.tdep_info'.
* spu-tdep.c (spu_gdbarch_init): Likewise.
* gdbarch.h: Regenerate.
---
Hi,
I made this change in response to my own concerns about casts expressed
here: <https://sourceware.org/ml/gdb-patches/2016-07/msg00332.html> --
IMHO having casts sprinkled all over the place is not a sign of a
particularly good coding style and it certainly makes code less readable
and more prone to errors. I decided to give the updated `tdep_info'
member back its original type, as this will be used by the change the
review of which I referred to above; for the record the change itself is
here: <https://sourceware.org/ml/gdb-patches/2016-06/msg00425.html>.
Besides, having a `void *' member does not make sense if the purpose of
the change is to avoid casts. This member is still used even now by
gdbarch code itself.
The predominant use for `tdep_info' is to store a pointer to `struct
tdesc_arch_data', which is also what the MIPS target currently does, so I
concluded running regression testing for `mips-mti-linux-gnu' just to be
sure I haven't botched something up would be enough. There have been no
regressions. I have no SPU target to test, but the change is I believe
obviously correct.
OK to apply?
Maciej
gdb-gdbarch-tdep-info-union.diff
Index: binutils/gdb/aarch64-tdep.c
===================================================================
--- binutils.orig/gdb/aarch64-tdep.c 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/aarch64-tdep.c 2016-10-18 02:47:30.000642302 +0100
@@ -2807,7 +2807,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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/amd64-linux-tdep.c 2016-10-18 02:47:30.016044107 +0100
@@ -1852,8 +1852,7 @@ amd64_linux_init_abi (struct gdbarch_inf
{
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;
@@ -2069,8 +2068,7 @@ amd64_x32_linux_init_abi (struct gdbarch
{
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/gdbarch.h
===================================================================
--- binutils.orig/gdb/gdbarch.h 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/gdbarch.h 2016-10-18 02:47:30.030463430 +0100
@@ -1613,7 +1613,12 @@ struct gdbarch_info
bfd *abfd;
/* Use default: NULL (ZERO). */
- void *tdep_info;
+ union
+ {
+ struct gdbarch_tdep_info *tdep_info;
+ struct tdesc_arch_data *tdesc_data;
+ int *id;
+ };
/* Use default: GDB_OSABI_UNINITIALIZED (-1). */
enum gdb_osabi osabi;
Index: binutils/gdb/gdbarch.sh
===================================================================
--- binutils.orig/gdb/gdbarch.sh 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/gdbarch.sh 2016-10-18 02:47:30.048973683 +0100
@@ -1459,7 +1459,12 @@ struct gdbarch_info
bfd *abfd;
/* Use default: NULL (ZERO). */
- void *tdep_info;
+ union
+ {
+ struct gdbarch_tdep_info *tdep_info;
+ struct tdesc_arch_data *tdesc_data;
+ 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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/i386-linux-tdep.c 2016-10-18 02:47:30.060328441 +0100
@@ -821,8 +821,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 2016-10-18 02:47:24.000000000 +0100
+++ binutils/gdb/i386-tdep.c 2016-10-18 02:47:30.072577119 +0100
@@ -8563,7 +8563,7 @@ i386_gdbarch_init (struct gdbarch_info i
set_gdbarch_insn_is_jump (gdbarch, i386_insn_is_jump);
/* Hook in ABI-specific overrides, if they have been registered. */
- 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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/mips-linux-tdep.c 2016-10-18 02:47:30.083801277 +0100
@@ -1646,8 +1646,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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/mips-tdep.c 2016-10-18 02:47:30.101120720 +0100
@@ -8850,7 +8850,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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/nds32-tdep.c 2016-10-18 03:32:20.843129509 +0100
@@ -2138,7 +2138,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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/ppc-linux-tdep.c 2016-10-18 02:47:30.120562194 +0100
@@ -1352,7 +1352,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;
@@ -1652,8 +1652,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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/rs6000-tdep.c 2016-10-18 02:47:30.139154795 +0100
@@ -6528,7 +6528,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/spu-multiarch.c
===================================================================
--- binutils.orig/gdb/spu-multiarch.c 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/spu-multiarch.c 2016-10-18 02:47:30.152561941 +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 2016-10-18 02:37:31.000000000 +0100
+++ binutils/gdb/spu-tdep.c 2016-10-18 02:47:30.166769728 +0100
@@ -2697,8 +2697,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)
next reply other threads:[~2016-10-18 16:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-18 16:20 Maciej W. Rozycki [this message]
2016-11-08 8:47 ` Maciej W. Rozycki
2016-11-08 21:20 ` Yao Qi
2016-11-08 21:43 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.00.1610181615410.31859@tp.orcam.me.uk \
--to=macro@imgtec.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@ericsson.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox