From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15507 invoked by alias); 24 Apr 2011 10:14:29 -0000 Received: (qmail 15497 invoked by uid 22791); 24 Apr 2011 10:14:26 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FAKE_REPLY_C,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-pw0-f41.google.com (HELO mail-pw0-f41.google.com) (209.85.160.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 24 Apr 2011 10:14:11 +0000 Received: by pwi10 with SMTP id 10so1375177pwi.0 for ; Sun, 24 Apr 2011 03:14:10 -0700 (PDT) Received: by 10.142.3.13 with SMTP id 13mr1717911wfc.337.1303640050298; Sun, 24 Apr 2011 03:14:10 -0700 (PDT) Received: from bubble.grove.modra.org ([115.187.252.19]) by mx.google.com with ESMTPS id n7sm3509973wfl.11.2011.04.24.03.14.05 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 24 Apr 2011 03:14:08 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id A68B0170C2B0; Sun, 24 Apr 2011 19:44:01 +0930 (CST) Date: Sun, 24 Apr 2011 10:14:00 -0000 From: Alan Modra To: "H.J. Lu" , binutils@sourceware.org, richard.sandiford@linaro.org Subject: Re: Change bfd_link_hash_entry.type to a bitfield? Message-ID: <20110424101401.GQ19947@bubble.grove.modra.org> Mail-Followup-To: "H.J. Lu" , binutils@sourceware.org, richard.sandiford@linaro.org MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-04/txt/msg00331.txt.bz2 On Thu, Apr 21, 2011 at 12:54:57PM +0100, Richard Sandiford wrote: > > On Wed, Apr 20, 2011 at 6:57 PM, Alan Modra wrote: > >> How do people feel about poking some flags into bfd_link_hash_entry? > >> This can be done without increasing the size of the struct, since > >> "type" is an unsigned int but only needs 3 bits.  The negative side is > >> that hosts without efficient byte read access will see a linker > >> slowdown.  "type" is the most used field of bfd_link_hash_entry. > >> On the positive side, this would allow the linker lto plugin code to > >> lose a potentially very large symbol hash table.  Also, other fields > >> from elf_link_hash_entry and coff_link_hash_entry could move into > >> bfd_link_hash_entry, reducing the size of the linker hash table. > >> > > > "H.J. Lu" writes: > > I like it, > > +1. How about defining ENUM_BITFIELD in ansidecl.h, so that other > headers could use it too? That would be a good idea. Maybe for a later patch.. This version fixes PR 12696 as well. Committed. include/ PR ld/12365 PR ld/12696 * bfdlink.h (ENUM_BITFIELD): Define. (struct bfd_link_hash_entry): Make "type" a bitfield. Add "non_ir_ref". (struct bfd_link_callbacks ): Pass bfd_link_hash_entry pointer rather than "name". bfd/ PR ld/12365 PR ld/12696 * coff-aux.c (coff_m68k_aux_link_add_one_symbol): Update "notice" call. * linker.c (_bfd_link_hash_newfunc): Clear bitfields. (_bfd_generic_link_add_one_symbol): Update "notice" call. * elflink.c (_bfd_elf_merge_symbol): Don't skip weak redefs when it is a redef of an IR symbol in a real BFD. ld/ PR ld/12365 PR ld/12696 * ldmain.c (notice): Delete "name" param, add "h". * plugin.c (plugin_notice): Likewise. Set non_ir_ref. Handle redefinitions of IR symbols in real BFDs. (plugin_multiple_definition, plugin_multiple_common): Delete. (non_ironly_hash, init_non_ironly_hash): Delete. (is_visible_from_outside): Traverse entry_symbol chain. (get_symbols): Use non_ir_ref flag rather than hash lookup. Index: include/bfdlink.h =================================================================== RCS file: /cvs/src/src/include/bfdlink.h,v retrieving revision 1.83 diff -u -p -r1.83 bfdlink.h --- include/bfdlink.h 20 Apr 2011 00:11:29 -0000 1.83 +++ include/bfdlink.h 24 Apr 2011 07:35:37 -0000 @@ -24,6 +24,12 @@ #ifndef BFDLINK_H #define BFDLINK_H +#if (__GNUC__ * 1000 + __GNUC_MINOR__ > 2000) +#define ENUM_BITFIELD(TYPE) __extension__ enum TYPE +#else +#define ENUM_BITFIELD(TYPE) unsigned int +#endif + /* Which symbols to strip during a link. */ enum bfd_link_strip { @@ -91,7 +97,9 @@ struct bfd_link_hash_entry struct bfd_hash_entry root; /* Type of this entry. */ - enum bfd_link_hash_type type; + ENUM_BITFIELD (bfd_link_hash_type) type : 8; + + unsigned int non_ir_ref : 1; /* A union of information depending upon the type. */ union @@ -570,11 +578,11 @@ struct bfd_link_callbacks (struct bfd_link_info *, const char *name, bfd *abfd, asection *section, bfd_vma address); /* A function which is called when a symbol in notice_hash is - defined or referenced. NAME is the symbol. ABFD, SECTION and - ADDRESS are the value of the symbol. If SECTION is + defined or referenced. H is the symbol. ABFD, SECTION and + ADDRESS are the (new) value of the symbol. If SECTION is bfd_und_section, this is a reference. */ bfd_boolean (*notice) - (struct bfd_link_info *, const char *name, + (struct bfd_link_info *, struct bfd_link_hash_entry *h, bfd *abfd, asection *section, bfd_vma address); /* Error or warning link info message. */ void (*einfo) Index: bfd/elflink.c =================================================================== RCS file: /cvs/src/src/bfd/elflink.c,v retrieving revision 1.401 diff -u -p -r1.401 elflink.c --- bfd/elflink.c 20 Apr 2011 07:17:01 -0000 1.401 +++ bfd/elflink.c 24 Apr 2011 07:35:20 -0000 @@ -1427,7 +1427,10 @@ _bfd_elf_merge_symbol (bfd *abfd, /* Skip weak definitions of symbols that are already defined. */ if (newdef && olddef && newweak) { - *skip = TRUE; + /* Don't skip new non-IR weak syms. */ + if (!((oldbfd->flags & BFD_PLUGIN) != 0 + && (abfd->flags & BFD_PLUGIN) == 0)) + *skip = TRUE; /* Merge st_other. If the symbol already has a dynamic index, but visibility says it should not be visible, turn it into a Index: bfd/coff-aux.c =================================================================== RCS file: /cvs/src/src/bfd/coff-aux.c,v retrieving revision 1.10 diff -u -p -r1.10 coff-aux.c --- bfd/coff-aux.c 2 Sep 2009 07:18:36 -0000 1.10 +++ bfd/coff-aux.c 24 Apr 2011 07:35:10 -0000 @@ -105,7 +105,7 @@ coff_m68k_aux_link_add_one_symbol (info, && (bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != (struct bfd_hash_entry *) NULL)) { - if (! (*info->callbacks->notice) (info, name, abfd, section, value)) + if (! (*info->callbacks->notice) (info, h, abfd, section, value)) return FALSE; } Index: bfd/linker.c =================================================================== RCS file: /cvs/src/src/bfd/linker.c,v retrieving revision 1.80 diff -u -p -r1.80 linker.c --- bfd/linker.c 20 Apr 2011 00:22:08 -0000 1.80 +++ bfd/linker.c 24 Apr 2011 07:35:23 -0000 @@ -465,10 +465,8 @@ _bfd_link_hash_newfunc (struct bfd_hash_ struct bfd_link_hash_entry *h = (struct bfd_link_hash_entry *) entry; /* Initialize the local fields. */ - h->type = bfd_link_hash_new; - memset (&h->u.undef.next, 0, - (sizeof (struct bfd_link_hash_entry) - - offsetof (struct bfd_link_hash_entry, u.undef.next))); + memset ((char *) &h->root + sizeof (h->root), 0, + sizeof (*h) - sizeof (h->root)); } return entry; @@ -1609,8 +1607,7 @@ _bfd_generic_link_add_one_symbol (struct || (info->notice_hash != NULL && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL)) { - if (! (*info->callbacks->notice) (info, h->root.string, abfd, section, - value)) + if (! (*info->callbacks->notice) (info, h, abfd, section, value)) return FALSE; } Index: ld/ldmain.c =================================================================== RCS file: /cvs/src/src/ld/ldmain.c,v retrieving revision 1.153 diff -u -p -r1.153 ldmain.c --- ld/ldmain.c 20 Apr 2011 00:22:08 -0000 1.153 +++ ld/ldmain.c 24 Apr 2011 07:35:38 -0000 @@ -150,7 +150,8 @@ static bfd_boolean reloc_dangerous static bfd_boolean unattached_reloc (struct bfd_link_info *, const char *, bfd *, asection *, bfd_vma); static bfd_boolean notice - (struct bfd_link_info *, const char *, bfd *, asection *, bfd_vma); + (struct bfd_link_info *, struct bfd_link_hash_entry *, + bfd *, asection *, bfd_vma); static struct bfd_link_callbacks link_callbacks = { @@ -1479,18 +1480,21 @@ unattached_reloc (struct bfd_link_info * static bfd_boolean notice (struct bfd_link_info *info, - const char *name, + struct bfd_link_hash_entry *h, bfd *abfd, asection *section, bfd_vma value) { - if (name == NULL) + const char *name; + + if (h == NULL) { if (command_line.cref || nocrossref_list != NULL) return handle_asneeded_cref (abfd, (enum notice_asneeded_action) value); return TRUE; } + name = h->root.string; if (info->notice_hash != NULL && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL) { Index: ld/plugin.c =================================================================== RCS file: /cvs/src/src/ld/plugin.c,v retrieving revision 1.32 diff -u -p -r1.32 plugin.c --- ld/plugin.c 20 Apr 2011 00:22:08 -0000 1.32 +++ ld/plugin.c 24 Apr 2011 07:35:38 -0000 @@ -90,13 +90,6 @@ static plugin_t *called_plugin = NULL; /* Last plugin to cause an error, if any. */ static const char *error_plugin = NULL; -/* A hash table that records symbols referenced by non-IR files. Used - at get_symbols time to determine whether any prevailing defs from - IR files are referenced only from other IR files, so tthat we can - we can distinguish the LDPR_PREVAILING_DEF and LDPR_PREVAILING_DEF_IRONLY - cases when establishing symbol resolutions. */ -static struct bfd_hash_table *non_ironly_hash = NULL; - /* State of linker "notice" interface before we poked at it. */ static bfd_boolean orig_notice_all; @@ -133,18 +126,8 @@ static const size_t tv_header_size = ARR /* Forward references. */ static bfd_boolean plugin_notice (struct bfd_link_info *info, - const char *name, bfd *abfd, + struct bfd_link_hash_entry *h, bfd *abfd, asection *section, bfd_vma value); -static bfd_boolean plugin_multiple_definition (struct bfd_link_info *info, - struct bfd_link_hash_entry *h, - bfd *nbfd, - asection *nsec, - bfd_vma nval); -static bfd_boolean plugin_multiple_common (struct bfd_link_info *info, - struct bfd_link_hash_entry *h, - bfd *nbfd, - enum bfd_link_hash_type ntype, - bfd_vma nsize); #if !defined (HAVE_DLFCN_H) && defined (HAVE_WINDOWS_H) @@ -438,6 +421,8 @@ static inline bfd_boolean is_visible_from_outside (struct ld_plugin_symbol *lsym, asection *section, struct bfd_link_hash_entry *blhe) { + struct bfd_sym_chain *sym; + /* Section's owner may be NULL if it is the absolute section, fortunately is_ir_dummy_bfd handles that. */ if (!is_ir_dummy_bfd (section->owner)) @@ -466,6 +451,12 @@ is_visible_from_outside (struct ld_plugi return (lsym->visibility == LDPV_DEFAULT || lsym->visibility == LDPV_PROTECTED); } + + for (sym = &entry_symbol; sym != NULL; sym = sym->next) + if (sym->name + && strcmp (sym->name, blhe->root.string) == 0) + return TRUE; + return FALSE; } @@ -520,9 +511,8 @@ get_symbols (const void *handle, int nsy even potentially-referenced, perhaps in a future final link if this is a partial one, perhaps dynamically at load-time if the symbol is externally visible. */ - ironly = (!is_visible_from_outside (&syms[n], owner_sec, blhe) - && !bfd_hash_lookup (non_ironly_hash, syms[n].name, - FALSE, FALSE)); + ironly = !(blhe->non_ir_ref + || is_visible_from_outside (&syms[n], owner_sec, blhe)); /* If it was originally undefined or common, then it has been resolved; determine how. */ @@ -740,27 +730,6 @@ plugin_active_plugins_p (void) return plugins_list != NULL; } -/* Init the non_ironly hash table. */ -static void -init_non_ironly_hash (void) -{ - struct bfd_sym_chain *sym; - - non_ironly_hash - = (struct bfd_hash_table *) xmalloc (sizeof (struct bfd_hash_table)); - if (!bfd_hash_table_init_n (non_ironly_hash, - bfd_hash_newfunc, - sizeof (struct bfd_hash_entry), - 61)) - einfo (_("%P%F: bfd_hash_table_init failed: %E\n")); - - for (sym = &entry_symbol; sym != NULL; sym = sym->next) - if (sym->name - && !bfd_hash_lookup (non_ironly_hash, sym->name, TRUE, TRUE)) - einfo (_("%P%X: hash table failure adding symbol %s\n"), - sym->name); -} - /* Load up and initialise all plugins after argument parsing. */ int plugin_load_plugins (void) @@ -814,7 +783,6 @@ plugin_load_plugins (void) plugin_callbacks.notice = &plugin_notice; link_info.notice_all = TRUE; link_info.callbacks = &plugin_callbacks; - init_non_ironly_hash (); return 0; } @@ -888,9 +856,6 @@ plugin_call_all_symbols_read (void) /* Disable any further file-claiming. */ no_more_claiming = TRUE; - plugin_callbacks.multiple_definition = &plugin_multiple_definition; - plugin_callbacks.multiple_common = &plugin_multiple_common; - while (curplug) { if (curplug->all_symbols_read_handler) @@ -934,88 +899,47 @@ plugin_call_cleanup (void) /* To determine which symbols should be resolved LDPR_PREVAILING_DEF and which LDPR_PREVAILING_DEF_IRONLY, we notice all the symbols as - the linker adds them to the linker hash table. If we see a symbol - being referenced from a non-IR file, we add it to the non_ironly hash - table. If we can't find it there at get_symbols time, we know that - it was referenced only by IR files. We have to notice_all symbols, - because we won't necessarily know until later which ones will be - contributed by IR files. */ + the linker adds them to the linker hash table. Mark those + referenced from a non-IR file with non_ir_ref. We have to + notice_all symbols, because we won't necessarily know until later + which ones will be contributed by IR files. */ static bfd_boolean plugin_notice (struct bfd_link_info *info, - const char *name, + struct bfd_link_hash_entry *h, bfd *abfd, asection *section, bfd_vma value) { - if (name != NULL) + if (h != NULL) { /* No further processing if this def/ref is from an IR dummy BFD. */ if (is_ir_dummy_bfd (abfd)) return TRUE; - /* We only care about refs, not defs, indicated by section - pointing to the undefined section (according to the bfd - linker notice callback interface definition). */ + /* If this is a ref, set non_ir_ref. */ if (bfd_is_und_section (section)) - { - /* This is a ref from a non-IR file, so note the ref'd - symbol in the non-IR-only hash. */ - if (!bfd_hash_lookup (non_ironly_hash, name, TRUE, TRUE)) - einfo (_("%P%X: %s: hash table failure adding symbol %s\n"), - abfd->filename, name); - } + h->non_ir_ref = TRUE; + + /* Otherwise, it must be a new def. Ensure any symbol defined + in an IR dummy BFD takes on a new value from a real BFD. + Weak symbols are not normally overridden by a new weak + definition, and strong symbols will normally cause multiple + definition errors. Avoid this by making the symbol appear + to be undefined. */ + else if (((h->type == bfd_link_hash_defweak + || h->type == bfd_link_hash_defined) + && is_ir_dummy_bfd (h->u.def.section->owner)) + || (h->type == bfd_link_hash_common + && is_ir_dummy_bfd (h->u.c.p->section->owner))) + h->type = bfd_link_hash_undefweak; } /* Continue with cref/nocrossref/trace-sym processing. */ - if (name == NULL + if (h == NULL || orig_notice_all || (info->notice_hash != NULL - && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL)) - return (*orig_callbacks->notice) (info, name, abfd, section, value); + && bfd_hash_lookup (info->notice_hash, h->root.string, + FALSE, FALSE) != NULL)) + return (*orig_callbacks->notice) (info, h, abfd, section, value); return TRUE; } - -/* When we add new object files to the link at all symbols read time, - these contain the real code and symbols generated from the IR files, - and so duplicate all the definitions already supplied by the dummy - IR-only BFDs that we created at claim files time. We use the linker's - multiple-definitions callback hook to fix up the clash, discarding - the symbol from the IR-only BFD in favour of the symbol from the - real BFD. We return true if this was not-really-a-clash because - we've fixed it up, or anyway if --allow-multiple-definition was in - effect (before we disabled it to ensure we got called back). */ -static bfd_boolean -plugin_multiple_definition (struct bfd_link_info *info, - struct bfd_link_hash_entry *h, - bfd *nbfd, asection *nsec, bfd_vma nval) -{ - if (h->type == bfd_link_hash_defined - && is_ir_dummy_bfd (h->u.def.section->owner)) - { - /* Replace it with new details. */ - h->u.def.section = nsec; - h->u.def.value = nval; - return TRUE; - } - - return (*orig_callbacks->multiple_definition) (info, h, nbfd, nsec, nval); -} - -static bfd_boolean -plugin_multiple_common (struct bfd_link_info *info, - struct bfd_link_hash_entry *h, - bfd *nbfd, enum bfd_link_hash_type ntype, bfd_vma nsize) -{ - if (h->type == bfd_link_hash_common - && is_ir_dummy_bfd (h->u.c.p->section->owner) - && ntype == bfd_link_hash_common - && !is_ir_dummy_bfd (nbfd)) - { - /* Arrange to have it replaced. */ - ASSERT (nsize != 0); - h->u.c.size = 0; - return TRUE; - } - - return (*orig_callbacks->multiple_common) (info, h, nbfd, ntype, nsize); -} -- Alan Modra Australia Development Lab, IBM