Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFC: A change to the way we initialize new gdbarches
@ 2006-10-04 23:10 Daniel Jacobowitz
  2006-10-05 20:06 ` Mark Kettenis
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Jacobowitz @ 2006-10-04 23:10 UTC (permalink / raw)
  To: gdb-patches

I've been working on code that makes some new uses of the gdbarch switching
machinery, based on information provided by (or guessed from) the target.
While doing so I ran into a snag, which it turns out I can demonstrate
clearly in current GDB.  Start an x86-64 GDB with a 64-bit file:

(gdb) show architecture
The target architecture is set automatically (currently i386:x86-64)
(gdb) set architecture i386
The target architecture is assumed to be i386
(gdb) set architecture auto
The target architecture is set automatically (currently i386)

Oops!  We should have gone back to the file's 64-bit architecture!  This
perhaps unintentional command has messed up the debug session and now we
have to know to set it back to i386:x86-64.  Similarly (but exercising
different code paths), you can do this with no file at all.  GDB will
default to i386, and if you set it to i386:x86-64, it won't ever go back. 
"set endian" has similar trouble.

This happens because we fill in the defaults for a new architecture from the
previously selected architecture.  These are derived properties, based on
the user's settings (at the time), the selected file (at the time), and so
forth.  This filling happens in two general places; in gdbarch_info_fill,
and in tdep files in the gdbarch_init method.

Here's a proposed approach to change that.  Note: it only compiles on i386
or x86-64.  Changing the other architectures will not be hard, but it will
be somewhat time consuming, so I wanted to get feedback on what I've done
so far.

Instead of inheriting derived properties (like "endianness") from the
previous architecture, only inherit primary properties (like "the endianness
of the user's file").  This lets gdbarch_update_p continue to work - you can
set just one thing in info, and then you'll get back a new architecture just
like the previous one, but with that property changed.  One way to see this
is that gdbarch_update_p is a "specialization" mechanism, and without my
patches we're fine as long as we are increasing the specialization - but
we can't decrease it.

I cached specific information from the bfd, rather than relying on the bfd
to stay open.  This lets us continue to reuse architectures, although we're
slightly pickier about what constitutes "similar enough for reuse" now.
Some architectures fetch their own additional information from the bfd;
I plan to add a new registration function that will let them record it
in gdbarch_info, using the currently unused gdbarch_tdep_info.  Right
now bfd_info is the only cached bit; in the future, properties read from
the target will be cached also.

Another advantage of this approach is that currently the "check
gdbarch_info, then check abfd, then check the last architecture" code is
duplicated in each tdep file.  After this, any information which we would
have gathered from the BFD will always be available in struct gdbarch_info
for future architectures - I find getting this right pretty
headache-inducing in the current setup and I expect the new version to be
much briefer.

What do you all think?  Is this sensible, or am I moving in the wrong
direction?

-- 
Daniel Jacobowitz
CodeSourcery

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	4 Oct 2006 22:56:06 -0000
@@ -460,14 +460,19 @@ 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;
+      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__,
@@ -531,7 +536,18 @@ gdbarch_from_bfd (bfd *abfd)
   struct gdbarch_info info;
 
   gdbarch_info_init (&info);
-  info.abfd = abfd;
+  if (abfd == NULL)
+    info.bfd_info.valid = -1;
+  else
+    {
+      info.bfd_info.valid = 1;
+      info.bfd_info.bfd_arch_info = bfd_get_arch_info (abfd);
+      info.bfd_info.byte_order = (bfd_big_endian (abfd) ? BFD_ENDIAN_BIG
+				   : bfd_little_endian (abfd) ? BFD_ENDIAN_LITTLE
+				   : BFD_ENDIAN_UNKNOWN);
+      info.bfd_info.osabi = gdbarch_lookup_osabi (abfd);
+    }
+
   return gdbarch_find_by_info (info);
 }
 
@@ -567,6 +583,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 +595,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 +609,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 +642,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 "
@@ -680,19 +699,27 @@ gdbarch_info_init (struct gdbarch_info *
 void
 gdbarch_info_fill (struct gdbarch *gdbarch, struct gdbarch_info *info)
 {
+  /* Use the previous architecture's bfd, or none.  FIXME: The correct
+     way to handle valid == -1 might be to do some of this in
+     gdbarch_info_init, and allow our callers to clear it again.  */
+  if (!info->bfd_info.valid)
+    info->bfd_info = gdbarch_bfd_info (gdbarch);
+  else if (info->bfd_info.valid == -1)
+    info->bfd_info.valid = 0;
+
   /* "(gdb) set architecture ...".  */
   if (info->bfd_arch_info == NULL
       && !target_architecture_auto
       && gdbarch != NULL)
     info->bfd_arch_info = gdbarch_bfd_arch_info (gdbarch);
   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);
+      && info->bfd_info.valid
+      && info->bfd_info.bfd_arch_info->arch != bfd_arch_unknown
+      && info->bfd_info.bfd_arch_info->arch != bfd_arch_obscure)
+    info->bfd_arch_info = info->bfd_info.bfd_arch_info;
+  /* 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
@@ -701,21 +728,21 @@ gdbarch_info_fill (struct gdbarch *gdbar
     info->byte_order = gdbarch_byte_order (gdbarch);
   /* 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);
+      && info->bfd_info.valid)
+    info->byte_order = info->bfd_info.byte_order;
+  /* 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);
+    info->osabi = gdbarch_osabi_selected ();
   if (info->osabi == GDB_OSABI_UNINITIALIZED
-      && gdbarch != NULL)
-    info->osabi = gdbarch_osabi (gdbarch);
+      && info->bfd_info.valid)
+    info->osabi = info->bfd_info.osabi;
+  if (info->osabi == GDB_OSABI_UNINITIALIZED
+      && !info->bfd_info.valid)
+    info->osabi = gdbarch_osabi_default ();
 
   /* Must have at least filled in the architecture.  */
   gdb_assert (info->bfd_arch_info != 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	4 Oct 2006 22:56:06 -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	4 Oct 2006 22:56:06 -0000
@@ -92,6 +92,7 @@ struct gdbarch
   const struct bfd_arch_info * bfd_arch_info;
   int byte_order;
   enum gdb_osabi osabi;
+  struct gdbarch_bfd_info bfd_info;
 
   /* target specific vector. */
   struct gdbarch_tdep *tdep;
@@ -250,6 +251,7 @@ struct gdbarch startup_gdbarch =
   &bfd_default_arch_struct,  /* bfd_arch_info */
   BFD_ENDIAN_BIG,  /* byte_order */
   GDB_OSABI_UNKNOWN,  /* osabi */
+  { 0 },  /* bfd_info */
   /* target specific vector and its dump routine */
   NULL, NULL,
   /*per-architecture data-pointers and swap regions */
@@ -394,6 +396,7 @@ gdbarch_alloc (const struct gdbarch_info
   current_gdbarch->bfd_arch_info = info->bfd_arch_info;
   current_gdbarch->byte_order = info->byte_order;
   current_gdbarch->osabi = info->osabi;
+  current_gdbarch->bfd_info = info->bfd_info;
 
   /* Force the explicit initialization of these. */
   current_gdbarch->short_bit = 2*TARGET_CHAR_BIT;
@@ -732,6 +735,9 @@ gdbarch_dump (struct gdbarch *current_gd
   fprintf_unfiltered (file,
                       "gdbarch_dump: bfd_arch_info = %s\n",
                       TARGET_ARCHITECTURE->printable_name);
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: bfd_info = %s\n",
+                      "TODO - Not dumpable");
 #ifdef TARGET_BFD_VMA_BIT
   fprintf_unfiltered (file,
                       "gdbarch_dump: TARGET_BFD_VMA_BIT # %s\n",
@@ -1647,6 +1653,15 @@ gdbarch_osabi (struct gdbarch *gdbarch)
   return gdbarch->osabi;
 }
 
+struct gdbarch_bfd_info
+gdbarch_bfd_info (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_bfd_info called\n");
+  return gdbarch->bfd_info;
+}
+
 int
 gdbarch_short_bit (struct gdbarch *gdbarch)
 {
@@ -3976,6 +3991,20 @@ gdbarch_list_lookup_by_info (struct gdba
 	continue;
       if (info->osabi != arches->gdbarch->osabi)
 	continue;
+      if (info->bfd_info.valid != arches->gdbarch->bfd_info.valid)
+	continue;
+      if (info->bfd_info.valid)
+	{
+	  if (info->bfd_info.bfd_arch_info
+	      != arches->gdbarch->bfd_info.bfd_arch_info)
+	    continue;
+	  if (info->bfd_info.byte_order
+	      != arches->gdbarch->bfd_info.byte_order)
+	    continue;
+	  if (info->bfd_info.osabi
+	      != arches->gdbarch->bfd_info.osabi)
+	    continue;
+	}
       return arches;
     }
   return NULL;
@@ -4020,9 +4049,12 @@ find_arch_by_info (struct gdbarch *old_g
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.osabi %d (%s)\n",
 			  info.osabi, gdbarch_osabi_name (info.osabi));
+#if 0
+      /* TODO: Add debugging for gdbarch_bfd_info.  */
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.abfd 0x%lx\n",
 			  (long) info.abfd);
+#endif
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.tdep_info 0x%lx\n",
 			  (long) info.tdep_info);
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	4 Oct 2006 22:56:06 -0000
@@ -83,6 +83,9 @@ extern enum gdb_osabi gdbarch_osabi (str
 #define TARGET_OSABI (gdbarch_osabi (current_gdbarch))
 #endif
 
+extern struct gdbarch_bfd_info gdbarch_bfd_info (struct gdbarch *gdbarch);
+/* set_gdbarch_bfd_info() - not applicable - pre-initialized. */
+
 
 /* The following are initialized by the target dependent code. */
 
@@ -1419,8 +1422,8 @@ extern struct gdbarch_tdep *gdbarch_tdep
    architecture; ARCHES which is a list of the previously created
    ``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
+   The INFO parameter is, as far as possible, pre-initialized with
+   information obtained from the current file or the previously selected
    architecture.
 
    The ARCHES parameter is a linked list (sorted most recently used)
@@ -1447,6 +1450,15 @@ struct gdbarch_list
   struct gdbarch_list *next;
 };
 
+struct gdbarch_bfd_info
+{
+  int valid;
+
+  const struct bfd_arch_info *bfd_arch_info;
+  int byte_order;
+  enum gdb_osabi osabi;
+};
+
 struct gdbarch_info
 {
   /* Use default: NULL (ZERO). */
@@ -1456,13 +1468,12 @@ struct gdbarch_info
   int byte_order;
 
   /* Use default: NULL (ZERO). */
-  bfd *abfd;
-
-  /* Use default: NULL (ZERO). */
   struct gdbarch_tdep_info *tdep_info;
 
   /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
   enum gdb_osabi osabi;
+
+  struct gdbarch_bfd_info bfd_info;
 };
 
 typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
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	4 Oct 2006 22:56:06 -0000
@@ -372,6 +372,8 @@ i:TARGET_ARCHITECTURE:const struct bfd_a
 i:TARGET_BYTE_ORDER:int:byte_order:::BFD_ENDIAN_BIG
 #
 i:TARGET_OSABI:enum gdb_osabi:osabi:::GDB_OSABI_UNKNOWN
+#
+i::struct gdbarch_bfd_info:bfd_info:::{ 0 }:::0:"TODO - Not dumpable"
 # Number of bits in a char or unsigned char for the target machine.
 # Just like CHAR_BIT in <limits.h> but describes the target machine.
 # v:TARGET_CHAR_BIT:int:char_bit::::8 * sizeof (char):8::0:
@@ -942,8 +944,8 @@ extern struct gdbarch_tdep *gdbarch_tdep
    architecture; ARCHES which is a list of the previously created
    \`\`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
+   The INFO parameter is, as far as possible, pre-initialized with
+   information obtained from the current file or the previously selected
    architecture.
 
    The ARCHES parameter is a linked list (sorted most recently used)
@@ -970,6 +972,15 @@ struct gdbarch_list
   struct gdbarch_list *next;
 };
 
+struct gdbarch_bfd_info
+{
+  int valid;
+
+  const struct bfd_arch_info *bfd_arch_info;
+  int byte_order;
+  enum gdb_osabi osabi;
+};
+
 struct gdbarch_info
 {
   /* Use default: NULL (ZERO). */
@@ -979,13 +990,12 @@ struct gdbarch_info
   int byte_order;
 
   /* Use default: NULL (ZERO). */
-  bfd *abfd;
-
-  /* Use default: NULL (ZERO). */
   struct gdbarch_tdep_info *tdep_info;
 
   /* Use default: GDB_OSABI_UNINITIALIZED (-1).  */
   enum gdb_osabi osabi;
+
+  struct gdbarch_bfd_info bfd_info;
 };
 
 typedef struct gdbarch *(gdbarch_init_ftype) (struct gdbarch_info info, struct gdbarch_list *arches);
@@ -2039,6 +2049,20 @@ gdbarch_list_lookup_by_info (struct gdba
 	continue;
       if (info->osabi != arches->gdbarch->osabi)
 	continue;
+      if (info->bfd_info.valid != arches->gdbarch->bfd_info.valid)
+	continue;
+      if (info->bfd_info.valid)
+	{
+	  if (info->bfd_info.bfd_arch_info
+	      != arches->gdbarch->bfd_info.bfd_arch_info)
+	    continue;
+	  if (info->bfd_info.byte_order
+	      != arches->gdbarch->bfd_info.byte_order)
+	    continue;
+	  if (info->bfd_info.osabi
+	      != arches->gdbarch->bfd_info.osabi)
+	    continue;
+	}
       return arches;
     }
   return NULL;
@@ -2083,9 +2107,12 @@ find_arch_by_info (struct gdbarch *old_g
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.osabi %d (%s)\n",
 			  info.osabi, gdbarch_osabi_name (info.osabi));
+#if 0
+      /* TODO: Add debugging for gdbarch_bfd_info.  */
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.abfd 0x%lx\n",
 			  (long) info.abfd);
+#endif
       fprintf_unfiltered (gdb_stdlog,
 			  "find_arch_by_info: info.tdep_info 0x%lx\n",
 			  (long) info.tdep_info);
Index: osabi.c
===================================================================
RCS file: /cvs/src/src/gdb/osabi.c,v
retrieving revision 1.37
diff -u -p -r1.37 osabi.c
--- osabi.c	10 Feb 2006 20:56:14 -0000	1.37
+++ osabi.c	4 Oct 2006 22:56:07 -0000
@@ -196,25 +196,32 @@ gdbarch_register_osabi_sniffer (enum bfd
 \f
 
 enum gdb_osabi
-gdbarch_lookup_osabi (bfd *abfd)
+gdbarch_osabi_selected (void)
 {
-  struct gdb_osabi_sniffer *sniffer;
-  enum gdb_osabi osabi, match;
-  int match_specific;
-
   /* If we aren't in "auto" mode, return the specified OS ABI.  */
   if (user_osabi_state == osabi_user)
     return user_selected_osabi;
 
+  return GDB_OSABI_UNINITIALIZED;
+}
+
+enum gdb_osabi
+gdbarch_osabi_default (void)
+{
   /* If we don't have a binary, return the default OS ABI (if set) or
      an inconclusive result (otherwise).  */
-  if (abfd == NULL) 
-    {
-      if (GDB_OSABI_DEFAULT != GDB_OSABI_UNKNOWN)
-	return GDB_OSABI_DEFAULT;
-      else
-	return GDB_OSABI_UNINITIALIZED;
-    }
+  if (GDB_OSABI_DEFAULT != GDB_OSABI_UNKNOWN)
+    return GDB_OSABI_DEFAULT;
+  else
+    return GDB_OSABI_UNINITIALIZED;
+}
+
+enum gdb_osabi
+gdbarch_lookup_osabi (bfd *abfd)
+{
+  struct gdb_osabi_sniffer *sniffer;
+  enum gdb_osabi osabi, match;
+  int match_specific;
 
   match = GDB_OSABI_UNKNOWN;
   match_specific = 0;
Index: osabi.h
===================================================================
RCS file: /cvs/src/src/gdb/osabi.h,v
retrieving revision 1.9
diff -u -p -r1.9 osabi.h
--- osabi.h	17 Dec 2005 22:34:01 -0000	1.9
+++ osabi.h	4 Oct 2006 22:56:07 -0000
@@ -38,6 +38,9 @@ void gdbarch_register_osabi (enum bfd_ar
                              void (*)(struct gdbarch_info,
 				      struct gdbarch *));
 
+enum gdb_osabi gdbarch_osabi_selected (void);
+enum gdb_osabi gdbarch_osabi_default (void);
+
 /* Lookup the OS ABI corresponding to the specified BFD.  */
 enum gdb_osabi gdbarch_lookup_osabi (bfd *);
 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RFC: A change to the way we initialize new gdbarches
  2006-10-04 23:10 RFC: A change to the way we initialize new gdbarches Daniel Jacobowitz
@ 2006-10-05 20:06 ` Mark Kettenis
  2006-10-05 20:25   ` Daniel Jacobowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Kettenis @ 2006-10-05 20:06 UTC (permalink / raw)
  To: drow; +Cc: gdb-patches

> Date: Wed, 4 Oct 2006 19:10:25 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> I've been working on code that makes some new uses of the gdbarch switching
> machinery, based on information provided by (or guessed from) the target.
> While doing so I ran into a snag, which it turns out I can demonstrate
> clearly in current GDB.  Start an x86-64 GDB with a 64-bit file:
> 
> (gdb) show architecture
> The target architecture is set automatically (currently i386:x86-64)
> (gdb) set architecture i386
> The target architecture is assumed to be i386
> (gdb) set architecture auto
> The target architecture is set automatically (currently i386)
> 
> Oops!  We should have gone back to the file's 64-bit architecture!  This
> perhaps unintentional command has messed up the debug session and now we
> have to know to set it back to i386:x86-64.  Similarly (but exercising
> different code paths), you can do this with no file at all.  GDB will
> default to i386, and if you set it to i386:x86-64, it won't ever go back. 
> "set endian" has similar trouble.

So what exactly should "set architecture auto" do?

I think we should set the architecture to the architecture of the
loaded file, or the default architecture if there is no loaded file.
Will there always be a meaningful default architecture?  I think the
answer is yes for all "hosted" targets.  For example if I configure
with --target==mips64-openbsd, the default endianness should be
big-endian.  But for embedded targets that's not always clear I think.

> This happens because we fill in the defaults for a new architecture from the
> previously selected architecture.  These are derived properties, based on
> the user's settings (at the time), the selected file (at the time), and so
> forth.  This filling happens in two general places; in gdbarch_info_fill,
> and in tdep files in the gdbarch_init method.
> 
> Here's a proposed approach to change that.  Note: it only compiles on i386
> or x86-64.  Changing the other architectures will not be hard, but it will
> be somewhat time consuming, so I wanted to get feedback on what I've done
> so far.

Whatever we decide, I think it is necessary to have a thorough look at
architectures that don't have a fixed endian-ness (like mips).

> Instead of inheriting derived properties (like "endianness") from the
> previous architecture, only inherit primary properties (like "the endianness
> of the user's file").

I wonder whether inheriting anything from the previous architecture is
a good idea at all.

> This lets gdbarch_update_p continue to work - you can set just one
> thing in info, and then you'll get back a new architecture just like
> the previous one, but with that property changed.  One way to see
> this is that gdbarch_update_p is a "specialization" mechanism, and
> without my patches we're fine as long as we are increasing the
> specialization - but we can't decrease it.
> 
> I cached specific information from the bfd, rather than relying on the bfd
> to stay open.  This lets us continue to reuse architectures, although we're
> slightly pickier about what constitutes "similar enough for reuse" now.
> Some architectures fetch their own additional information from the bfd;
> I plan to add a new registration function that will let them record it
> in gdbarch_info, using the currently unused gdbarch_tdep_info.  Right
> now bfd_info is the only cached bit; in the future, properties read from
> the target will be cached also.

You're thinking about caching things to avoid refetching the info from
the target isn't it?  But we should invalidate that info when we
disconnect from the target.

> Another advantage of this approach is that currently the "check
> gdbarch_info, then check abfd, then check the last architecture" code is
> duplicated in each tdep file.  After this, any information which we would
> have gathered from the BFD will always be available in struct gdbarch_info
> for future architectures - I find getting this right pretty
> headache-inducing in the current setup and I expect the new version to be
> much briefer.

That'd be good.

> What do you all think?  Is this sensible, or am I moving in the wrong
> direction?

I find this particular bit of code very hard to understand, and I'm
afraid I can't answer this question right now.  Hopefully your answers
to my questions will help me judging this better.

Mark


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RFC: A change to the way we initialize new gdbarches
  2006-10-05 20:06 ` Mark Kettenis
