From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13311 invoked by alias); 9 Nov 2006 19:50:43 -0000 Received: (qmail 12974 invoked by uid 22791); 9 Nov 2006 19:50:28 -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; Thu, 09 Nov 2006 19:50:16 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1GiFue-0004Hm-Vk; Thu, 09 Nov 2006 14:50:13 -0500 Date: Thu, 09 Nov 2006 19:50:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Cc: Eli Zaretskii Subject: [rfa/doc] Re: RFC: A change to the way we initialize new gdbarches Message-ID: <20061109195012.GB14732@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org, Eli Zaretskii References: <20061004231025.GA9687@nevyn.them.org> <200610052006.k95K69UK024656@elgar.sibelius.xs4all.nl> <20061005202536.GA10876@nevyn.them.org> <20061018204802.GA20757@nevyn.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20061018204802.GA20757@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-11/txt/msg00052.txt.bz2 On Wed, Oct 18, 2006 at 04:48:02PM -0400, Daniel Jacobowitz wrote: > 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 :-) And I've now finished adapting all of this work to HEAD, and I'll be submitting the foundations for it momentarily - I won't say how long that's taken me... Joel liked the new approach, and no one has jumped up and shouted that I'm crazy, which is gratifying in its own way. So I plan to check this in, but not without some documentation. I've added a bit of documentation of the gdbarch initialization process to gdbint.texinfo in the attached version. The highlight of this change is that architectures no longer need to check the first item on their "arches" argument to fill in their defaults. Continuing to do so is a bug, but not a particularly important one; those cases will now be basically dead, since in usual usage info.exec_bfd will be set and take precedence. I'm not planning to go through and fix every existing architecture, but I'll fix them as I have occasion to work on them, and I encourage others to also. Eli, when you have a chance, could you take a look at the new gdbint chapter? After that, I'll commit this. I've tested it variously on GNU/Linux for x86_64, ARM, MIPS, and MIPS64. -- Daniel Jacobowitz CodeSourcery 2006-11-09 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. 2006-11-09 Daniel Jacobowitz * gdbint.texinfo (Target Architecture Definition): Add new Initializing a New Architecture section. --- gdb/arch-utils.c | 136 ++++++++++++++++++++++++------------------------- gdb/arch-utils.h | 11 +-- gdb/doc/gdbint.texinfo | 35 ++++++++++++ gdb/exec.c | 2 gdb/gdbarch.c | 10 +-- gdb/gdbarch.h | 3 - gdb/gdbarch.sh | 13 ++-- gdb/remote-sim.c | 2 8 files changed, 122 insertions(+), 90 deletions(-) Index: src/gdb/arch-utils.c =================================================================== --- src.orig/gdb/arch-utils.c 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/arch-utils.c 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/arch-utils.h =================================================================== --- src.orig/gdb/arch-utils.h 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/arch-utils.h 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/exec.c =================================================================== --- src.orig/gdb/exec.c 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/exec.c 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/gdbarch.c =================================================================== --- src.orig/gdb/gdbarch.c 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/gdbarch.c 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/gdbarch.h =================================================================== --- src.orig/gdb/gdbarch.h 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/gdbarch.h 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/gdbarch.sh =================================================================== --- src.orig/gdb/gdbarch.sh 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/gdbarch.sh 2006-11-09 14:14:33.000000000 -0500 @@ -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: src/gdb/remote-sim.c =================================================================== --- src.orig/gdb/remote-sim.c 2006-11-09 14:07:12.000000000 -0500 +++ src/gdb/remote-sim.c 2006-11-09 14:14:33.000000000 -0500 @@ -506,7 +506,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"); Index: src/gdb/doc/gdbint.texinfo =================================================================== --- src.orig/gdb/doc/gdbint.texinfo 2006-11-09 14:14:55.000000000 -0500 +++ src/gdb/doc/gdbint.texinfo 2006-11-09 14:33:05.000000000 -0500 @@ -2722,6 +2722,41 @@ architecture, a warning will be issued a with the defaults already established for @var{gdbarch}. @end deftypefun +@section Initializing a New Architecture + +Each @code{gdbarch} is associated with a single @sc{bfd} architecture, +via a @code{bfd_arch_@var{arch}} constant. The @code{gdbarch} is +registered by a call to @code{register_gdbarch_init}, usually from +the file's @code{_initialize_@var{filename}} routine, which will +be automatically called during @value{GDBN} startup. The arguments +are a @sc{bfd} architecture constant and an initialization function. + +The initialization function has this type: + +@smallexample +static struct gdbarch * +@var{arch}_gdbarch_init (struct gdbarch_info info, + struct gdbarch_list *arches) +@end smallexample + +The @var{info} argument contains parameters used to select the correct +architecture, and @var{arches} is a list of architectures which +have already been created with the same @code{bfd_arch_@var{arch}} +value. + +The initialization function should first make sure that @var{info} +is acceptable, and return @code{NULL} if it is not. Then, it should +search through @var{arches} for an exact match to @var{info}, and +return one if found. Lastly, if no exact match was found, it should +create a new architecture based on @var{info} and return it. + +Only information in @var{info} should be used to choose the new +architecture. Historically, @var{info} could be sparse, and +defaults would be collected from the first element on @var{arches}. +However, @value{GDBN} now fills in @var{info} more thoroughly, +so new @code{gdbarch} initialization functions should not take +defaults from @var{arches}. + @section Registers and Memory @value{GDBN}'s model of the target machine is rather simple.