This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: A change to the way we initialize new gdbarches


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  <dan@codesourcery.com>

	* 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");


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]