Subject: binutils 2.11.92.0.5 is broke (Re: Binutils Bug)

From: "H . J . Lu" <[email-masked]>
Date: Sat, 13 Oct 2001 19:00:34 -0700

On Sat, Oct 13, 2001 at 07:52:07AM -0700, Leonard Zubkoff wrote:
> HJ,
> 
> In recompiling my whole system with your latest binutils-2.11.92.0.5, I
> received the following error while linking telnetd from the netkit-telnet-0.17
> package:
> 
> gcc  telnetd.o state.o termstat.o slc.o sys_term.o utility.o global.o setproctitle.o -lutil -lutil -o telnetd
> /usr/bin/ld: BFD internal error, aborting at elf32-i386.c line 646 in elf_i386_copy_indirect_symbol
> 
> /usr/bin/ld: Please report this bug.
> 
> collect2: ld returned 1 exit status
> 
> Thought you'd want to know...
> 

Hi Alan,

This patch

http://sources.redhat.com/ml/binutils/2001-10/msg00035.html

is incomplete. You cannot do any backend processing when

if (dir == ind->weakdef)

I will double check all backend xxx_hash_copy_indirect.

I am planning to make binutils 2.11.92.0.6 within a week.

Sorry for that.

H.J.

---

From: "H . J . Lu" <[email-masked]>
Date: Sat, 13 Oct 2001 23:56:21 -0700

> I am planning to make binutils 2.11.92.0.6 within a week.

Here is a proposed patch for binutils 2.11.92.0.6. I will run more
tests before releasing it. Please test it as much as you can.

Thanks.

H.J.
---
2001-10-13  H.J. Lu <[email-masked]>

	* elf32-hppa.c (elf32_hppa_copy_indirect_symbol): Don't abort
	if this is a weakdef.
	* elf32-i386.c (elf_i386_copy_indirect_symbol): Likewise.
	* elf64-ppc.c (ppc64_elf_copy_indirect_symbol): Likewise.

	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Set plt.offset
	to -1 and clear the ELF_LINK_HASH_NEEDS_PLT bit if the symbol
	is not a function.
	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Likewise.
	* elf32-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf64-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf64-x86-64.c (elf64_x86_64_adjust_dynamic_symbol):
	Likewise.

Index: elf32-hppa.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf32-hppa.c,v
retrieving revision 1.41
diff -u -p -r1.41 elf32-hppa.c
--- elf32-hppa.c	2001/10/03 15:55:57	1.41
+++ elf32-hppa.c	2001/10/14 06:43:23
@@ -1147,7 +1147,7 @@ elf32_hppa_copy_indirect_symbol (dir, in
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
+  else if (dir != ind->weakdef && eind->dyn_relocs != NULL)
     abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
@@ -1843,6 +1843,11 @@ elf32_hppa_adjust_dynamic_symbol (info, 
 	}
 
       return true;
+    }
+  else
+    {
+      h->plt.offset = (bfd_vma) -1;
+      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
     }
 
   /* If this is a weak symbol, and there is a real definition, the
Index: elf32-i386.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf32-i386.c,v
retrieving revision 1.42
diff -u -p -r1.42 elf32-i386.c
--- elf32-i386.c	2001/10/03 15:55:57	1.42
+++ elf32-i386.c	2001/10/14 06:36:15
@@ -642,7 +642,7 @@ elf_i386_copy_indirect_symbol (dir, ind)
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
+  else if (dir != ind->weakdef && eind->dyn_relocs != NULL)
     abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
Index: elf32-mips.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf32-mips.c,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -p -r1.46 -r1.47
--- elf32-mips.c	2001/10/05 20:31:11	1.46
+++ elf32-mips.c	2001/10/11 18:15:44	1.47
@@ -6319,8 +6319,10 @@ mips_elf_calculate_relocation (abfd,
       if ((info->shared
 	   || (elf_hash_table (info)->dynamic_sections_created
 	       && h != NULL
-	       && ((h->root.elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC)
-		   != 0)))
+	       && ((h->root.elf_link_hash_flags
+		    & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
+	       && ((h->root.elf_link_hash_flags
+		    & ELF_LINK_HASH_DEF_REGULAR) == 0)))
 	  && (input_section->flags & SEC_ALLOC) != 0)
 	{
 	  /* If we're creating a shared library, or this relocation is
Index: elf32-ppc.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf32-ppc.c,v
retrieving revision 1.23
diff -u -p -r1.23 elf32-ppc.c
--- elf32-ppc.c	2001/10/07 23:29:41	1.23
+++ elf32-ppc.c	2001/10/14 05:58:40
@@ -1797,6 +1797,11 @@ ppc_elf_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
+    {
+      h->plt.offset = (bfd_vma) -1;
+      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
+    }
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: elf32-s390.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf32-s390.c,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 elf32-s390.c
--- elf32-s390.c	2001/09/29 16:26:18	1.1.1.8
+++ elf32-s390.c	2001/10/14 06:09:42
@@ -998,6 +998,11 @@ elf_s390_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
+    {
+      h->plt.offset = (bfd_vma) -1;
+      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
+    }
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: elf64-ppc.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf64-ppc.c,v
retrieving revision 1.1.1.5
diff -u -p -r1.1.1.5 elf64-ppc.c
--- elf64-ppc.c	2001/10/03 15:30:41	1.1.1.5
+++ elf64-ppc.c	2001/10/14 06:36:37
@@ -1799,7 +1799,7 @@ ppc64_elf_copy_indirect_symbol (dir, ind
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
+  else if (dir != ind->weakdef && eind->dyn_relocs != NULL)
     abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
Index: elf64-s390.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf64-s390.c,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 elf64-s390.c
--- elf64-s390.c	2001/09/29 16:26:20	1.1.1.8
+++ elf64-s390.c	2001/10/14 06:10:26
@@ -976,6 +976,11 @@ elf_s390_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
+    {
+      h->plt.offset = (bfd_vma) -1;
+      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
+    }
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: elf64-x86-64.c
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elf64-x86-64.c,v
retrieving revision 1.1.1.22
diff -u -p -r1.1.1.22 elf64-x86-64.c
--- elf64-x86-64.c	2001/09/29 16:26:21	1.1.1.22
+++ elf64-x86-64.c	2001/10/14 06:10:46
@@ -854,6 +854,11 @@ elf64_x86_64_adjust_dynamic_symbol (info
 
       return true;
     }
+  else
+    {
+      h->plt.offset = (bfd_vma) -1;
+      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
+    }
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.82
retrieving revision 1.83
diff -u -p -r1.82 -r1.83
--- elflink.h	2001/10/05 20:32:13	1.82
+++ elflink.h	2001/10/11 18:15:44	1.83
@@ -909,8 +909,11 @@ elf_merge_symbol (abfd, info, name, sym,
 
      As above, we again permit a common symbol in a regular object to
      override a definition in a shared object if the shared object
-     symbol is a function or is weak.  */
+     symbol is a function or is weak.
 
