From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4833 invoked by alias); 20 Apr 2006 14:40:20 -0000 Received: (qmail 4822 invoked by uid 22791); 20 Apr 2006 14:40:19 -0000 X-Spam-Check-By: sourceware.org Received: from smtp111.sbc.mail.re2.yahoo.com (HELO smtp111.sbc.mail.re2.yahoo.com) (68.142.229.94) by sourceware.org (qpsmtpd/0.31) with SMTP; Thu, 20 Apr 2006 14:40:16 +0000 Received: (qmail 31216 invoked from network); 20 Apr 2006 14:40:15 -0000 Received: from unknown (HELO lucon.org) (hjjean@sbcglobal.net@75.0.171.244 with login) by smtp111.sbc.mail.re2.yahoo.com with SMTP; 20 Apr 2006 14:40:14 -0000 Received: by lucon.org (Postfix, from userid 1000) id 07A5F64034; Thu, 20 Apr 2006 07:40:13 -0700 (PDT) Date: Thu, 20 Apr 2006 16:29:00 -0000 From: "H. J. Lu" To: binutils@sources.redhat.com Subject: Re: PATCH: Should ld cache bfd_check_format return? Message-ID: <20060420144012.GA4523@lucon.org> References: <20060419142124.GA24515@lucon.org> <20060419204444.GA26952@lucon.org> <20060419234800.GC27430@bubble.grove.modra.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20060419234800.GC27430@bubble.grove.modra.org> User-Agent: Mutt/1.4.2.1i Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2006-04/txt/msg00285.txt.bz2 On Thu, Apr 20, 2006 at 09:18:00AM +0930, Alan Modra wrote: > On Wed, Apr 19, 2006 at 01:44:44PM -0700, H. J. Lu wrote: > > On Wed, Apr 19, 2006 at 07:21:24AM -0700, H. J. Lu wrote: > > > load_symbols does > > > > > > ldfile_open_file (entry); > > > > > > if (! bfd_check_format (entry->the_bfd, bfd_archive) > > > && ! bfd_check_format_matches (entry->the_bfd, bfd_object, &matching)) > > > > > > ldfile_open_file also calls bfd_check_format. Should ld cache > > > bfd_check_format return? > > > > > > > This patch caches bfd_check_format return. > > We already cache in bfd->format. > But we only cache the postive result. We will do the whole check for bfd_archive over and over on a bfd_object file. This patch also caches the negative result. I also clean up the linker a little bit to make it easier to read. BTW, I changed (unsigned int) abfd->format >= (unsigned int) bfd_type_end) to (unsigned int) format >= (unsigned int) bfd_type_end abfd->format will always be valid. We never put garbage on it. H.J. ---- bfd/ 2006-04-20 H.J. Lu * bfd-in.h (bfd_format): Add bfd_object_mask and bfd_type_max. (bfd_format_negative_bit): New. * bfd-in2.h: Regenerated. * format.c (bfd_check_format_matches): Check format instead of abfd->format. Cache and check negative result. (bfd_set_format): Likewise. ld/ 2006-04-20 H.J. Lu * ldfile.c (ldfile_try_open_bfd): Cleanup. * ldlang.c (load_symbols): Only call bfd_check_format_matches to get the last of matching targets for error. --- binutils/bfd/bfd-in.h.format 2006-03-16 12:37:42.000000000 -0800 +++ binutils/bfd/bfd-in.h 2006-04-20 07:24:37.000000000 -0700 @@ -168,13 +168,19 @@ typedef unsigned char bfd_byte; typedef enum bfd_format { bfd_unknown = 0, /* File format is unknown. */ - bfd_object, /* Linker/assembler/compiler output. */ - bfd_archive, /* Object archive file. */ - bfd_core, /* Core dump. */ - bfd_type_end /* Marks the end; don't use it! */ + bfd_object = 1, /* Linker/assembler/compiler output. */ + bfd_archive = 2, /* Object archive file. */ + bfd_core = 3, /* Core dump. */ + bfd_object_mask = 0x3, + bfd_type_end = 4, /* Marks the end; don't use it! */ + /* Make sure that bfd_format is big enough to hold 1 << (3 + 1). */ + bfd_type_max = 1 << 4 } bfd_format; +/* The bit to indicate that file isn't FORMAT. */ +#define bfd_format_negative_bit(format) (1 << ((format) + 1)) + /* Values that may appear in the flags field of a BFD. These also appear in the object_flags field of the bfd_target structure, where they indicate the set of flags used by that backend (not all flags --- binutils/bfd/format.c.format 2005-12-14 09:20:29.000000000 -0800 +++ binutils/bfd/format.c 2006-04-20 07:26:19.000000000 -0700 @@ -122,16 +122,26 @@ bfd_check_format_matches (bfd *abfd, bfd const bfd_target *save_targ, *right_targ, *ar_right_targ; int match_count; int ar_match_index; + bfd_format save_format; - if (!bfd_read_p (abfd) - || (unsigned int) abfd->format >= (unsigned int) bfd_type_end) + if ((unsigned int) format >= (unsigned int) bfd_type_end + || !bfd_read_p (abfd)) { bfd_set_error (bfd_error_invalid_operation); return FALSE; } if (abfd->format != bfd_unknown) - return abfd->format == format; + { + if ((abfd->format & bfd_object_mask) != 0) + return abfd->format == format; + /* FIXME: If caller wants the list of matching targets, we can't + just return. */ + else if (!matching + && (abfd->format + & bfd_format_negative_bit (format)) != 0) + return FALSE; + } /* Since the target type was defaulted, check them all in the hope that one will be uniquely recognized. */ @@ -154,6 +164,7 @@ bfd_check_format_matches (bfd *abfd, bfd ar_right_targ = 0; /* Presume the answer is yes. */ + save_format = abfd->format | bfd_format_negative_bit (format); abfd->format = format; /* If the target type was explicitly specified, just check that target. */ @@ -201,7 +212,7 @@ bfd_check_format_matches (bfd *abfd, bfd if (format == bfd_archive && save_targ == &binary_vec) { abfd->xvec = save_targ; - abfd->format = bfd_unknown; + abfd->format = save_format; if (matching) free (matching_vector); @@ -273,7 +284,7 @@ bfd_check_format_matches (bfd *abfd, bfd else if (err != bfd_error_wrong_format) { abfd->xvec = save_targ; - abfd->format = bfd_unknown; + abfd->format = save_format; if (matching) free (matching_vector); @@ -343,7 +354,7 @@ bfd_check_format_matches (bfd *abfd, bfd } abfd->xvec = save_targ; /* Restore original target type. */ - abfd->format = bfd_unknown; /* Restore original format. */ + abfd->format = save_format; /* Restore original format. */ if (match_count == 0) { @@ -390,22 +401,30 @@ DESCRIPTION bfd_boolean bfd_set_format (bfd *abfd, bfd_format format) { - if (bfd_read_p (abfd) - || (unsigned int) abfd->format >= (unsigned int) bfd_type_end) + bfd_format save_format; + + if ((unsigned int) format >= (unsigned int) bfd_type_end + || !bfd_write_p (abfd)) { bfd_set_error (bfd_error_invalid_operation); return FALSE; } if (abfd->format != bfd_unknown) - return abfd->format == format; + { + if ((abfd->format & bfd_object_mask) != 0) + return abfd->format == format; + else if ((abfd->format & bfd_format_negative_bit (format)) != 0) + return FALSE; + } /* Presume the answer is yes. */ + save_format = abfd->format | bfd_format_negative_bit (format); abfd->format = format; if (!BFD_SEND_FMT (abfd, _bfd_set_format, (abfd))) { - abfd->format = bfd_unknown; + abfd->format = save_format; return FALSE; } --- binutils/ld/ldfile.c.format 2005-05-16 11:04:40.000000000 -0700 +++ binutils/ld/ldfile.c 2006-04-20 07:09:07.000000000 -0700 @@ -159,18 +159,20 @@ ldfile_try_open_bfd (const char *attempt bfd *check; if (bfd_check_format (entry->the_bfd, bfd_archive)) - check = bfd_openr_next_archived_file (entry->the_bfd, NULL); + { + check = bfd_openr_next_archived_file (entry->the_bfd, NULL); + if (!check || !bfd_check_format (check, bfd_object)) + /* No further check on unknown file. */ + return TRUE; + } else - check = entry->the_bfd; - - if (check != NULL) { - if (! bfd_check_format (check, bfd_object)) + check = entry->the_bfd; + if (!bfd_check_format (entry->the_bfd, bfd_object)) { - if (check == entry->the_bfd - && entry->search_dirs_flag + if (entry->search_dirs_flag && bfd_get_error () == bfd_error_file_not_recognized - && ! ldemul_unrecognized_file (entry)) + && !ldemul_unrecognized_file (entry)) { int token, skip = 0; char *arg, *arg1, *arg2, *arg3; @@ -254,37 +256,37 @@ ldfile_try_open_bfd (const char *attempt { einfo (_("%P: skipping incompatible %s when searching for %s\n"), attempt, entry->local_sym_name); - bfd_close (entry->the_bfd); - entry->the_bfd = NULL; - return FALSE; + goto bad; } } + + /* No further check on unknown file. */ return TRUE; } - if (!entry->dynamic && (entry->the_bfd->flags & DYNAMIC) != 0) + if (!entry->dynamic + && (entry->the_bfd->flags & DYNAMIC) != 0) { einfo (_("%F%P: attempted static link of dynamic object `%s'\n"), attempt); - bfd_close (entry->the_bfd); - entry->the_bfd = NULL; - return FALSE; + goto bad; } + } - if (entry->search_dirs_flag - && !bfd_arch_get_compatible (check, output_bfd, - command_line.accept_unknown_input_arch) - /* XCOFF archives can have 32 and 64 bit objects. */ - && ! (bfd_get_flavour (check) == bfd_target_xcoff_flavour - && bfd_get_flavour (output_bfd) == bfd_target_xcoff_flavour - && bfd_check_format (entry->the_bfd, bfd_archive))) - { - einfo (_("%P: skipping incompatible %s when searching for %s\n"), - attempt, entry->local_sym_name); - bfd_close (entry->the_bfd); - entry->the_bfd = NULL; - return FALSE; - } + if (entry->search_dirs_flag + && !bfd_arch_get_compatible (check, output_bfd, + command_line.accept_unknown_input_arch) + /* XCOFF archives can have 32 and 64 bit objects. */ + && ! (bfd_get_flavour (check) == bfd_target_xcoff_flavour + && bfd_get_flavour (output_bfd) == bfd_target_xcoff_flavour + && bfd_check_format (entry->the_bfd, bfd_archive))) + { + einfo (_("%P: skipping incompatible %s when searching for %s\n"), + attempt, entry->local_sym_name); +bad: + bfd_close (entry->the_bfd); + entry->the_bfd = NULL; + return FALSE; } } --- binutils/ld/ldlang.c.format 2006-04-14 14:44:46.000000000 -0700 +++ binutils/ld/ldlang.c 2006-04-20 07:09:07.000000000 -0700 @@ -2309,15 +2309,13 @@ static bfd_boolean load_symbols (lang_input_statement_type *entry, lang_statement_list_type *place) { - char **matching; - if (entry->loaded) return TRUE; ldfile_open_file (entry); if (! bfd_check_format (entry->the_bfd, bfd_archive) - && ! bfd_check_format_matches (entry->the_bfd, bfd_object, &matching)) + && ! bfd_check_format (entry->the_bfd, bfd_object)) { bfd_error_type err; lang_statement_list_type *hold; @@ -2337,7 +2335,10 @@ load_symbols (lang_input_statement_type einfo (_("%B: file not recognized: %E\n"), entry->the_bfd); einfo (_("%B: matching formats:"), entry->the_bfd); - for (p = matching; *p != NULL; p++) + + /* Get the list of matching targets. */ + bfd_check_format_matches (entry->the_bfd, bfd_object, &p); + for (; *p != NULL; p++) einfo (" %s", *p); einfo ("%F\n"); }