* Change ld "notice" interface for better handling of indirect symbols
@ 2014-08-12 12:39 Alan Modra
2014-08-27 14:02 ` Kyle McMartin
0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2014-08-12 12:39 UTC (permalink / raw)
To: binutils
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 <notice>): 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.
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;
}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Change ld "notice" interface for better handling of indirect symbols
2014-08-12 12:39 Change ld "notice" interface for better handling of indirect symbols Alan Modra
@ 2014-08-27 14:02 ` Kyle McMartin
2014-08-28 2:11 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: Kyle McMartin @ 2014-08-27 14:02 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On Tue, Aug 12, 2014 at 10:09:11PM +0930, Alan Modra wrote:
> 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".
>
Happily, this seems to resolve an issue building with LTO and ld.bfd...
We had a bug report where building systemd on aarch64 and s390 were
failing with multiply defined symbols (everyone else has gold ported,
which was, for some as yet undebugged by me reason, didn't do this.)
https://bugzilla.redhat.com/show_bug.cgi?id=1133960
I tried to reduce your patch into something a little more friendly for
2.24 stable, but ran out of time to commit and just slurped it entirely
back into the Fedora binutils tree.
Anyway, thanks!
regards, Kyle
>
> include/
> * bfdlink.h (struct bfd_link_callbacks <notice>): 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.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Change ld "notice" interface for better handling of indirect symbols
2014-08-27 14:02 ` Kyle McMartin
@ 2014-08-28 2:11 ` Alan Modra
0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2014-08-28 2:11 UTC (permalink / raw)
To: Kyle McMartin; +Cc: binutils
On Wed, Aug 27, 2014 at 10:02:13AM -0400, Kyle McMartin wrote:
> On Tue, Aug 12, 2014 at 10:09:11PM +0930, Alan Modra wrote:
> > 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".
>
> Happily, this seems to resolve an issue building with LTO and ld.bfd...
> We had a bug report where building systemd on aarch64 and s390 were
> failing with multiply defined symbols (everyone else has gold ported,
> which was, for some as yet undebugged by me reason, didn't do this.)
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1133960
That's nice to know. I'd been looking at LTO and ld.bfd problems in
general, and warning symbols specifically (PR16746) when I saw the
potential problem with non_ir_ref and lack of following a warning sym
link in plugin_notice. So I guess it isn't surprising that this fixed
your bug.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-08-28 2:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-12 12:39 Change ld "notice" interface for better handling of indirect symbols Alan Modra
2014-08-27 14:02 ` Kyle McMartin
2014-08-28 2:11 ` Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).