+     As above, we permit a non-weak definition in a shared object to
+     override a weak definition in a regular object.  */
+
   if (! newdyn
       && (newdef
 	  || (bfd_is_com_section (sec)
@@ -919,7 +922,8 @@ elf_merge_symbol (abfd, info, name, sym,
       && olddyn
       && olddef
       && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_DYNAMIC) != 0
-      && bind != STB_WEAK)
+      && (bind != STB_WEAK
+	  || h->root.type == bfd_link_hash_defweak))
     {
       /* Change the hash table entry to undefined, and let
 	 _bfd_generic_link_add_one_symbol do the right thing with the
@@ -1022,13 +1026,14 @@ elf_merge_symbol (abfd, info, name, sym,
       *sym_hash = h;
     }
 
-  /* Handle the special case of a definition in a shared object
-     followed by a weak definition in a regular object.  In this case
-     we prefer to definition in the shared object.  To make this work
-     we have to tell the caller to not treat the new symbol as a
-     definition.  */
+  /* Handle the special case of a non-weak definition in a shared
+     object followed by a weak definition in a regular object.  In
+     this case we prefer to definition in the shared object.  To make
+     this work we have to tell the caller to not treat the new symbol
+     as a definition.  */
   if (olddef
       && olddyn
+      && h->root.type != bfd_link_hash_defweak
       && newdef
       && ! newdyn
       && bind == STB_WEAK)

---

From: "H . J . Lu" <[email-masked]>
Date: Sat, 13 Oct 2001 23:59:24 -0700

> tests before releasing it. Please test it as much as you can.

Ooops. Here is the complete changelog against 2.11.92.0.5.

H.J.
----

2001-10-11  H.J. Lu  <[email-masked]>

	* elf32-mips.c (mips_elf_calculate_relocation): Don't create
	dynamic relocation for symbols defined in regular objects when
	creating executables.

2001-10-11  H.J. Lu  <[email-masked]>

	* elflink.h (elf_merge_symbol): Revert the change made on
	2001-10-03.

---

From: Alan Modra <[email-masked]>
Date: Mon, 15 Oct 2001 09:33:17 +0930

> > is incomplete. You cannot do any backend processing when

Err, yes.  Thanks for looking into it, HJ.  I've been away this weekend,
which is why it appears that I've been ignoring the problem.

> Index: elf32-hppa.c

I suspect this is not the correct fix.  dyn_relocs is being used to count
relocs, and probably what should happen is something like

  else if (eind->dyn_relocs != NULL)
    {
      struct elf32_hppa_dyn_reloc_entry *p;

      if (edir != eind->elf.weakdef)
	abort ();

      /* Add reloc counts against the weak sym to the strong sym list.
	 Entries on the eind list should have a different p->sec from
	 any on the dir list, so we don't need to merge entries.  */
      for (p = eind->dyn_relocs; p->next != NULL; p = p->next)
	;
      p->next = edir->dyn_relocs;
      edir->dyn_relocs = eind->dyn_relocs;
      eind->dyn_relocs = NULL;
    }

Untested as yet, because I don't have a testcase.  I'll see if I can
dream one up.

> +      h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;

This part is corrent, and similarly for the other architectures.

> +     override a weak definition in a regular object.  */

I don't disagree with this change, but has this been discussed sufficiently
here and on the glibc list?

Alan

---

From: Alan Modra <[email-masked]>
Date: Mon, 15 Oct 2001 16:45:39 +0930

I'm about to commit this to fix the problems introduced by the new
reference counting scheme.

2001-10-15  Alan Modra  <[email-masked]>
	    H.J. Lu  <[email-masked]>

	* elf32-hppa.c (elf32_hppa_copy_indirect_symbol): Merge dyn_reloc
	counts for aliases instead of aborting.
	* elf32-i386.c (elf_i386_copy_indirect_symbol): Likewise.
	* elf64-ppc.c (ppc64_elf_copy_indirect_symbol): Likewise.

	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Set plt.offset
	to -1 for non-function symbols.
	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Likewise.
	* elf32-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Likewise.
	* elf64-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf64-x86-64.c (elf64_x86_64_adjust_dynamic_symbol): Likewise.
	* elf32-i386.c (elf_i386_adjust_dynamic_symbol): Refer to
	plt.offset instead of plt.refcount when setting to -1.

-- 
Alan Modra

Index: bfd/elf32-hppa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-hppa.c,v
retrieving revision 1.52
diff -u -p -r1.52 elf32-hppa.c
--- elf32-hppa.c	2001/10/03 08:33:18	1.52
+++ elf32-hppa.c	2001/10/15 05:12:07
@@ -1142,13 +1142,41 @@ elf32_hppa_copy_indirect_symbol (dir, in
   edir = (struct elf32_hppa_link_hash_entry *) dir;
   eind = (struct elf32_hppa_link_hash_entry *) ind;
 
-  if (edir->dyn_relocs == NULL)
+  if (eind->dyn_relocs != NULL)
     {
+      if (edir->dyn_relocs != NULL)
+	{
+	  struct elf32_hppa_dyn_reloc_entry **pp;
+	  struct elf32_hppa_dyn_reloc_entry *p;
+
+	  if (dir != ind->weakdef)
+	    abort ();
+
+	  /* Add reloc counts against the weak sym to the strong sym
+	     list.  Merge any entries against the same section.  */
+	  for (pp = &eind->dyn_relocs; (p = *pp) != NULL; )
+	    {
+	      struct elf32_hppa_dyn_reloc_entry *q;
+
+	      for (q = edir->dyn_relocs; q != NULL; q = q->next)
+		if (q->sec == p->sec)
+		  {
+#if RELATIVE_DYNRELOCS
+		    q->relative_count += p->relative_count;
+#endif
+		    q->count += p->count;
+		    *pp = p->next;
+		    break;
+		  }
+	      if (q == NULL)
+		pp = &p->next;
+	    }
+	  *pp = edir->dyn_relocs;
+	}
+
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
-    abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
 }
@@ -1844,6 +1872,8 @@ elf32_hppa_adjust_dynamic_symbol (info, 
 
       return true;
     }
+  else
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf32-i386.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-i386.c,v
retrieving revision 1.54
diff -u -p -r1.54 elf32-i386.c
--- elf32-i386.c	2001/10/03 15:11:46	1.54
+++ elf32-i386.c	2001/10/15 05:12:07
@@ -637,13 +637,39 @@ elf_i386_copy_indirect_symbol (dir, ind)
   edir = (struct elf_i386_link_hash_entry *) dir;
   eind = (struct elf_i386_link_hash_entry *) ind;
 
-  if (edir->dyn_relocs == NULL)
+  if (eind->dyn_relocs != NULL)
     {
+      if (edir->dyn_relocs != NULL)
+	{
+	  struct elf_i386_dyn_relocs **pp;
+	  struct elf_i386_dyn_relocs *p;
+
+	  if (dir != ind->weakdef)
+	    abort ();
+
+	  /* Add reloc counts against the weak sym to the strong sym
+	     list.  Merge any entries against the same section.  */
+	  for (pp = &eind->dyn_relocs; (p = *pp) != NULL; )
+	    {
+	      struct elf_i386_dyn_relocs *q;
+
+	      for (q = edir->dyn_relocs; q != NULL; q = q->next)
+		if (q->sec == p->sec)
+		  {
+		    q->pc_count += p->pc_count;
+		    q->count += p->count;
+		    *pp = p->next;
+		    break;
+		  }
+	      if (q == NULL)
+		pp = &p->next;
+	    }
+	  *pp = edir->dyn_relocs;
+	}
+
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
-    abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
 }
@@ -1086,7 +1112,7 @@ elf_i386_adjust_dynamic_symbol (info, h)
 	     object, or if all references were garbage collected.  In
 	     such a case, we don't actually need to build a procedure
 	     linkage table, and we can just do a PC32 reloc instead.  */
-	  h->plt.refcount = -1;
+	  h->plt.offset = (bfd_vma) -1;
 	  h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
 	}
 
@@ -1098,7 +1124,7 @@ elf_i386_adjust_dynamic_symbol (info, h)
        check_relocs.  We can't decide accurately between function and
        non-function syms in check-relocs;  Objects loaded later in
        the link may change h->type.  So fix it now.  */
-    h->plt.refcount = -1;
+    h->plt.offset = (bfd_vma) -1;
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf32-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-ppc.c,v
retrieving revision 1.32
diff -u -p -r1.32 elf32-ppc.c
--- elf32-ppc.c	2001/09/29 06:21:59	1.32
+++ elf32-ppc.c	2001/10/15 05:12:09
@@ -1797,6 +1797,8 @@ ppc_elf_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf32-s390.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-s390.c,v
retrieving revision 1.9
diff -u -p -r1.9 elf32-s390.c
--- elf32-s390.c	2001/09/29 06:21:59	1.9
+++ elf32-s390.c	2001/10/15 05:12:11
@@ -998,6 +998,8 @@ elf_s390_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf64-ppc.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-ppc.c,v
retrieving revision 1.7
diff -u -p -r1.7 elf64-ppc.c
--- elf64-ppc.c	2001/10/03 08:33:18	1.7
+++ elf64-ppc.c	2001/10/15 05:12:13
@@ -1794,13 +1794,39 @@ ppc64_elf_copy_indirect_symbol (dir, ind
   edir = (struct ppc_link_hash_entry *) dir;
   eind = (struct ppc_link_hash_entry *) ind;
 
-  if (edir->dyn_relocs == NULL)
+  if (eind->dyn_relocs != NULL)
     {
+      if (edir->dyn_relocs != NULL)
+	{
+	  struct ppc_dyn_relocs **pp;
+	  struct ppc_dyn_relocs *p;
+
+	  if (dir != ind->weakdef)
+	    abort ();
+
+	  /* Add reloc counts against the weak sym to the strong sym
+	     list.  Merge any entries against the same section.  */
+	  for (pp = &eind->dyn_relocs; (p = *pp) != NULL; )
+	    {
+	      struct ppc_dyn_relocs *q;
+
+	      for (q = edir->dyn_relocs; q != NULL; q = q->next)
+		if (q->sec == p->sec)
+		  {
+		    q->pc_count += p->pc_count;
+		    q->count += p->count;
+		    *pp = p->next;
+		    break;
+		  }
+	      if (q == NULL)
+		pp = &p->next;
+	    }
+	  *pp = edir->dyn_relocs;
+	}
+
       edir->dyn_relocs = eind->dyn_relocs;
       eind->dyn_relocs = NULL;
     }
-  else if (eind->dyn_relocs != NULL)
-    abort ();
 
   _bfd_elf_link_hash_copy_indirect (dir, ind);
 }
@@ -2366,6 +2392,8 @@ ppc64_elf_adjust_dynamic_symbol (info, h
 	}
       return true;
     }
+  else
+    h->plt.offset = (bfd_vma) -1;
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf64-s390.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-s390.c,v
retrieving revision 1.9
diff -u -p -r1.9 elf64-s390.c
--- elf64-s390.c	2001/09/29 06:21:59	1.9
+++ elf64-s390.c	2001/10/15 05:12:15
@@ -976,6 +976,8 @@ elf_s390_adjust_dynamic_symbol (info, h)
 
       return true;
     }
+  else
 
   /* If this is a weak symbol, and there is a real definition, the
      processor independent code will have arranged for us to see the
Index: bfd/elf64-x86-64.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-x86-64.c,v
retrieving revision 1.29
diff -u -p -r1.29 elf64-x86-64.c
--- elf64-x86-64.c	2001/09/29 06:21:59	1.29
+++ elf64-x86-64.c	2001/10/15 05:12:17
@@ -854,6 +854,8 @@ elf64_x86_64_adjust_dynamic_symbol (info
 
       return true;
     }
+  else