From 4613510308cea27713e8c7424b2afee9b99f6226 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 12 Aug 2014 10:43:33 +0930 Subject: [PATCH] Change ld "notice" interface for better handling of indirect symbols The main aim of this change was to have non_ir_ref set correctly on new indirect symbols. I could have added a "copy" param to the "notice" function, so that indirect symbols could be created in plugin_notice, but it seemed cleaner to create indirect syms earlier and pass them rather than "string" to "notice". include/ * bfdlink.h (struct bfd_link_callbacks ): Remove "string" param, add "inh". bfd/ * coff-aux.c (coff_m68k_aux_link_add_one_symbol): Only call "notice" here when not calling the generic add_symbol function. Formatting. Correct handling of indirect symbols. Update notice call. * elflink.c (_bfd_elf_notice_as_needed): Update notice call. * linker.c (_bfd_generic_link_add_one_symbol): Create indirect symbols early. Update notice call. Add comments regarding weak symbols vs. indirect. ld/ * ldmain.c (notice): Update args. * plugin.c (plugin_notice): Likewise. Follow warning sym link. Handle new indirect symbol. --- bfd/coff-aux.c | 62 ++++++++++++++++++++++------------------- bfd/elflink.c | 2 +- bfd/linker.c | 82 +++++++++++++++++++++++++++++-------------------------- include/bfdlink.h | 13 ++++----- ld/ldmain.c | 6 ++-- ld/plugin.c | 33 ++++++++++++---------- 9 files changed, 128 insertions(+), 91 deletions(-) diff --git a/bfd/coff-aux.c b/bfd/coff-aux.c index e79c77a..d95b98b 100644 --- a/bfd/coff-aux.c +++ b/bfd/coff-aux.c @@ -73,20 +73,17 @@ coff_m68k_aux_link_add_one_symbol (struct bfd_link_info *info, bfd_boolean collect, struct bfd_link_hash_entry **hashp) { - struct bfd_link_hash_entry *h; + struct bfd_link_hash_entry *h, *inh, *t; - if ((flags & (BSF_WARNING | BSF_CONSTRUCTOR | BSF_WEAK)) == 0 && - !bfd_is_und_section (section) && - !bfd_is_com_section (section)) + if ((flags & (BSF_WARNING | BSF_CONSTRUCTOR | BSF_WEAK)) == 0 + && !bfd_is_und_section (section) + && !bfd_is_com_section (section)) { /* The new symbol is a definition or an indirect definition */ /* This bit copied from linker.c */ if (hashp != NULL && *hashp != NULL) - { - h = *hashp; - BFD_ASSERT (strcmp (h->root.string, name) == 0); - } + h = *hashp; else { h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE); @@ -98,37 +95,46 @@ coff_m68k_aux_link_add_one_symbol (struct bfd_link_info *info, } } - if (info->notice_hash != (struct bfd_hash_table *) NULL - && (bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) - != (struct bfd_hash_entry *) NULL)) - { - if (! (*info->callbacks->notice) (info, h, abfd, section, value, - flags, string)) - return FALSE; - } - if (hashp != (struct bfd_link_hash_entry **) NULL) *hashp = h; /* end duplication from linker.c */ - if (h->type == bfd_link_hash_defined - || h->type == bfd_link_hash_indirect) + t = h; + inh = NULL; + if (h->type == bfd_link_hash_indirect) { - asection *msec; + inh = h->u.i.link; + t = inh; + } - if (h->type == bfd_link_hash_defined) - msec = h->u.def.section; - else - msec = bfd_ind_section_ptr; + if (t->type == bfd_link_hash_defined) + { + asection *msec = t->u.def.section; + bfd_boolean special = FALSE; if (bfd_is_abs_section (msec) && !bfd_is_abs_section (section)) { - h->u.def.section = section; - h->u.def.value = value; - return TRUE; + t->u.def.section = section; + t->u.def.value = value; + special = TRUE; } else if (bfd_is_abs_section (section) && !bfd_is_abs_section (msec)) - return TRUE; + special = TRUE; + + if (special) + { + if (info->notice_all + || (info->notice_hash != NULL + && bfd_hash_lookup (info->notice_hash, name, + FALSE, FALSE) != NULL)) + { + if (!(*info->callbacks->notice) (info, h, inh, + abfd, section, value, flags)) + return FALSE; + } + + return TRUE; + } } } diff --git a/bfd/elflink.c b/bfd/elflink.c index 69a87a6..de0a734 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -3299,7 +3299,7 @@ _bfd_elf_notice_as_needed (bfd *ibfd, struct bfd_link_info *info, enum notice_asneeded_action act) { - return (*info->callbacks->notice) (info, NULL, ibfd, NULL, act, 0, NULL); + return (*info->callbacks->notice) (info, NULL, NULL, ibfd, NULL, act, 0); } /* Add symbols from an ELF object file to the linker hash table. */ diff --git a/bfd/linker.c b/bfd/linker.c index 1a5ecef..abdf5b0 100644 --- a/bfd/linker.c +++ b/bfd/linker.c @@ -1442,13 +1442,23 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, { enum link_row row; struct bfd_link_hash_entry *h; + struct bfd_link_hash_entry *inh = NULL; bfd_boolean cycle; BFD_ASSERT (section != NULL); if (bfd_is_ind_section (section) || (flags & BSF_INDIRECT) != 0) - row = INDR_ROW; + { + row = INDR_ROW; + /* Create the indirect symbol here. This is for the benefit of + the plugin "notice" function. + STRING is the name of the symbol we want to indirect to. */ + inh = bfd_wrapped_link_hash_lookup (abfd, info, string, TRUE, + copy, FALSE); + if (inh == NULL) + return FALSE; + } else if ((flags & BSF_WARNING) != 0) row = WARN_ROW; else if ((flags & BSF_CONSTRUCTOR) != 0) @@ -1493,8 +1503,8 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, || (info->notice_hash != NULL && bfd_hash_lookup (info->notice_hash, name, FALSE, FALSE) != NULL)) { - if (! (*info->callbacks->notice) (info, h, - abfd, section, value, flags, string)) + if (! (*info->callbacks->notice) (info, h, inh, + abfd, section, value, flags)) return FALSE; } @@ -1728,44 +1738,40 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, return FALSE; /* Fall through. */ case IND: - /* Create an indirect symbol. */ - { - struct bfd_link_hash_entry *inh; - - /* STRING is the name of the symbol we want to indirect - to. */ - inh = bfd_wrapped_link_hash_lookup (abfd, info, string, TRUE, - copy, FALSE); - if (inh == NULL) + if (inh->type == bfd_link_hash_indirect + && inh->u.i.link == h) + { + (*_bfd_error_handler) + (_("%B: indirect symbol `%s' to `%s' is a loop"), + abfd, name, string); + bfd_set_error (bfd_error_invalid_operation); return FALSE; - if (inh->type == bfd_link_hash_indirect - && inh->u.i.link == h) - { - (*_bfd_error_handler) - (_("%B: indirect symbol `%s' to `%s' is a loop"), - abfd, name, string); - bfd_set_error (bfd_error_invalid_operation); - return FALSE; - } - if (inh->type == bfd_link_hash_new) - { - inh->type = bfd_link_hash_undefined; - inh->u.undef.abfd = abfd; - bfd_link_add_undef (info->hash, inh); - } + } + if (inh->type == bfd_link_hash_new) + { + inh->type = bfd_link_hash_undefined; + inh->u.undef.abfd = abfd; + bfd_link_add_undef (info->hash, inh); + } - /* If the indirect symbol has been referenced, we need to - push the reference down to the symbol we are - referencing. */ - if (h->type != bfd_link_hash_new) - { - row = UNDEF_ROW; - cycle = TRUE; - } + /* If the indirect symbol has been referenced, we need to + push the reference down to the symbol we are referencing. */ + if (h->type != bfd_link_hash_new) + { + /* ??? If inh->type == bfd_link_hash_undefweak this + converts inh to bfd_link_hash_undefined. */ + row = UNDEF_ROW; + cycle = TRUE; + } - h->type = bfd_link_hash_indirect; - h->u.i.link = inh; - } + h->type = bfd_link_hash_indirect; + h->u.i.link = inh; + /* Not setting h = h->u.i.link here means that when cycle is + set above we'll always go to REFC, and then cycle again + to the indirected symbol. This means that any successful + change of an existing symbol to indirect counts as a + reference. ??? That may not be correct when the existing + symbol was defweak. */ break; case SET: diff --git a/include/bfdlink.h b/include/bfdlink.h index 58dba2a..125683d 100644 --- a/include/bfdlink.h +++ b/include/bfdlink.h @@ -640,15 +640,14 @@ 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. 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. FLAGS are the symbol - BSF_* flags. STRING is the name of the symbol to indirect to if - the sym is indirect, or the warning string if a warning sym. */ + defined or referenced. H is the symbol, INH the indirect symbol + if applicable. ABFD, SECTION and ADDRESS are the (new) value of + the symbol. If SECTION is bfd_und_section, this is a reference. + FLAGS are the symbol BSF_* flags. */ bfd_boolean (*notice) (struct bfd_link_info *, struct bfd_link_hash_entry *h, - bfd *abfd, asection *section, bfd_vma address, flagword flags, - const char *string); + struct bfd_link_hash_entry *inh, + bfd *abfd, asection *section, bfd_vma address, flagword flags); /* Error or warning link info message. */ void (*einfo) (const char *fmt, ...); diff --git a/ld/ldmain.c b/ld/ldmain.c index ea25afe..77235d5 100644 --- a/ld/ldmain.c +++ b/ld/ldmain.c @@ -137,7 +137,7 @@ static bfd_boolean unattached_reloc (struct bfd_link_info *, const char *, bfd *, asection *, bfd_vma); static bfd_boolean notice (struct bfd_link_info *, struct bfd_link_hash_entry *, - bfd *, asection *, bfd_vma, flagword, const char *); + struct bfd_link_hash_entry *, bfd *, asection *, bfd_vma, flagword); static struct bfd_link_callbacks link_callbacks = { @@ -1461,11 +1461,11 @@ unattached_reloc (struct bfd_link_info *info ATTRIBUTE_UNUSED, static bfd_boolean notice (struct bfd_link_info *info, struct bfd_link_hash_entry *h, + struct bfd_link_hash_entry *inh ATTRIBUTE_UNUSED, bfd *abfd, asection *section, bfd_vma value, - flagword flags ATTRIBUTE_UNUSED, - const char *string ATTRIBUTE_UNUSED) + flagword flags ATTRIBUTE_UNUSED) { const char *name; diff --git a/ld/plugin.c b/ld/plugin.c index 8d6ae05..8cca7d0 100644 --- a/ld/plugin.c +++ b/ld/plugin.c @@ -127,8 +127,9 @@ static const size_t tv_header_size = ARRAY_SIZE (tv_header_tags); /* Forward references. */ static bfd_boolean plugin_notice (struct bfd_link_info *, - struct bfd_link_hash_entry *, bfd *, - asection *, bfd_vma, flagword, const char *); + struct bfd_link_hash_entry *, + struct bfd_link_hash_entry *, + bfd *, asection *, bfd_vma, flagword); #if !defined (HAVE_DLFCN_H) && defined (HAVE_WINDOWS_H) @@ -962,16 +963,21 @@ plugin_call_cleanup (void) static bfd_boolean plugin_notice (struct bfd_link_info *info, struct bfd_link_hash_entry *h, + struct bfd_link_hash_entry *inh, bfd *abfd, asection *section, bfd_vma value, - flagword flags, - const char *string) + flagword flags) { + struct bfd_link_hash_entry *orig_h = h; + if (h != NULL) { bfd *sym_bfd; + if (h->type == bfd_link_hash_warning) + h = h->u.i.link; + /* Nothing to do here if this def/ref is from an IR dummy BFD. */ if (is_ir_dummy_bfd (abfd)) ; @@ -981,16 +987,15 @@ plugin_notice (struct bfd_link_info *info, else if (bfd_is_ind_section (section) || (flags & BSF_INDIRECT) != 0) { + /* ??? Some of this is questionable. See comments in + _bfd_generic_link_add_one_symbol for case IND. */ if (h->type != bfd_link_hash_new) { - struct bfd_link_hash_entry *inh; - h->non_ir_ref = TRUE; - inh = bfd_wrapped_link_hash_lookup (abfd, info, string, FALSE, - FALSE, FALSE); - if (inh != NULL) - inh->non_ir_ref = TRUE; + inh->non_ir_ref = TRUE; } + else if (inh->type == bfd_link_hash_new) + inh->non_ir_ref = TRUE; } /* Nothing to do here for warning symbols. */ @@ -1031,13 +1036,13 @@ plugin_notice (struct bfd_link_info *info, } /* Continue with cref/nocrossref/trace-sym processing. */ - if (h == NULL + if (orig_h == NULL || orig_notice_all || (info->notice_hash != NULL - && bfd_hash_lookup (info->notice_hash, h->root.string, + && bfd_hash_lookup (info->notice_hash, orig_h->root.string, FALSE, FALSE) != NULL)) - return (*orig_callbacks->notice) (info, h, - abfd, section, value, flags, string); + return (*orig_callbacks->notice) (info, orig_h, inh, + abfd, section, value, flags); return TRUE; } -- 1.9.3