@ 2006-10-05 20:25   ` Daniel Jacobowitz
  2006-10-18 20:48     ` Daniel Jacobowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Jacobowitz @ 2006-10-05 20:25 UTC (permalink / raw)
  To: gdb-patches

On Thu, Oct 05, 2006 at 10:06:09PM +0200, Mark Kettenis wrote:
> So what exactly should "set architecture auto" do?
> 
> I think we should set the architecture to the architecture of the
> loaded file, or the default architecture if there is no loaded file.

I agree entirely; in fact, that's exactly the behavior I'm trying to
implement.  The code never used to consult the default architecture
after GDB startup.

> Will there always be a meaningful default architecture?  I think the
> answer is yes for all "hosted" targets.  For example if I configure
> with --target==mips64-openbsd, the default endianness should be
> big-endian.  But for embedded targets that's not always clear I think.

I think there's just as likely to be a valid default on embedded as
hosted targets.  Of course, it's just a default - I don't think we need
to jump through hoops to make GDB do something sensible if your system
has a different default.  Today, we pick up the default directly from
BFD; I think that's still sane.

> > This happens because we fill in the defaults for a new architecture from the
> > previously selected architecture.  These are derived properties, based on
> > the user's settings (at the time), the selected file (at the time), and so
> > forth.  This filling happens in two general places; in gdbarch_info_fill,
> > and in tdep files in the gdbarch_init method.
> > 
> > Here's a proposed approach to change that.  Note: it only compiles on i386
> > or x86-64.  Changing the other architectures will not be hard, but it will
> > be somewhat time consuming, so I wanted to get feedback on what I've done
> > so far.
> 
> Whatever we decide, I think it is necessary to have a thorough look at
> architectures that don't have a fixed endian-ness (like mips).

