IA64 linker is broken

Alan Modra amodra@bigpond.net.au
Mon Jan 31 07:13:00 GMT 2005


On Mon, Jan 31, 2005 at 10:32:27AM +1030, Alan Modra wrote:
> On Sun, Jan 30, 2005 at 11:22:49AM -0800, H. J. Lu wrote:
> > /usr/lib/crt1.o:(.dynamic+0x0): multiple definition of `_DYNAMIC'
> [snip]
> > The 2005-01-24 binutils is OK. It may have something to do with
> > 
> > http://sources.redhat.com/ml/binutils/2005-01/msg00405.html
> 
> Possible, I suppose.  An as-needed shared lib will define syms whether
> or not the lib is actually linked.  It will be linked if any symbol it
> defines satisfies an undefined reference, and conversely it isn't linked
> then there are no references to its symbols.  That should make it safe
> to leave its symbols in the symbol table, so long as we properly treat
> them in _bfd_elf_merge_symbol.  If there is a problem, it's likely to be
> in _bfd_elf_merge_symbol.

I'm testing the following on powerpc and x86 to ensure I haven't made
any silly mistakes.  It's a big hammer approach but cleaner than what we
had before, I think.  Rather than tweaking _bfd_elf_merge_symbol and
other places to specially handle symbols defined in unused --as-needed
libs, I've munged all such symbols back to their new state.  Hopefully
this won't break too many back-end elf_link_hash_traverse functions..

Would you please test this on ia64?  I don't have access to ia64
hardware, so testing this isn't so easy.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre
-------------- next part --------------
include/
	* bfdlink.h (bfd_link_repair_undef_list): Declare.
bfd/
	* elf64-ppc.c (ppc64_elf_check_directives): Move undefs list fixup..
	* linker.c (bfd_link_repair_undef_list): ..to new function, but don't
	remove anything but new and undefweak.
	* elflink.c (bfd_elf_record_link_assignment): Call
	bfd_link_repair_undef_list.
	(_bfd_elf_merge_symbol): Don't handle as-needed syms here.
	(struct elf_smash_data): New.
	(elf_smash_syms): New function.
	(elf_link_add_object_symbols): Call elf_smash_syms.  Don't add
	unneeded dynamic objects to loaded list.
	(elf_link_output_extsym): Don't handle as-needed here.  Strip
	bfd_link_hash_new symbols.
	* elf32-cris.c (elf_cris_discard_excess_program_dynamics): Don't
	delref when dynindx is already -1.
	* elf64-alpha.c (elf64_alpha_output_extsym): Strip bfd_link_hash_new
	symbols.
	* elfxx-mips.c (mips_elf_output_extsym): Likewise.
ld/
	* ld.texinfo: Clarify --as-needed operation.

Index: include/bfdlink.h
===================================================================
RCS file: /cvs/src/src/include/bfdlink.h,v
retrieving revision 1.50
diff -u -p -r1.50 bfdlink.h
--- include/bfdlink.h	15 Nov 2004 23:21:26 -0000	1.50
+++ include/bfdlink.h	31 Jan 2005 06:46:06 -0000
@@ -197,6 +197,10 @@ extern void bfd_link_hash_traverse
 extern void bfd_link_add_undef
   (struct bfd_link_hash_table *, struct bfd_link_hash_entry *);
 
+/* Remove symbols from the undefs list that don't belong there.  */
+extern void bfd_link_repair_undef_list
+  (struct bfd_link_hash_table *table);
+
 struct bfd_sym_chain
 {
   struct bfd_sym_chain *next;
Index: bfd/elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.185
diff -u -p -r1.185 elf64-ppc.c
--- bfd/elf64-ppc.c	11 Jan 2005 09:32:52 -0000	1.185
+++ bfd/elf64-ppc.c	31 Jan 2005 06:45:19 -0000
@@ -4066,34 +4066,7 @@ ppc64_elf_check_directives (bfd *abfd AT
      undef_weak.  */
   if (htab->twiddled_syms)
     {
-      struct bfd_link_hash_entry **pun;
-
-      pun = &htab->elf.root.undefs;
-      while (*pun != NULL)
-	{
-	  struct bfd_link_hash_entry *h = *pun;
-
-	  if (h->type != bfd_link_hash_undefined
-	      && h->type != bfd_link_hash_common)
-	    {
-	      *pun = h->u.undef.next;
-	      h->u.undef.next = NULL;
-	      if (h == htab->elf.root.undefs_tail)
-		{
-		  if (pun == &htab->elf.root.undefs)
-		    htab->elf.root.undefs_tail = NULL;
-		  else
-		    /* pun points at an u.undef.next field.  Go back to
-		       the start of the link_hash_entry.  */
-		    htab->elf.root.undefs_tail = (struct bfd_link_hash_entry *)
-		      ((char *) pun - ((char *) &h->u.undef.next - (char *) h));
-		  break;
-		}
-	    }
-	  else
-	    pun = &h->u.undef.next;
-	}
-
+      bfd_link_repair_undef_list (&htab->elf.root);
       htab->twiddled_syms = 0;
     }
   return TRUE;
Index: bfd/linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.42
diff -u -p -r1.42 linker.c
--- bfd/linker.c	21 Oct 2004 15:28:30 -0000	1.42
+++ bfd/linker.c	31 Jan 2005 06:45:32 -0000
@@ -623,6 +623,45 @@ bfd_link_add_undef (struct bfd_link_hash
     table->undefs = h;
   table->undefs_tail = h;
 }
+
+/* The undefs list was designed so that in normal use we don't need to
+   remove entries.  However, if symbols on the list are changed from
+   bfd_link_hash_undefined to either bfd_link_hash_undefweak or
+   bfd_link_hash_new for some reason, then they must be removed from the
+   list.  Failure to do so might result in the linker attempting to add
+   the symbol to the list again at a later stage.  */
+
+void
+bfd_link_repair_undef_list (struct bfd_link_hash_table *table)
+{
+  struct bfd_link_hash_entry **pun;
+
+  pun = &table->undefs;
+  while (*pun != NULL)
+    {
+      struct bfd_link_hash_entry *h = *pun;
+
+      if (h->type == bfd_link_hash_new
+	  || h->type == bfd_link_hash_undefweak)
+	{
+	  *pun = h->u.undef.next;
+	  h->u.undef.next = NULL;
+	  if (h == table->undefs_tail)
+	    {
+	      if (pun == &table->undefs)
+		table->undefs_tail = NULL;
+	      else
+		/* pun points at an u.undef.next field.  Go back to
+		   the start of the link_hash_entry.  */
+		table->undefs_tail = (struct bfd_link_hash_entry *)
+		  ((char *) pun - ((char *) &h->u.undef.next - (char *) h));
+	      break;
+	    }
+	}
+      else
+	pun = &h->u.undef.next;
+    }
+}
 
 /* Routine to create an entry in a generic link hash table.  */
 
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.124
diff -u -p -r1.124 elflink.c
--- bfd/elflink.c	25 Jan 2005 01:40:01 -0000	1.124
+++ bfd/elflink.c	31 Jan 2005 06:45:25 -0000
@@ -442,15 +442,16 @@ bfd_elf_record_link_assignment (bfd *out
 
   /* Since we're defining the symbol, don't let it seem to have not
      been defined.  record_dynamic_symbol and size_dynamic_sections
-     may depend on this.
-     ??? Changing bfd_link_hash_undefined to bfd_link_hash_new (or
-     to bfd_link_hash_undefweak, see linker.c:link_action) runs the risk
-     of some later symbol manipulation setting the symbol back to
-     bfd_link_hash_undefined, and the linker trying to add the symbol to
-     the undefs list twice.  */
+     may depend on this.  */
   if (h->root.type == bfd_link_hash_undefweak
       || h->root.type == bfd_link_hash_undefined)
-    h->root.type = bfd_link_hash_new;
+    {
+      struct elf_link_hash_table *htab = elf_hash_table (info);
+
+      if (h->root.u.undef.next != NULL || htab->root.undefs_tail == &h->root)
+	bfd_link_repair_undef_list (&htab->root);
+      h->root.type = bfd_link_hash_new;
+    }
 
   if (h->root.type == bfd_link_hash_new)
     h->non_elf = 0;
@@ -728,7 +729,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
   int bind;
   bfd *oldbfd;
   bfd_boolean newdyn, olddyn, olddef, newdef, newdyncommon, olddyncommon;
-  bfd_boolean newweak, oldweak, old_asneeded;
+  bfd_boolean newweak, oldweak;
 
   *skip = FALSE;
   *override = FALSE;
@@ -858,14 +859,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
   else
     olddef = TRUE;
 
-  /* If the old definition came from an as-needed dynamic library which
-     wasn't found to be needed, treat the sym as undefined.  */
-  old_asneeded = FALSE;
-  if (newdyn
-      && olddyn
-      && (elf_dyn_lib_class (oldbfd) & DYN_AS_NEEDED) != 0)
-    old_asneeded = TRUE;
-
   /* Check TLS symbol.  */
   if ((ELF_ST_TYPE (sym->st_info) == STT_TLS || h->type == STT_TLS)
       && ELF_ST_TYPE (sym->st_info) != h->type)
@@ -1068,7 +1061,6 @@ _bfd_elf_merge_symbol (bfd *abfd,
 
   if (olddyn
       && olddef
-      && !old_asneeded
       && h->root.type == bfd_link_hash_defined
       && h->def_dynamic
       && (h->root.u.def.section->flags & SEC_ALLOC) != 0
@@ -1120,7 +1112,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
 
   if (newdyn
       && newdef
-      && ((olddef && !old_asneeded)
+      && (olddef
 	  || (h->root.type == bfd_link_hash_common
 	      && (newweak
 		  || ELF_ST_TYPE (sym->st_info) == STT_FUNC))))
@@ -1170,7 +1162,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
      symbol is a function or is weak.  */
 
   flip = NULL;
-  if ((!newdyn || old_asneeded)
+  if (!newdyn
       && (newdef
 	  || (bfd_is_com_section (sec)
 	      && (oldweak
@@ -2813,6 +2805,69 @@ elf_add_dt_needed_tag (bfd *abfd,
   return 0;
 }
 
+/* Called via elf_link_hash_traverse, elf_smash_syms sets all symbols
+   belonging to NOT_NEEDED to bfd_link_hash_new.  We know there are no
+   references to these symbols.  */
+
+struct elf_smash_syms_data
+{
+  bfd *not_needed;
+  struct elf_link_hash_table *htab;
+  bfd_boolean twiddled;
+};
+
+static bfd_boolean
+elf_smash_syms (struct elf_link_hash_entry *h, void *data)
+{
+  struct elf_smash_syms_data *inf = (struct elf_smash_syms_data *) data;
+  struct bfd_link_hash_entry *bh;
+
+  switch (h->root.type)
+    {
+    default:
+    case bfd_link_hash_new:
+      return TRUE;
+
+    case bfd_link_hash_undefined:
+    case bfd_link_hash_undefweak:
+      if (h->root.u.undef.abfd != inf->not_needed)
+	return TRUE;
+      break;
+
+    case bfd_link_hash_defined:
+    case bfd_link_hash_defweak:
+      if (h->root.u.def.section->owner != inf->not_needed)
+	return TRUE;
+      break;
+
+    case bfd_link_hash_common:
+      if (h->root.u.c.p->section->owner != inf->not_needed)
+	return TRUE;
+      break;
+
+    case bfd_link_hash_warning:
+    case bfd_link_hash_indirect:
+      elf_smash_syms ((struct elf_link_hash_entry *) h->root.u.i.link, data);
+      if (h->root.u.i.link->type != bfd_link_hash_new)
+	return TRUE;
+      if (h->root.u.i.link->u.undef.abfd != inf->not_needed)
+	return TRUE;
+      break;
+    }
+
+  /* Set sym back to newly created state, but keep undefs list pointer.  */
+  bh = h->root.u.undef.next;
+  if (bh != NULL || inf->htab->root.undefs_tail == &h->root)
+    inf->twiddled = TRUE;
+  (*inf->htab->root.table.newfunc) (&h->root.root,
+				    &inf->htab->root.table,
+				    h->root.root.string);
+  h->root.u.undef.next = bh;
+  h->root.u.undef.abfd = inf->not_needed;
+  h->non_elf = 0;
+  return TRUE;
+}
+
 /* Sort symbol by value and section.  */
 static int
 elf_sort_symbol (const void *arg1, const void *arg2)
@@ -4031,6 +4086,18 @@ elf_link_add_object_symbols (bfd *abfd, 
     free (isymbuf);
   isymbuf = NULL;
 
+  if (!add_needed)
+    {
+      struct elf_smash_syms_data inf;
+      inf.not_needed = abfd;
+      inf.htab = hash_table;
+      inf.twiddled = FALSE;
+      elf_link_hash_traverse (hash_table, elf_smash_syms, &inf);
+      if (inf.twiddled)
+	bfd_link_repair_undef_list (&hash_table->root);
+      weaks = NULL;
+    }
+
   /* Now set the weakdefs field correctly for all the weak defined
      symbols we found.  The only way to do this is to search all the
      symbols.  Since we only need the information for non functions in
@@ -4263,7 +4330,7 @@ elf_link_add_object_symbols (bfd *abfd, 
 	}
     }
 
-  if (is_elf_hash_table (hash_table))
+  if (is_elf_hash_table (hash_table) && add_needed)
     {
       /* Add this bfd to the loaded list.  */
       struct elf_link_loaded_list *n;
@@ -6143,11 +6210,7 @@ elf_link_output_extsym (struct elf_link_
   const struct elf_backend_data *bed;
 
   if (h->root.type == bfd_link_hash_warning)
-    {
-      h = (struct elf_link_hash_entry *) h->root.u.i.link;
-      if (h->root.type == bfd_link_hash_new)
-	return TRUE;
-    }
+    h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
   /* Decide whether to output this symbol in this pass.  */
   if (eoinfo->localsyms)
@@ -6170,7 +6233,6 @@ elf_link_output_extsym (struct elf_link_
   if (h->root.type == bfd_link_hash_undefined
       && h->ref_dynamic
       && !h->ref_regular
-      && (elf_dyn_lib_class (h->root.u.undef.abfd) & DYN_AS_NEEDED) == 0
       && ! elf_link_check_versioned_symbol (finfo->info, bed, h)
       && finfo->info->unresolved_syms_in_shared_libs != RM_IGNORE)
     {
@@ -6212,7 +6274,8 @@ elf_link_output_extsym (struct elf_link_
   if (h->indx == -2)
     strip = FALSE;
   else if ((h->def_dynamic
-	    || h->ref_dynamic)
+	    || h->ref_dynamic
+	    || h->root.type == bfd_link_hash_new)
 	   && !h->def_regular
 	   && !h->ref_regular)
     strip = TRUE;
Index: bfd/elf32-cris.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-cris.c,v
retrieving revision 1.61
diff -u -p -r1.61 elf32-cris.c
--- bfd/elf32-cris.c	11 Jan 2005 09:32:46 -0000	1.61
+++ bfd/elf32-cris.c	31 Jan 2005 06:45:12 -0000
@@ -3130,6 +3130,7 @@ elf_cris_discard_excess_program_dynamics
 	 introduce new problems.  Of course we don't do this if we're
 	 exporting all dynamic symbols.  */
       if (! info->export_dynamic
+	  && h->root.dynindx != -1
 	  && !h->root.def_dynamic
 	  && !h->root.ref_dynamic)
 	{
Index: bfd/elf64-alpha.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-alpha.c,v
retrieving revision 1.125
diff -u -p -r1.125 elf64-alpha.c
--- bfd/elf64-alpha.c	21 Oct 2004 15:28:25 -0000	1.125
+++ bfd/elf64-alpha.c	31 Jan 2005 06:45:15 -0000
@@ -2741,7 +2741,9 @@ elf64_alpha_output_extsym (h, data)
 
   if (h->root.indx == -2)
     strip = FALSE;
-  else if ((h->root.def_dynamic || h->root.ref_dynamic)
+  else if ((h->root.def_dynamic
+	    || h->root.ref_dynamic
+	    || h->root.root.type == bfd_link_hash_new)
 	   && !h->root.def_regular
 	   && !h->root.ref_regular)
     strip = TRUE;
Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.117
diff -u -p -r1.117 elfxx-mips.c
--- bfd/elfxx-mips.c	14 Dec 2004 09:48:10 -0000	1.117
+++ bfd/elfxx-mips.c	31 Jan 2005 06:45:30 -0000
@@ -1491,7 +1491,8 @@ mips_elf_output_extsym (struct mips_elf_
   if (h->root.indx == -2)
     strip = FALSE;
   else if ((h->root.def_dynamic
-	    || h->root.ref_dynamic)
+	    || h->root.ref_dynamic
+	    || h->root.type == bfd_link_hash_new)
 	   && !h->root.def_regular
 	   && !h->root.ref_regular)
     strip = TRUE;
Index: ld/ld.texinfo
===================================================================
RCS file: /cvs/src/src/ld/ld.texinfo,v
retrieving revision 1.138
diff -u -p -r1.138 ld.texinfo
--- ld/ld.texinfo	28 Jan 2005 17:24:40 -0000	1.138
+++ ld/ld.texinfo	31 Jan 2005 06:46:12 -0000
@@ -994,8 +994,9 @@ This option affects ELF DT_NEEDED tags f
 on the command line after the @option{--as-needed} option.  Normally,
 the linker will add a DT_NEEDED tag for each dynamic library mentioned
 on the command line, regardless of whether the library is actually
-needed. @option{--as-needed} causes DT_NEEDED tags to only be emitted
-for libraries that satisfy some reference from regular objects.
+needed.  @option{--as-needed} causes DT_NEEDED tags to only be emitted
+for libraries that satisfy some symbol reference from regular objects
+which is undefined at the point that the library was linked.
 @option{--no-as-needed} restores the default behaviour.
 
 @kindex --add-needed


More information about the Binutils mailing list