From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1762 invoked by alias); 4 Oct 2006 23:10:36 -0000 Received: (qmail 1745 invoked by uid 22791); 4 Oct 2006 23:10:33 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Wed, 04 Oct 2006 23:10:28 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1GVFsf-0002pb-Tw for gdb-patches@sourceware.org; Wed, 04 Oct 2006 19:10:26 -0400 Date: Wed, 04 Oct 2006 23:10:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: RFC: A change to the way we initialize new gdbarches Message-ID: <20061004231025.GA9687@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.13 (2006-08-11) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-10/txt/msg00028.txt.bz2 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 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 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 *);