Definitely.  In fact, I've gotten sidetracked onto x86-64 by Jan's
recent posting; I was originally working on this logic for MIPS and
that's the next architecture I'd be looking at.

> > Instead of inheriting derived properties (like "endianness") from the
> > previous architecture, only inherit primary properties (like "the endianness
> > of the user's file").
> 
> I wonder whether inheriting anything from the previous architecture is
> a good idea at all.

In fact, there's an alternative.  We could keep it separate.  But I
suspect we still need to hold on to some global state; consider "set
endian" when there's a file loaded.  We want to ignore any autodetected
endianness, but if the file indicates 64-bit MIPS rather than 32-bit
MIPS, we don't want to lose track of that information.

"Indicates ARM rather than MIPS" is a flashier example but requires a
GDB which supports both.  Which we've been very close to for a while,
and I think I may make a final push to take us there soon.  I'm going
to have to do lots of gdbarch cleanups now anyway :-)

I thought about breaking up the two bits of state, "current properties"
and "current gdbarch".  But they're both troublesome for a hypothetical
future GDB which supports debugging more than one architecture at a
time, and the current properties bit would have the same problems as
the current gdbarch.

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?

> You're thinking about caching things to avoid refetching the info from
> the target isn't it?  But we should invalidate that info when we
> disconnect from the target.

