From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16623 invoked by alias); 18 Oct 2006 20:48:12 -0000 Received: (qmail 16611 invoked by uid 22791); 18 Oct 2006 20:48:10 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Wed, 18 Oct 2006 20:48:04 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1GaIKY-0005Uy-5A for gdb-patches@sourceware.org; Wed, 18 Oct 2006 16:48:02 -0400 Date: Wed, 18 Oct 2006 20:48:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: Re: RFC: A change to the way we initialize new gdbarches Message-ID: <20061018204802.GA20757@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org References: <20061004231025.GA9687@nevyn.them.org> <200610052006.k95K69UK024656@elgar.sibelius.xs4all.nl> <20061005202536.GA10876@nevyn.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20061005202536.GA10876@nevyn.them.org> User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-10/txt/msg00235.txt.bz2 These changes are a prerequisite for one of the big projects I've been working on (self-describing targets), and I'm pretty ashamed of how long I've been working on it without getting more of it merged to HEAD, so I'm back to straighten out this initialization order issue :-) On Thu, Oct 05, 2006 at 04:25:36PM -0400, Daniel Jacobowitz wrote: > But thinking about it more today, maybe I got this backwards. We've > already got a global exec_bfd. We could check that every time we > updated the architecture. Instead of the current gdbarch_info_init and > gdbarch_info_fill, we'd have a single function which recomputed all the > interesting properties from all of the available sources, and call it > whenever one of them changed. That function would have to look at: > > - The global override variables (set endian, set architecture, set > osabi). > - The current executable file. > - The current target, for target-fetched information. > > What do you think? I'll tell you what I think: I think it's much clearer. This patch revamps the core code so that we never inherit from one gdbarch to the next. The major change is that info.abfd only used to be filled in when selecting a file; now, gdbarch_info_fill sets it from exec_bfd. This means that most of the former guessing and inheriting is no longer necessary. If we previously derived a parameter from the executable file, we can do so again. This patch is complete and useful on its own. A nice followup would be to go through every gdbarch_init function and look for blocks where a setting is inherited from the previously selected architecture - which used to be correct and necessary, though complex - and delete it. Blocks like this one, in mips-tdep.c: if (info.abfd && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour) elf_flags = elf_elfheader (info.abfd)->e_flags; else if (arches != NULL) elf_flags = gdbarch_tdep (arches->gdbarch)->elf_flags; else elf_flags = 0; The only way arches->tdep->elf_flags would have gotten set is from a previous info.abfd, and if it's still valid it will be set again, so the middle clause of the if is now redundant. MIPS has three occurances of this pattern; most platforms have at least one. Comments welcome! I believe this meets the goals I was originally trying to accomplish, so for now, I will work on the next parts of the puzzle on a tree with this patch already applied. -- Daniel Jacobowitz CodeSourcery 2006-10-18 Daniel Jacobowitz * arch-utils.c (target_byte_order_user): Renamed from target_byte_order. (target_byte_order_auto, selected_byte_order): Removed. (show_endian): Check target_byte_order_user. (set_endian): Always update the architecture. Set target_byte_order_user after success. (target_architecture_auto): Removed. (target_architecture_user): New. (selected_architecture_name, show_architecture): Check it. (set_architecture): Set target_architecture_user after success. (gdbarch_from_bfd): Check the argument. (default_byte_order): New. (initialize_current_architecture): Set the global default architecture and endianness. (gdbarch_info_fill): Remove GDBARCH argument. Do not check the previous architecture. Use exec_bfd, global selected architecture and endianness, and global defaults. * arch-utils.h (selected_byte_order): Remove prototype. (gdbarch_info_fill): Update. * exec.c (exec_file_attach): Update the architecture after removing the current file. * gdbarch.sh: Update comments. (find_arch_by_info): Remove OLD_GDBARCH argument. Update call to gdbarch_info_fill. (gdbarch_find_by_info): Update call to find_arch_by_info. * gdbarch.h, gdbarch.c: Regenerated. * remote-sim.c (gdbsim_open): Use TARGET_BYTE_ORDER. Index: arch-utils.c =================================================================== RCS file: /cvs/src/src/gdb/arch-utils.c,v retrieving revision 1.133 diff -u -p -r1.133 arch-utils.c --- arch-utils.c 17 Dec 2005 22:33:59 -0000 1.133 +++ arch-utils.c 18 Oct 2006 20:23:56 -0000 @@ -335,24 +335,7 @@ generic_instruction_nullified (struct gd /* Functions to manipulate the endianness of the target. */ -/* ``target_byte_order'' is only used when non- multi-arch. - Multi-arch targets obtain the current byte order using the - TARGET_BYTE_ORDER gdbarch method. - - The choice of initial value is entirely arbitrary. During startup, - the function initialize_current_architecture() updates this value - based on default byte-order information extracted from BFD. */ -static int target_byte_order = BFD_ENDIAN_BIG; -static int target_byte_order_auto = 1; - -enum bfd_endian -selected_byte_order (void) -{ - if (target_byte_order_auto) - return BFD_ENDIAN_UNKNOWN; - else - return target_byte_order; -} +static int target_byte_order_user = BFD_ENDIAN_UNKNOWN; static const char endian_big[] = "big"; static const char endian_little[] = "little"; @@ -372,7 +355,7 @@ static void show_endian (struct ui_file *file, int from_tty, struct cmd_list_element *c, const char *value) { - if (target_byte_order_auto) + if (target_byte_order_user != BFD_ENDIAN_UNKNOWN) if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) fprintf_unfiltered (file, _("The target endianness is set automatically " "(currently big endian)\n")); @@ -391,31 +374,37 @@ show_endian (struct ui_file *file, int f static void set_endian (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_auto = 1; + target_byte_order_user = BFD_ENDIAN_UNKNOWN; + if (! gdbarch_update_p (info)) + internal_error (__FILE__, __LINE__, + _("set_endian: architecture update failed")); } else if (set_endian_string == endian_little) { - struct gdbarch_info info; - target_byte_order_auto = 0; - gdbarch_info_init (&info); info.byte_order = BFD_ENDIAN_LITTLE; if (! gdbarch_update_p (info)) printf_unfiltered (_("Little endian target not supported by GDB\n")); + else + target_byte_order_user = BFD_ENDIAN_LITTLE; } else if (set_endian_string == endian_big) { - struct gdbarch_info info; - target_byte_order_auto = 0; - gdbarch_info_init (&info); info.byte_order = BFD_ENDIAN_BIG; if (! gdbarch_update_p (info)) printf_unfiltered (_("Big endian target not supported by GDB\n")); + else + target_byte_order_user = BFD_ENDIAN_BIG; } else internal_error (__FILE__, __LINE__, _("set_endian: bad value")); + show_endian (gdb_stdout, from_tty, NULL, NULL); } @@ -423,14 +412,14 @@ set_endian (char *ignore_args, int from_ enum set_arch { set_arch_auto, set_arch_manual }; -static int target_architecture_auto = 1; +static const struct bfd_arch_info *target_architecture_user; static const char *set_architecture_string; const char * selected_architecture_name (void) { - if (target_architecture_auto) + if (target_architecture_user == NULL) return NULL; else return set_architecture_string; @@ -445,7 +434,7 @@ show_architecture (struct ui_file *file, { const char *arch; arch = TARGET_ARCHITECTURE->printable_name; - if (target_architecture_auto) + if (target_architecture_user == NULL) fprintf_filtered (file, _("\ The target architecture is set automatically (currently %s)\n"), arch); else @@ -460,20 +449,25 @@ The target architecture is assumed to be static void set_architecture (char *ignore_args, int from_tty, struct cmd_list_element *c) { + struct gdbarch_info info; + + gdbarch_info_init (&info); + if (strcmp (set_architecture_string, "auto") == 0) { - target_architecture_auto = 1; + target_architecture_user = NULL; + if (!gdbarch_update_p (info)) + internal_error (__FILE__, __LINE__, + _("could not select an architecture automatically")); } else { - struct gdbarch_info info; - gdbarch_info_init (&info); info.bfd_arch_info = bfd_scan_arch (set_architecture_string); if (info.bfd_arch_info == NULL) internal_error (__FILE__, __LINE__, _("set_architecture: bfd_scan_arch failed")); if (gdbarch_update_p (info)) - target_architecture_auto = 0; + target_architecture_user = info.bfd_arch_info; else printf_unfiltered (_("Architecture `%s' not recognized.\n"), set_architecture_string); @@ -530,6 +524,13 @@ gdbarch_from_bfd (bfd *abfd) struct gdbarch *new_gdbarch; struct gdbarch_info info; + /* If we call gdbarch_find_by_info without filling in info.abfd, + then it will use the global exec_bfd. That's fine if we don't + have one of those either. And that's the only time we should + reach here with a NULL ABFD argument - when we are discarding + the executable. */ + gdb_assert (abfd != NULL || exec_bfd == NULL); + gdbarch_info_init (&info); info.abfd = abfd; return gdbarch_find_by_info (info); @@ -567,6 +568,8 @@ static const bfd_target *default_bfd_vec static const bfd_target *default_bfd_vec; #endif +static int default_byte_order = BFD_ENDIAN_UNKNOWN; + void initialize_current_architecture (void) { @@ -577,10 +580,7 @@ initialize_current_architecture (void) gdbarch_info_init (&info); /* Find a default architecture. */ - if (info.bfd_arch_info == NULL - && default_bfd_arch != NULL) - info.bfd_arch_info = default_bfd_arch; - if (info.bfd_arch_info == NULL) + if (default_bfd_arch == NULL) { /* Choose the architecture by taking the first one alphabetically. */ @@ -594,30 +594,32 @@ initialize_current_architecture (void) if (chosen == NULL) internal_error (__FILE__, __LINE__, _("initialize_current_architecture: No arch")); - info.bfd_arch_info = bfd_scan_arch (chosen); - if (info.bfd_arch_info == NULL) + default_bfd_arch = bfd_scan_arch (chosen); + if (default_bfd_arch == NULL) internal_error (__FILE__, __LINE__, _("initialize_current_architecture: Arch not found")); } + info.bfd_arch_info = default_bfd_arch; + /* Take several guesses at a byte order. */ - if (info.byte_order == BFD_ENDIAN_UNKNOWN + if (default_byte_order == BFD_ENDIAN_UNKNOWN && default_bfd_vec != NULL) { /* Extract BFD's default vector's byte order. */ switch (default_bfd_vec->byteorder) { case BFD_ENDIAN_BIG: - info.byte_order = BFD_ENDIAN_BIG; + default_byte_order = BFD_ENDIAN_BIG; break; case BFD_ENDIAN_LITTLE: - info.byte_order = BFD_ENDIAN_LITTLE; + default_byte_order = BFD_ENDIAN_LITTLE; break; default: break; } } - if (info.byte_order == BFD_ENDIAN_UNKNOWN) + if (default_byte_order == BFD_ENDIAN_UNKNOWN) { /* look for ``*el-*'' in the target name. */ const char *chp; @@ -625,14 +627,16 @@ initialize_current_architecture (void) if (chp != NULL && chp - 2 >= target_name && strncmp (chp - 2, "el", 2) == 0) - info.byte_order = BFD_ENDIAN_LITTLE; + default_byte_order = BFD_ENDIAN_LITTLE; } - if (info.byte_order == BFD_ENDIAN_UNKNOWN) + if (default_byte_order == BFD_ENDIAN_UNKNOWN) { /* Wire it to big-endian!!! */ - info.byte_order = BFD_ENDIAN_BIG; + default_byte_order = BFD_ENDIAN_BIG; } + info.byte_order = default_byte_order; + if (! gdbarch_update_p (info)) internal_error (__FILE__, __LINE__, _("initialize_current_architecture: Selection of " @@ -674,48 +678,46 @@ gdbarch_info_init (struct gdbarch_info * } /* Similar to init, but this time fill in the blanks. Information is - obtained from the specified architecture, global "set ..." options, - and explicitly initialized INFO fields. */ + obtained from the global "set ..." options and explicitly + initialized INFO fields. */ void -gdbarch_info_fill (struct gdbarch *gdbarch, struct gdbarch_info *info) +gdbarch_info_fill (struct gdbarch_info *info) { + /* Check for the current file. */ + if (info->abfd == NULL) + info->abfd = exec_bfd; + /* "(gdb) set architecture ...". */ if (info->bfd_arch_info == NULL - && !target_architecture_auto - && gdbarch != NULL) - info->bfd_arch_info = gdbarch_bfd_arch_info (gdbarch); + && target_architecture_user) + info->bfd_arch_info = target_architecture_user; if (info->bfd_arch_info == NULL && info->abfd != NULL && bfd_get_arch (info->abfd) != bfd_arch_unknown && bfd_get_arch (info->abfd) != bfd_arch_obscure) info->bfd_arch_info = bfd_get_arch_info (info->abfd); - if (info->bfd_arch_info == NULL - && gdbarch != NULL) - info->bfd_arch_info = gdbarch_bfd_arch_info (gdbarch); + /* From the default. */ + if (info->bfd_arch_info == NULL) + info->bfd_arch_info = default_bfd_arch; /* "(gdb) set byte-order ...". */ if (info->byte_order == BFD_ENDIAN_UNKNOWN - && !target_byte_order_auto - && gdbarch != NULL) - info->byte_order = gdbarch_byte_order (gdbarch); + && target_byte_order_user != BFD_ENDIAN_UNKNOWN) + info->byte_order = target_byte_order_user; /* From the INFO struct. */ if (info->byte_order == BFD_ENDIAN_UNKNOWN && info->abfd != NULL) info->byte_order = (bfd_big_endian (info->abfd) ? BFD_ENDIAN_BIG - : bfd_little_endian (info->abfd) ? BFD_ENDIAN_LITTLE - : BFD_ENDIAN_UNKNOWN); - /* From the current target. */ - if (info->byte_order == BFD_ENDIAN_UNKNOWN - && gdbarch != NULL) - info->byte_order = gdbarch_byte_order (gdbarch); + : bfd_little_endian (info->abfd) ? BFD_ENDIAN_LITTLE + : BFD_ENDIAN_UNKNOWN); + /* From the default. */ + if (info->byte_order == BFD_ENDIAN_UNKNOWN) + info->byte_order = default_byte_order; /* "(gdb) set osabi ...". Handled by gdbarch_lookup_osabi. */ if (info->osabi == GDB_OSABI_UNINITIALIZED) info->osabi = gdbarch_lookup_osabi (info->abfd); - if (info->osabi == GDB_OSABI_UNINITIALIZED - && gdbarch != NULL) - info->osabi = gdbarch_osabi (gdbarch); /* Must have at least filled in the architecture. */ gdb_assert (info->bfd_arch_info != NULL); Index: arch-utils.h =================================================================== RCS file: /cvs/src/src/gdb/arch-utils.h,v retrieving revision 1.80 diff -u -p -r1.80 arch-utils.h --- arch-utils.h 4 Oct 2006 20:14:44 -0000 1.80 +++ arch-utils.h 18 Oct 2006 20:23:56 -0000 @@ -126,10 +126,6 @@ extern int generic_instruction_nullified extern int legacy_register_sim_regno (int regnum); -/* Return the selected byte order, or BFD_ENDIAN_UNKNOWN if no byte - order was explicitly selected. */ -extern enum bfd_endian selected_byte_order (void); - /* Return the selected architecture's name, or NULL if no architecture was explicitly selected. */ extern const char *selected_architecture_name (void); @@ -141,10 +137,9 @@ extern const char *selected_architecture extern void gdbarch_info_init (struct gdbarch_info *info); /* Similar to init, but this time fill in the blanks. Information is - obtained from the specified architecture, global "set ..." options, - and explicitly initialized INFO fields. */ -extern void gdbarch_info_fill (struct gdbarch *gdbarch, - struct gdbarch_info *info); + obtained from the global "set ..." options and explicitly + initialized INFO fields. */ +extern void gdbarch_info_fill (struct gdbarch_info *info); /* Return the architecture for ABFD. If no suitable architecture could be find, return NULL. */ Index: exec.c =================================================================== RCS file: /cvs/src/src/gdb/exec.c,v retrieving revision 1.62 diff -u -p -r1.62 exec.c --- exec.c 24 Jul 2006 20:38:07 -0000 1.62 +++ exec.c 18 Oct 2006 20:23:56 -0000 @@ -188,6 +188,8 @@ exec_file_attach (char *filename, int fr { if (from_tty) printf_unfiltered (_("No executable file now.\n")); + + set_gdbarch_from_file (NULL); } else { Index: gdbarch.c =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.c,v retrieving revision 1.330 diff -u -p -r1.330 gdbarch.c --- gdbarch.c 16 Jul 2006 11:03:41 -0000 1.330 +++ gdbarch.c 18 Oct 2006 20:23:56 -0000 @@ -3987,7 +3987,7 @@ gdbarch_list_lookup_by_info (struct gdba that there is no current architecture. */ static struct gdbarch * -find_arch_by_info (struct gdbarch *old_gdbarch, struct gdbarch_info info) +find_arch_by_info (struct gdbarch_info info) { struct gdbarch *new_gdbarch; struct gdbarch_registration *rego; @@ -3997,9 +3997,9 @@ find_arch_by_info (struct gdbarch *old_g gdb_assert (current_gdbarch == NULL); /* Fill in missing parts of the INFO struct using a number of - sources: "set ..."; INFOabfd supplied; and the existing - architecture. */ - gdbarch_info_fill (old_gdbarch, &info); + sources: "set ..."; INFOabfd supplied; and the global + defaults. */ + gdbarch_info_fill (&info); /* Must have found some sort of architecture. */ gdb_assert (info.bfd_arch_info != NULL); @@ -4130,7 +4130,7 @@ gdbarch_find_by_info (struct gdbarch_inf struct gdbarch *old_gdbarch = current_gdbarch_swap_out_hack (); /* Find the specified architecture. */ - struct gdbarch *new_gdbarch = find_arch_by_info (old_gdbarch, info); + struct gdbarch *new_gdbarch = find_arch_by_info (info); /* Restore the existing architecture. */ gdb_assert (current_gdbarch == NULL); Index: gdbarch.h =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.h,v retrieving revision 1.286 diff -u -p -r1.286 gdbarch.h --- gdbarch.h 16 Jul 2006 11:03:41 -0000 1.286 +++ gdbarch.h 18 Oct 2006 20:23:57 -0000 @@ -1420,8 +1420,7 @@ extern struct gdbarch_tdep *gdbarch_tdep ``struct gdbarch'' for this architecture. The INFO parameter is, as far as possible, be pre-initialized with - information obtained from INFO.ABFD or the previously selected - architecture. + information obtained from INFO.ABFD or the global defaults. The ARCHES parameter is a linked list (sorted most recently used) of all the previously created architures for this architecture Index: gdbarch.sh =================================================================== RCS file: /cvs/src/src/gdb/gdbarch.sh,v retrieving revision 1.366 diff -u -p -r1.366 gdbarch.sh --- gdbarch.sh 16 Jul 2006 11:03:41 -0000 1.366 +++ gdbarch.sh 18 Oct 2006 20:23:57 -0000 @@ -943,8 +943,7 @@ extern struct gdbarch_tdep *gdbarch_tdep \`\`struct gdbarch'' for this architecture. The INFO parameter is, as far as possible, be pre-initialized with - information obtained from INFO.ABFD or the previously selected - architecture. + information obtained from INFO.ABFD or the global defaults. The ARCHES parameter is a linked list (sorted most recently used) of all the previously created architures for this architecture @@ -2050,7 +2049,7 @@ gdbarch_list_lookup_by_info (struct gdba that there is no current architecture. */ static struct gdbarch * -find_arch_by_info (struct gdbarch *old_gdbarch, struct gdbarch_info info) +find_arch_by_info (struct gdbarch_info info) { struct gdbarch *new_gdbarch; struct gdbarch_registration *rego; @@ -2060,9 +2059,9 @@ find_arch_by_info (struct gdbarch *old_g gdb_assert (current_gdbarch == NULL); /* Fill in missing parts of the INFO struct using a number of - sources: "set ..."; INFOabfd supplied; and the existing - architecture. */ - gdbarch_info_fill (old_gdbarch, &info); + sources: "set ..."; INFOabfd supplied; and the global + defaults. */ + gdbarch_info_fill (&info); /* Must have found some sort of architecture. */ gdb_assert (info.bfd_arch_info != NULL); @@ -2193,7 +2192,7 @@ gdbarch_find_by_info (struct gdbarch_inf struct gdbarch *old_gdbarch = current_gdbarch_swap_out_hack (); /* Find the specified architecture. */ - struct gdbarch *new_gdbarch = find_arch_by_info (old_gdbarch, info); + struct gdbarch *new_gdbarch = find_arch_by_info (info); /* Restore the existing architecture. */ gdb_assert (current_gdbarch == NULL); Index: remote-sim.c =================================================================== RCS file: /cvs/src/src/gdb/remote-sim.c,v retrieving revision 1.54 diff -u -p -r1.54 remote-sim.c --- remote-sim.c 18 Apr 2006 19:20:06 -0000 1.54 +++ remote-sim.c 18 Oct 2006 20:23:57 -0000 @@ -507,7 +507,7 @@ gdbsim_open (char *args, int from_tty) strcpy (arg_buf, "gdbsim"); /* 7 */ /* Specify the byte order for the target when it is both selectable and explicitly specified by the user (not auto detected). */ - switch (selected_byte_order ()) + switch (TARGET_BYTE_ORDER) { case BFD_ENDIAN_BIG: strcat (arg_buf, " -E big");