From patchwork Tue Oct 18 16:19:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 16622 Received: (qmail 90266 invoked by alias); 18 Oct 2016 16:20:15 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 90249 invoked by uid 89); 18 Oct 2016 16:20:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00, KAM_ASCII_DIVIDERS, RP_MATCHES_RCVD, SPF_PASS autolearn=no version=3.3.2 spammy=employed, ZERO, 8217, botched X-HELO: mailapp01.imgtec.com Received: from mailapp02.imgtec.com (HELO mailapp01.imgtec.com) (217.156.133.132) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Oct 2016 16:20:05 +0000 Received: from HHMAIL03.hh.imgtec.org (unknown [10.44.0.21]) by Forcepoint Email with ESMTPS id 1AA4BEB64E4D3; Tue, 18 Oct 2016 17:19:58 +0100 (IST) Received: from HHMAIL01.hh.imgtec.org (10.100.10.19) by HHMAIL03.hh.imgtec.org (10.44.0.21) with Microsoft SMTP Server (TLS) id 14.3.294.0; Tue, 18 Oct 2016 17:20:00 +0100 Received: from [10.20.78.148] (10.20.78.148) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Tue, 18 Oct 2016 17:19:59 +0100 Date: Tue, 18 Oct 2016 17:19:50 +0100 From: "Maciej W. Rozycki" To: CC: Simon Marchi Subject: [PATCH] gdbarch: Use an anonymous union for target data in `gdbarch_info' Message-ID: User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 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: -- 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: . 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)