There's two cachings going on.  One of them, the one I was talking
about here, is in today's GDB already: we try to reuse "struct gdbarch"
throughout a debugging session.  So we don't want to have to create a
new one just because the user said "file".

The other, in our local GDB branch, is caching of data read from the
target.  You're exactly right: we read it once, on connection, and make
sure not to read it again and to invalidate it when we disconnect.

It's a little more complicated: we sometimes refetch while remaining
connected.  The "architecture" debugged by a stub can change when it
switches from one executable to another, for instance if one of them
uses a kernel FPU emulator and so appears to have floating point
registers, and the other is compiled for soft FP and thus doesn't have
meaningful FP registers.  But that's just a detail.

> > What do you all think?  Is this sensible, or am I moving in the wrong
> > direction?
> 
> I find this particular bit of code very hard to understand, and I'm
> afraid I can't answer this question right now.  Hopefully your answers
> to my questions will help me judging this better.

Thanks for looking at it!  As you can tell from my floundering around
above, I find this stuff pretty confusing too; it takes twice as long
to grok as most bits of GDB.  I hope these changes will make it a
little clearer.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RFC: A change to the way we initialize new gdbarches
  2006-10-05 20:25   ` Daniel Jacobowitz
@ 2006-10-18 20:48     ` Daniel Jacobowitz
  2006-10-19 20:00       ` Joel Brobecker
  2006-11-09 19:50       ` [rfa/doc] " Daniel Jacobowitz
  0 siblings, 2 replies; 9+ messages in thread
From: Daniel Jacobowitz @ 2006-10-18 20:48 UTC (permalink / raw)
  To: gdb-patches

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RFC: A change to the way we initialize new gdbarches
  2006-10-18 20:48     ` Daniel Jacobowitz
@ 2006-10-19 20:00       ` Joel Brobecker
  2006-11-09 19:50       ` [rfa/doc] " Daniel Jacobowitz
  1 sibling, 0 replies; 9+ messages in thread
From: Joel Brobecker @ 2006-10-19 20:00 UTC (permalink / raw)
  To: gdb-patches

> 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.

FWIW, I had a look a this change. I like the approach.

-- 
Joel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [rfa/doc] Re: RFC: A change to the way we initialize new gdbarches
  2006-10-18 20:48     ` Daniel Jacobowitz
  2006-10-19 20:00       ` Joel Brobecker
@ 2006-11-09 19:50       ` Daniel Jacobowitz
  2006-11-09 21:23         ` Eli Zaretskii
  2006-11-10 15:33         ` Eli Zaretskii
  1 sibling, 2 replies; 9+ messages in thread
From: Daniel Jacobowitz @ 2006-11-09 19:50 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii

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  <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.

2006-11-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* 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
 \f
 /* 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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfa/doc] Re: RFC: A change to the way we initialize new gdbarches
  2006-11-09 19:50       ` [rfa/doc] " Daniel Jacobowitz
@ 2006-11-09 21:23         ` Eli Zaretskii
  2006-11-10 15:33         ` Eli Zaretskii
  1 sibling, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2006-11-09 21:23 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> Date: Thu, 9 Nov 2006 14:50:12 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> Eli, when you have a chance, could you take a look at the new gdbint
> chapter?

Will do.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfa/doc] Re: RFC: A change to the way we initialize new gdbarches
  2006-11-09 19:50       ` [rfa/doc] " Daniel Jacobowitz
  2006-11-09 21:23         ` Eli Zaretskii
@ 2006-11-10 15:33         ` Eli Zaretskii
  2006-11-10 16:56           ` Daniel Jacobowitz
  1 sibling, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2006-11-10 15:33 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> Date: Thu, 9 Nov 2006 14:50:12 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> 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

Thanks.  This patch is fine with me, except for the following minor
comment:

> +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

I think both `info' and `arches' should have a @var markup in the
@smallexample as well, to make them consistent with the following text
that describes them.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [rfa/doc] Re: RFC: A change to the way we initialize new gdbarches
  2006-11-10 15:33         ` Eli Zaretskii
@ 2006-11-10 16:56           ` Daniel Jacobowitz
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Jacobowitz @ 2006-11-10 16:56 UTC (permalink / raw)
  To: gdb-patches

On Fri, Nov 10, 2006 at 05:33:03PM +0200, Eli Zaretskii wrote:
> I think both `info' and `arches' should have a @var markup in the
> @smallexample as well, to make them consistent with the following text
> that describes them.

Thanks!  Retested on x86_64-pc-linux-gnu, and committed with that
improvement.

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-10  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.

2006-11-10  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdbint.texinfo (Target Architecture Definition): Add new
	Initializing a New Architecture section.

Index: gdb/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
--- gdb/arch-utils.c	17 Dec 2005 22:33:59 -0000	1.133
+++ gdb/arch-utils.c	10 Nov 2006 16:55:46 -0000
@@ -335,24 +335,7 @@ generic_instruction_nullified (struct gd
 \f
 /* 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: gdb/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
--- gdb/arch-utils.h	4 Oct 2006 20:14:44 -0000	1.80
+++ gdb/arch-utils.h	10 Nov 2006 16:55:46 -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: gdb/exec.c
===================================================================
RCS file: /cvs/src/src/gdb/exec.c,v
retrieving revision 1.62
diff -u -p -r1.62 exec.c
--- gdb/exec.c	24 Jul 2006 20:38:07 -0000	1.62
+++ gdb/exec.c	10 Nov 2006 16:55:46 -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: gdb/gdbarch.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.c,v
retrieving revision 1.330
diff -u -p -r1.330 gdbarch.c
--- gdb/gdbarch.c	16 Jul 2006 11:03:41 -0000	1.330
+++ gdb/gdbarch.c	10 Nov 2006 16:55:46 -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: gdb/gdbarch.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.h,v
retrieving revision 1.286
diff -u -p -r1.286 gdbarch.h
--- gdb/gdbarch.h	16 Jul 2006 11:03:41 -0000	1.286
+++ gdb/gdbarch.h	10 Nov 2006 16:55:46 -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: gdb/gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.366
diff -u -p -r1.366 gdbarch.sh
--- gdb/gdbarch.sh	16 Jul 2006 11:03:41 -0000	1.366
+++ gdb/gdbarch.sh	10 Nov 2006 16:55:47 -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: gdb/remote-sim.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-sim.c,v
retrieving revision 1.55
diff -u -p -r1.55 remote-sim.c
--- gdb/remote-sim.c	17 Oct 2006 21:55:23 -0000	1.55
+++ gdb/remote-sim.c	10 Nov 2006 16:55:47 -0000
@@ -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: gdb/doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.246
diff -u -p -r1.246 gdbint.texinfo
--- gdb/doc/gdbint.texinfo	17 Oct 2006 21:55:24 -0000	1.246
+++ gdb/doc/gdbint.texinfo	10 Nov 2006 16:55:48 -0000
@@ -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 @var{info},
+                         struct gdbarch_list *@var{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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2006-11-10 16:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-04 23:10 RFC: A change to the way we initialize new gdbarches Daniel Jacobowitz
2006-10-05 20:06 ` Mark Kettenis
2006-10-05 20:25   ` Daniel Jacobowitz
2006-10-18 20:48     ` Daniel Jacobowitz
2006-10-19 20:00       ` Joel Brobecker
2006-11-09 19:50       ` [rfa/doc] " Daniel Jacobowitz
2006-11-09 21:23         ` Eli Zaretskii
2006-11-10 15:33         ` Eli Zaretskii
2006-11-10 16:56           ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox