Fix assertion failure on aliases of dynamic weak symbols

Fix assertion failure on aliases of dynamic weak symbols

[Richard Sandiford]

The attached testcase fails on GNU/Linux targets with:

  BFD (GNU Binutils) 2.17.50.20070716 assertion fail .../bfd/elflink.c:2496

A shared library has a weak symbol "foo", and bfd determines that the
strong symbol "data_start" is an alias for "foo" (it has the same section
and value).  The shared library is then involved in another link, and the
script for that link defines "data_start" via a PROVIDE statement.

This works in the same way that it would for a non-PROVIDE assignment;
"data_start" is not used as an alias for "foo".  However, we handle the
PROVIDE case by setting "data_start"'s type to bfd_link_hash_undefined:

  /* If this symbol is being provided by the linker script, and it is
     currently defined by a dynamic object, but not by a regular
     object, then mark it as undefined so that the generic linker will
     force the correct value.  */
  if (provide
      && h->def_dynamic
      && !h->def_regular)
    h->root.type = bfd_link_hash_undefined;
  [...]
  h->def_regular = 1;

whereas _bfd_elf_fix_symbol_flags expects the type to be
bfd_link_hash_defined or bfd_link_hash_defweak.

The testcase is still being handled correctly, and it doesn't seem
appropriate to weaken the assert by checking for the non-obvious
bfd_link_hash_undefined, so I simply moved the assert into the
!def_regular case (i.e. the one where we continue to use the alias).

I've restricted the test to GNU/Linux targets because not all ELF
targets support -shared.

Tested on x86_64-linux-gnu, i686-pc-linux-gnu, arm-eabi, cris-elf,
mips-linux-gnu, mips64-linux-gnu, mipsisa64-elf, powerpc-linux-gnu,
powerpc64-linux-gnu and ia64-linux-gnu.  OK to install?

Richard


bfd/
	* elflink.c (_bfd_elf_fix_symbol_flags): Only assert the type
	of weakdef->root.type if weakdef has no regular definition.

ld/testsuite/
	* ld-elf/weak-dyn-1a.s, ld-elf/weak-dyn-1b.s, ld-elf/weak-dyn-1.ld,
	* ld-elf/weak-dyn-1.rd: New test.
	* ld-elf/elf.exp: Run it.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.269
diff -u -p -r1.269 elflink.c
--- bfd/elflink.c	10 Jul 2007 02:40:31 -0000	1.269
+++ bfd/elflink.c	16 Jul 2007 17:09:19 -0000
@@ -2492,8 +2492,6 @@ _bfd_elf_fix_symbol_flags (struct elf_li
 
       BFD_ASSERT (h->root.type == bfd_link_hash_defined
 		  || h->root.type == bfd_link_hash_defweak);
-      BFD_ASSERT (weakdef->root.type == bfd_link_hash_defined
-		  || weakdef->root.type == bfd_link_hash_defweak);
       BFD_ASSERT (weakdef->def_dynamic);
 
       /* If the real definition is defined by a regular object file,
@@ -2502,8 +2500,11 @@ _bfd_elf_fix_symbol_flags (struct elf_li
       if (weakdef->def_regular)
 	h->u.weakdef = NULL;
       else
-	(*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef,
-						  h);
+	{
+	  BFD_ASSERT (weakdef->root.type == bfd_link_hash_defined
+		      || weakdef->root.type == bfd_link_hash_defweak);
+	  (*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef, h);
+	}
     }
 
   return TRUE;
Index: ld/testsuite/ld-elf/weak-dyn-1a.s
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1a.s
diff -N ld/testsuite/ld-elf/weak-dyn-1a.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1a.s	16 Jul 2007 17:09:19 -0000
@@ -0,0 +1,7 @@
+	.globl	foo
+	.weak	foo
+	.type	foo,%object
+	.size	foo,1
+	.data
+foo:
+	.dc.a	data_begin
Index: ld/testsuite/ld-elf/weak-dyn-1b.s
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1b.s
diff -N ld/testsuite/ld-elf/weak-dyn-1b.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1b.s	16 Jul 2007 17:09:19 -0000
@@ -0,0 +1,3 @@
+	.data
+	.dc.a	foo
+	.dc.a	data_begin
Index: ld/testsuite/ld-elf/weak-dyn-1.ld
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1.ld
diff -N ld/testsuite/ld-elf/weak-dyn-1.ld
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1.ld	16 Jul 2007 17:09:19 -0000
@@ -0,0 +1,8 @@
+SECTIONS
+{
+  . = 0x800000;
+  .data : {
+    PROVIDE (data_begin = .);
+    *(.data)
+  }
+}
Index: ld/testsuite/ld-elf/weak-dyn-1.rd
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1.rd
diff -N ld/testsuite/ld-elf/weak-dyn-1.rd
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1.rd	16 Jul 2007 17:09:19 -0000
@@ -0,0 +1,3 @@
+#...
+0+800000 .* foo.*
+#pass
Index: ld/testsuite/ld-elf/elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/elf.exp,v
retrieving revision 1.10
diff -u -p -r1.10 elf.exp
--- ld/testsuite/ld-elf/elf.exp	6 Jul 2007 14:09:43 -0000	1.10
+++ ld/testsuite/ld-elf/elf.exp	16 Jul 2007 17:09:19 -0000
@@ -36,6 +36,19 @@ foreach t $test_list {
     run_dump_test [file rootname $t]
 }
 
+if { [istarget *-*-linux*] } {
+    run_ld_link_tests {
+	{"Weak symbols in dynamic objects 1 (support)"
+	    "-shared -Tweak-dyn-1.ld" "" {weak-dyn-1a.s}
+	    {}
+	    "libweakdyn1a.so"}
+	{"Weak symbols in dynamic objects 1 (main test)"
+	    "-shared tmpdir/libweakdyn1a.so -Tweak-dyn-1.ld" "" {weak-dyn-1b.s}
+	    {{readelf {--relocs --wide} weak-dyn-1.rd}}
+	    "libweakdyn1b.so"}
+    }
+}
+
 # The following tests require running the executable generated by ld.
 if ![isnative] {
     return

Re: Fix assertion failure on aliases of dynamic weak symbols

[H.J. Lu]

On Mon, Jul 16, 2007 at 06:10:10PM +0100, Richard Sandiford wrote:
> The attached testcase fails on GNU/Linux targets with:
> 
>   BFD (GNU Binutils) 2.17.50.20070716 assertion fail .../bfd/elflink.c:2496
> 
> A shared library has a weak symbol "foo", and bfd determines that the
> strong symbol "data_start" is an alias for "foo" (it has the same section
> and value).  The shared library is then involved in another link, and the
> script for that link defines "data_start" via a PROVIDE statement.

That piece of code was added by Richard on May 3, 1999 with
comments:

  /* 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
     dynamic objects, that's the only time we actually put anything on
     the list WEAKS.  We need this information so that if a regular
     object refers to a symbol defined weakly in a dynamic object, the
     real symbol in the dynamic object is also put in the dynamic
     symbols; we also must arrange for both symbols to point to the
     same memory location.  We could handle the general case of symbol
     aliasing, but a general symbol alias can only be generated in
     assembler code, handling it correctly would be very time
     consuming, and other ELF linkers don't handle general aliasing
     either.  */

It no longer applies today since there is no difference between
weak def and strong def in a shared library. Richard, do you
remember if there is a testcase for your change? Can we remove
the whole u.weakdef thing now? It will make linker simpler and
faster.

Thanks.


H.J.

PATCH: Remove u.weakdef from elf_link_hash_entry

[H.J. Lu]

On Mon, Jul 16, 2007 at 01:35:05PM -0700, H.J. Lu wrote:
> On Mon, Jul 16, 2007 at 06:10:10PM +0100, Richard Sandiford wrote:
> > The attached testcase fails on GNU/Linux targets with:
> > 
> >   BFD (GNU Binutils) 2.17.50.20070716 assertion fail .../bfd/elflink.c:2496
> > 
> > A shared library has a weak symbol "foo", and bfd determines that the
> > strong symbol "data_start" is an alias for "foo" (it has the same section
> > and value).  The shared library is then involved in another link, and the
> > script for that link defines "data_start" via a PROVIDE statement.
> 
> That piece of code was added by Richard on May 3, 1999 with
> comments:
> 
>   /* 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
>      dynamic objects, that's the only time we actually put anything on
>      the list WEAKS.  We need this information so that if a regular
>      object refers to a symbol defined weakly in a dynamic object, the
>      real symbol in the dynamic object is also put in the dynamic
>      symbols; we also must arrange for both symbols to point to the
>      same memory location.  We could handle the general case of symbol
>      aliasing, but a general symbol alias can only be generated in
>      assembler code, handling it correctly would be very time
>      consuming, and other ELF linkers don't handle general aliasing
>      either.  */
> 
> It no longer applies today since there is no difference between
> weak def and strong def in a shared library. Richard, do you
> remember if there is a testcase for your change? Can we remove
> the whole u.weakdef thing now? It will make linker simpler and
> faster.
> 

Here is a patch. Tested on Linux/ia32, Linux/x86-64 and Linux/ia64.


H.J.
---
2007-07-16  H.J. Lu  <hongjiu.lu@intel.com>

	* elf-bfd.h (elf_link_hash_entry): Remove u.weakdef. Change
	u.elf_hash_value to elf_hash_value.
	* elf-m10200.c (_bfd_mn10300_elf_adjust_dynamic_symbol): Updated.
	* elf32-arm.c (elf32_arm_adjust_dynamic_symbol): Likewise.
	* elf32-bfin.c (elf32_bfinfdpic_adjust_dynamic_symbol): Likewise.
	(bfin_adjust_dynamic_symbol): Likewise.
	* elf32-cris.c (elf_cris_adjust_dynamic_symbol): Likewise.
	* elf32-frv.c (elf32_frvfdpic_adjust_dynamic_symbol): Likewise.
	* elf32-hppa.c (elf32_hppa_adjust_dynamic_symbol): Likewise.
	* elf32-i370.c (i370_elf_adjust_dynamic_symbol): Likewise.
	* elf32-i386.c (elf_i386_adjust_dynamic_symbol): Likewise.
	* elf32-m32r.c (m32r_elf_adjust_dynamic_symbol): Likewise.
	* elf32-m68k.c (elf_m68k_adjust_dynamic_symbol): Likewise.
	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Likewise.
	* elf32-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf32-score.c (_bfd_score_elf_adjust_dynamic_symbol): Likewise.
	* elf32-sh.c (sh_elf_adjust_dynamic_symbol): Likewise.
	* elf32-vax.c (elf_vax_adjust_dynamic_symbol): Likewise.
	* elf32-xtensa.c (elf_xtensa_adjust_dynamic_symbol): Likewise.
	* elf64-alpha.c (elf64_alpha_adjust_dynamic_symbol): Likewise.
	* elf64-hppa.c (elf64_hppa_adjust_dynamic_symbol): Likewise.
	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Likewise.
	* elf64-s390.c (elf_s390_adjust_dynamic_symbol): Likewise.
	* elf64-sh64.c (sh64_elf64_adjust_dynamic_symbol): Likewise.
	* elf64-x86-64.c (elf64_x86_64_adjust_dynamic_symbol): Likewise.
	* elflink.c (bfd_elf_record_link_assignment): Likewise.
	(_bfd_elf_fix_symbol_flags): Likewise.
	(_bfd_elf_adjust_dynamic_symbol): Likewise.
	(elf_link_add_object_symbols): Likewise.
	(elf_collect_hash_codes): Likewise.
	(elf_link_output_extsym): Likewise.
	* elfxx-ia64.c (allocate_dynrel_entries): Likewise.
	* elfxx-mips.c (_bfd_mips_elf_adjust_dynamic_symbol): Likewise.
	(_bfd_mips_vxworks_adjust_dynamic_symbol): Likewise.
	* elfxx-sparc.c (_bfd_sparc_elf_adjust_dynamic_symbol): Likewise.

	* elflink.c (elf_sort_symbol): Removed.

--- bfd/elf-bfd.h.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf-bfd.h	2007-07-16 13:55:21.000000000 -0700
@@ -176,18 +176,9 @@ struct elf_link_hash_entry
   /* String table index in .dynstr if this is a dynamic symbol.  */
   unsigned long dynstr_index;
 
-  union
-  {
-    /* If this is a weak defined symbol from a dynamic object, this
-       field points to a defined symbol with the same value, if there is
-       one.  Otherwise it is NULL.  */
-    struct elf_link_hash_entry *weakdef;
-
-    /* Hash value of the name computed using the ELF hash function.
-       Used part way through size_dynamic_sections, after we've finished
-       with weakdefs.  */
-    unsigned long elf_hash_value;
-  } u;
+  /* Hash value of the name computed using the ELF hash function.
+     Used part way through size_dynamic_sections.  */
+  unsigned long elf_hash_value;
 
   /* Version information.  */
   union
--- bfd/elf-m10300.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf-m10300.c	2007-07-16 14:06:52.000000000 -0700
@@ -4103,7 +4103,6 @@ _bfd_mn10300_elf_adjust_dynamic_symbol (
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -4175,18 +4174,6 @@ _bfd_mn10300_elf_adjust_dynamic_symbol (
       return TRUE;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-arm.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-arm.c	2007-07-16 14:06:34.000000000 -0700
@@ -8034,7 +8034,6 @@ elf32_arm_adjust_dynamic_symbol (struct 
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -8075,18 +8074,6 @@ elf32_arm_adjust_dynamic_symbol (struct 
       eh->plt_thumb_refcount = 0;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* If there are no non-GOT references, we do not need a copy
      relocation.  */
   if (!h->non_got_ref)
--- bfd/elf32-bfin.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-bfin.c	2007-07-16 14:09:46.000000000 -0700
@@ -4323,21 +4323,9 @@ elf32_bfinfdpic_adjust_dynamic_symbol
 
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
-	      && (h->u.weakdef != NULL
-		  || (h->def_dynamic
-		      && h->ref_regular
-		      && !h->def_regular)));
-
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-    }
+	      && h->def_dynamic
+	      && h->ref_regular
+	      && !h->def_regular);
 
   return TRUE;
 }
@@ -5074,8 +5062,9 @@ bfin_adjust_dynamic_symbol (struct bfd_l
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
-		  || (h->def_dynamic && h->ref_regular && !h->def_regular)));
+		  || (h->def_dynamic
+		      && h->ref_regular
+		      && !h->def_regular)));
 
   /* If this is a function, put it in the procedure linkage table.  We
      will fill in the contents of the procedure linkage table later,
@@ -5085,18 +5074,6 @@ bfin_adjust_dynamic_symbol (struct bfd_l
       BFD_ASSERT(0);
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-cris.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-cris.c	2007-07-16 14:09:20.000000000 -0700
@@ -2229,7 +2229,6 @@ elf_cris_adjust_dynamic_symbol (info, h)
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -2368,18 +2367,6 @@ elf_cris_adjust_dynamic_symbol (info, h)
      count any more.  */
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-frv.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-frv.c	2007-07-16 14:10:11.000000000 -0700
@@ -5992,21 +5992,9 @@ elf32_frvfdpic_adjust_dynamic_symbol
 
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
-	      && (h->u.weakdef != NULL
-		  || (h->def_dynamic
-		      && h->ref_regular
-		      && !h->def_regular)));
-
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-    }
+	      && h->def_dynamic
+	      && h->ref_regular
+	      && !h->def_regular);
 
   return TRUE;
 }
--- bfd/elf32-hppa.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-hppa.c	2007-07-16 14:10:51.000000000 -0700
@@ -1849,21 +1849,6 @@ elf32_hppa_adjust_dynamic_symbol (struct
   else
     eh->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
-     real definition first, and we can just use the same value.  */
-  if (eh->u.weakdef != NULL)
-    {
-      if (eh->u.weakdef->root.type != bfd_link_hash_defined
-	  && eh->u.weakdef->root.type != bfd_link_hash_defweak)
-	abort ();
-      eh->root.u.def.section = eh->u.weakdef->root.u.def.section;
-      eh->root.u.def.value = eh->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS)
-	eh->non_got_ref = eh->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-i370.c.weakdef	2007-07-03 10:50:59.000000000 -0700
+++ bfd/elf32-i370.c	2007-07-16 14:10:33.000000000 -0700
@@ -470,7 +470,6 @@ i370_elf_adjust_dynamic_symbol (struct b
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -479,18 +478,6 @@ i370_elf_adjust_dynamic_symbol (struct b
   BFD_ASSERT (s != NULL);
   s->size += sizeof (Elf32_External_Rela);
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-i386.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-i386.c	2007-07-16 13:58:59.000000000 -0700
@@ -1464,20 +1464,6 @@ elf_i386_adjust_dynamic_symbol (struct b
        the link may change h->type.  So fix it now.  */
     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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS || info->nocopyreloc)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-m32r.c.weakdef	2007-07-03 10:51:00.000000000 -0700
+++ bfd/elf32-m32r.c	2007-07-16 14:11:07.000000000 -0700
@@ -1844,7 +1844,6 @@ m32r_elf_adjust_dynamic_symbol (struct b
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
               && (h->needs_plt
-                  || h->u.weakdef != NULL
                   || (h->def_dynamic
                       && h->ref_regular
                       && !h->def_regular)));
@@ -1875,18 +1874,6 @@ m32r_elf_adjust_dynamic_symbol (struct b
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-                  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-m68k.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-m68k.c	2007-07-16 14:11:21.000000000 -0700
@@ -1223,7 +1223,6 @@ elf_m68k_adjust_dynamic_symbol (info, h)
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -1304,18 +1303,6 @@ elf_m68k_adjust_dynamic_symbol (info, h)
      count any more.  */
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-ppc.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-ppc.c	2007-07-16 14:14:44.000000000 -0700
@@ -2688,7 +2688,9 @@ ppc_elf_copy_indirect_symbol (struct bfd
 
   /* If called to transfer flags for a weakdef during processing
      of elf_adjust_dynamic_symbol, don't copy non_got_ref.
-     We clear it ourselves for ELIMINATE_COPY_RELOCS.  */
+     We clear it ourselves for ELIMINATE_COPY_RELOCS.
+     
+     FIXME: weakdef is gone. Can we do it unconditionally?  */
   if (!(ELIMINATE_COPY_RELOCS
 	&& eind->elf.root.type != bfd_link_hash_indirect
 	&& edir->elf.dynamic_adjusted))
@@ -4217,7 +4219,6 @@ ppc_elf_adjust_dynamic_symbol (struct bf
   htab = ppc_elf_hash_table (info);
   BFD_ASSERT (htab->elf.dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -4255,20 +4256,6 @@ ppc_elf_adjust_dynamic_symbol (struct bf
   else
     h->plt.plist = NULL;
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-s390.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-s390.c	2007-07-16 14:15:35.000000000 -0700
@@ -922,7 +922,9 @@ elf_s390_copy_indirect_symbol (info, dir
     {
       /* If called to transfer flags for a weakdef during processing
 	 of elf_adjust_dynamic_symbol, don't copy non_got_ref.
-	 We clear it ourselves for ELIMINATE_COPY_RELOCS.  */
+	 We clear it ourselves for ELIMINATE_COPY_RELOCS.
+	 
+	 FIXME: weakdef is gone. Can we do it unconditionally?  */
       dir->ref_dynamic |= ind->ref_dynamic;
       dir->ref_regular |= ind->ref_regular;
       dir->ref_regular_nonweak |= ind->ref_regular_nonweak;
@@ -1616,20 +1618,6 @@ elf_s390_adjust_dynamic_symbol (info, h)
        the link may change h->type.  So fix it now.  */
     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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS || info->nocopyreloc)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-score.c.weakdef	2007-07-03 10:51:01.000000000 -0700
+++ bfd/elf32-score.c	2007-07-16 14:16:24.000000000 -0700
@@ -2816,8 +2816,9 @@ _bfd_score_elf_adjust_dynamic_symbol (st
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
               && (h->needs_plt
-                  || h->u.weakdef != NULL
-                  || (h->def_dynamic && h->ref_regular && !h->def_regular)));
+                  || (h->def_dynamic
+		      && h->ref_regular
+		      && !h->def_regular)));
 
   /* If this symbol is defined in a dynamic object, we need to copy
      any R_SCORE_ABS32 or R_SCORE_REL32 relocs against it into the output
@@ -2872,18 +2873,6 @@ _bfd_score_elf_adjust_dynamic_symbol (st
       return TRUE;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-                  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
   return TRUE;
--- bfd/elf32-sh.c.weakdef	2007-07-03 10:51:01.000000000 -0700
+++ bfd/elf32-sh.c	2007-07-16 14:15:56.000000000 -0700
@@ -2496,7 +2496,6 @@ sh_elf_adjust_dynamic_symbol (struct bfd
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (htab->root.dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -2526,20 +2525,6 @@ sh_elf_adjust_dynamic_symbol (struct bfd
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (info->nocopyreloc)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
@@ -4795,7 +4780,9 @@ sh_elf_copy_indirect_symbol (struct bfd_
     {
       /* If called to transfer flags for a weakdef during processing
 	 of elf_adjust_dynamic_symbol, don't copy non_got_ref.
-	 We clear it ourselves for ELIMINATE_COPY_RELOCS.  */
+	 We clear it ourselves for ELIMINATE_COPY_RELOCS.
+
+	 FIXME: weakdef is gone. Can we do it unconditionally?  */
       dir->ref_dynamic |= ind->ref_dynamic;
       dir->ref_regular |= ind->ref_regular;
       dir->ref_regular_nonweak |= ind->ref_regular_nonweak;
--- bfd/elf32-vax.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-vax.c	2007-07-16 14:17:07.000000000 -0700
@@ -932,7 +932,6 @@ elf_vax_adjust_dynamic_symbol (info, h)
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -1023,18 +1022,6 @@ elf_vax_adjust_dynamic_symbol (info, h)
      count any more.  */
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf32-xtensa.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf32-xtensa.c	2007-07-16 14:17:37.000000000 -0700
@@ -1201,20 +1201,8 @@ add_extra_plt_sections (struct bfd_link_
 
 static bfd_boolean
 elf_xtensa_adjust_dynamic_symbol (struct bfd_link_info *info ATTRIBUTE_UNUSED,
-				  struct elf_link_hash_entry *h)
+				  struct elf_link_hash_entry *h ATTRIBUTE_UNUSED)
 {
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object.  The
      reference must go through the GOT, so there's no need for COPY relocs,
      .dynbss, etc.  */
--- bfd/elf64-alpha.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf64-alpha.c	2007-07-16 14:03:13.000000000 -0700
@@ -2038,18 +2038,6 @@ elf64_alpha_adjust_dynamic_symbol (struc
   else
     h->needs_plt = FALSE;
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  The Alpha, since it uses .got entries for all
      symbols even in regular objects, does not need the hackery of a
--- bfd/elf64-hppa.c.weakdef	2007-07-10 07:40:58.000000000 -0700
+++ bfd/elf64-hppa.c	2007-07-16 14:04:27.000000000 -0700
@@ -1537,23 +1537,11 @@ allocate_dynrel_entries (dyn_h, data)
 static bfd_boolean
 elf64_hppa_adjust_dynamic_symbol (info, h)
      struct bfd_link_info *info ATTRIBUTE_UNUSED;
-     struct elf_link_hash_entry *h;
+     struct elf_link_hash_entry *h ATTRIBUTE_UNUSED;
 {
   /* ??? Undefined symbols with PLT entries should be re-defined
      to be the PLT entry.  */
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* If this is a reference to a symbol defined by a dynamic object which
      is not a function, we might allocate the symbol in our .dynbss section
      and allocate a COPY dynamic relocation.
--- bfd/elf64-ppc.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf64-ppc.c	2007-07-16 14:05:01.000000000 -0700
@@ -5825,20 +5825,6 @@ ppc64_elf_adjust_dynamic_symbol (struct 
   else
     h->plt.plist = NULL;
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* If we are creating a shared library, we must presume that the
      only references to the symbol are via the global offset table.
      For such cases we need not do anything here; the relocations will
--- bfd/elf64-s390.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf64-s390.c	2007-07-16 14:05:36.000000000 -0700
@@ -1591,20 +1591,6 @@ elf_s390_adjust_dynamic_symbol (info, h)
        the link may change h->type.  So fix it now.  */
     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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS || info->nocopyreloc)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf64-sh64.c.weakdef	2007-07-03 10:51:05.000000000 -0700
+++ bfd/elf64-sh64.c	2007-07-16 14:05:51.000000000 -0700
@@ -3334,7 +3334,6 @@ sh64_elf64_adjust_dynamic_symbol (struct
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -3406,18 +3405,6 @@ sh64_elf64_adjust_dynamic_symbol (struct
       return TRUE;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
--- bfd/elf64-x86-64.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elf64-x86-64.c	2007-07-16 13:55:52.000000000 -0700
@@ -1332,20 +1332,6 @@ elf64_x86_64_adjust_dynamic_symbol (stru
        the link may change h->type.  So fix it now.  */
     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
-     real definition first, and we can just use the same value.	 */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      if (ELIMINATE_COPY_RELOCS || info->nocopyreloc)
-	h->non_got_ref = h->u.weakdef->non_got_ref;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.	 */
 
--- bfd/elflink.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elflink.c	2007-07-16 13:59:49.000000000 -0700
@@ -580,21 +580,9 @@ bfd_elf_record_link_assignment (bfd *out
        || h->ref_dynamic
        || info->shared
        || (info->executable && elf_hash_table (info)->is_relocatable_executable))
-      && h->dynindx == -1)
-    {
-      if (! bfd_elf_link_record_dynamic_symbol (info, h))
-	return FALSE;
-
-      /* If this is a weak defined symbol, and we know a corresponding
-	 real symbol from the same dynamic object, make sure the real
-	 symbol is also made into a dynamic symbol.  */
-      if (h->u.weakdef != NULL
-	  && h->u.weakdef->dynindx == -1)
-	{
-	  if (! bfd_elf_link_record_dynamic_symbol (info, h->u.weakdef))
-	    return FALSE;
-	}
-    }
+      && h->dynindx == -1
+      && ! bfd_elf_link_record_dynamic_symbol (info, h))
+    return FALSE;
 
   return TRUE;
 }
@@ -2519,33 +2507,6 @@ _bfd_elf_fix_symbol_flags (struct elf_li
       (*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
     }
 
-  /* If this is a weak defined symbol in a dynamic object, and we know
-     the real definition in the dynamic object, copy interesting flags
-     over to the real definition.  */
-  if (h->u.weakdef != NULL)
-    {
-      struct elf_link_hash_entry *weakdef;
-
-      weakdef = h->u.weakdef;
-      if (h->root.type == bfd_link_hash_indirect)
-	h = (struct elf_link_hash_entry *) h->root.u.i.link;
-
-      BFD_ASSERT (h->root.type == bfd_link_hash_defined
-		  || h->root.type == bfd_link_hash_defweak);
-      BFD_ASSERT (weakdef->root.type == bfd_link_hash_defined
-		  || weakdef->root.type == bfd_link_hash_defweak);
-      BFD_ASSERT (weakdef->def_dynamic);
-
-      /* If the real definition is defined by a regular object file,
-	 don't do anything special.  See the longer description in
-	 _bfd_elf_adjust_dynamic_symbol, below.  */
-      if (weakdef->def_regular)
-	h->u.weakdef = NULL;
-      else
-	(*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef,
-						  h);
-    }
-
   return TRUE;
 }
 
@@ -2592,8 +2553,7 @@ _bfd_elf_adjust_dynamic_symbol (struct e
   if (!h->needs_plt
       && (h->def_regular
 	  || !h->def_dynamic
-	  || (!h->ref_regular
-	      && (h->u.weakdef == NULL || h->u.weakdef->dynindx == -1))))
+	  || !h->ref_regular))
     {
       h->plt = elf_hash_table (eif->info)->init_plt_offset;
       return TRUE;
@@ -2610,46 +2570,6 @@ _bfd_elf_adjust_dynamic_symbol (struct e
      recursively later after REF_REGULAR is set below.  */
   h->dynamic_adjusted = 1;
 
-  /* If this is a weak definition, and we know a real definition, and
-     the real symbol is not itself defined by a regular object file,
-     then get a good value for the real definition.  We handle the
-     real symbol first, for the convenience of the backend routine.
-
-     Note that there is a confusing case here.  If the real definition
-     is defined by a regular object file, we don't get the real symbol
-     from the dynamic object, but we do get the weak symbol.  If the
-     processor backend uses a COPY reloc, then if some routine in the
-     dynamic object changes the real symbol, we will not see that
-     change in the corresponding weak symbol.  This is the way other
-     ELF linkers work as well, and seems to be a result of the shared
-     library model.
-
-     I will clarify this issue.  Most SVR4 shared libraries define the
-     variable _timezone and define timezone as a weak synonym.  The
-     tzset call changes _timezone.  If you write
-       extern int timezone;
-       int _timezone = 5;
-       int main () { tzset (); printf ("%d %d\n", timezone, _timezone); }
-     you might expect that, since timezone is a synonym for _timezone,
-     the same number will print both times.  However, if the processor
-     backend uses a COPY reloc, then actually timezone will be copied
-     into your process image, and, since you define _timezone
-     yourself, _timezone will not.  Thus timezone and _timezone will
-     wind up at different memory locations.  The tzset call will set
-     _timezone, leaving timezone unchanged.  */
-
-  if (h->u.weakdef != NULL)
-    {
-      /* If we get to this point, we know there is an implicit
-	 reference by a regular object file via the weak symbol H.
-	 FIXME: Is this really true?  What if the traversal finds
-	 H->U.WEAKDEF before it finds H?  */
-      h->u.weakdef->ref_regular = 1;
-
-      if (! _bfd_elf_adjust_dynamic_symbol (h->u.weakdef, eif))
-	return FALSE;
-    }
-
   /* If a symbol has no type and no size and does not require a PLT
      entry, then we are probably about to do the wrong thing here: we
      are probably going to create a COPY reloc for an empty object.
@@ -3126,28 +3046,6 @@ elf_add_dt_needed_tag (bfd *abfd,
   return 0;
 }
 
-/* Sort symbol by value and section.  */
-static int
-elf_sort_symbol (const void *arg1, const void *arg2)
-{
-  const struct elf_link_hash_entry *h1;
-  const struct elf_link_hash_entry *h2;
-  bfd_signed_vma vdiff;
-
-  h1 = *(const struct elf_link_hash_entry **) arg1;
-  h2 = *(const struct elf_link_hash_entry **) arg2;
-  vdiff = h1->root.u.def.value - h2->root.u.def.value;
-  if (vdiff != 0)
-    return vdiff > 0 ? 1 : -1;
-  else
-    {
-      long sdiff = h1->root.u.def.section->id - h2->root.u.def.section->id;
-      if (sdiff != 0)
-	return sdiff > 0 ? 1 : -1;
-    }
-  return 0;
-}
-
 /* This function is used to adjust offsets into .dynstr for
    dynamic symbols.  This is called via elf_link_hash_traverse.  */
 
@@ -3303,7 +3201,6 @@ elf_link_add_object_symbols (bfd *abfd, 
   bfd_boolean dynamic;
   Elf_External_Versym *extversym = NULL;
   Elf_External_Versym *ever;
-  struct elf_link_hash_entry *weaks;
   struct elf_link_hash_entry **nondeflt_vers = NULL;
   bfd_size_type nondeflt_vers_cnt = 0;
   Elf_Internal_Sym *isymbuf = NULL;
@@ -3771,7 +3668,6 @@ elf_link_add_object_symbols (bfd *abfd, 
 	}
     }
 
-  weaks = NULL;
   ever = extversym != NULL ? extversym + extsymoff : NULL;
   for (isym = isymbuf, isymend = isymbuf + extsymcount;
        isym < isymend;
@@ -4082,30 +3978,6 @@ elf_link_add_object_symbols (bfd *abfd, 
       *sym_hash = h;
 
       new_weakdef = FALSE;
-      if (dynamic
-	  && definition
-	  && (flags & BSF_WEAK) != 0
-	  && !bed->is_function_type (ELF_ST_TYPE (isym->st_info))
-	  && is_elf_hash_table (htab)
-	  && h->u.weakdef == NULL)
-	{
-	  /* Keep a list of all weak defined non function symbols from
-	     a dynamic object, using the weakdef field.  Later in this
-	     function we will set the weakdef field to the correct
-	     value.  We only put non-function symbols from dynamic
-	     objects on this list, because that happens to be the only
-	     time we need to know the normal symbol corresponding to a
-	     weak symbol, and the information is time consuming to
-	     figure out.  If the weakdef field is not already NULL,
-	     then this symbol was already defined by some previous
-	     dynamic object, and we will be using that previous
-	     definition anyhow.  */
-
-	  h->u.weakdef = weaks;
-	  weaks = h;
-	  new_weakdef = TRUE;
-	}
-
       /* Set the alignment of a common symbol.  */
       if ((common || bfd_is_com_section (sec))
 	  && h->root.type == bfd_link_hash_common)
@@ -4308,10 +4180,7 @@ elf_link_add_object_symbols (bfd *abfd, 
 		 make the real symbol dynamic.  */
 	      if ((h == hi || !hi->forced_local)
 		  && (h->def_regular
-		      || h->ref_regular
-		      || (h->u.weakdef != NULL
-			  && ! new_weakdef
-			  && h->u.weakdef->dynindx != -1)))
+		      || h->ref_regular))
 		dynsym = TRUE;
 	    }
 
@@ -4351,13 +4220,6 @@ elf_link_add_object_symbols (bfd *abfd, 
 	    {
 	      if (! bfd_elf_link_record_dynamic_symbol (info, h))
 		goto error_free_vers;
-	      if (h->u.weakdef != NULL
-		  && ! new_weakdef
-		  && h->u.weakdef->dynindx == -1)
-		{
-		  if (!bfd_elf_link_record_dynamic_symbol (info, h->u.weakdef))
-		    goto error_free_vers;
-		}
 	    }
 	  else if (dynsym && h->dynindx != -1)
 	    /* If the symbol already has a dynamic index, but
@@ -4530,145 +4392,6 @@ elf_link_add_object_symbols (bfd *abfd, 
       nondeflt_vers = 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
-     dynamic objects, that's the only time we actually put anything on
-     the list WEAKS.  We need this information so that if a regular
-     object refers to a symbol defined weakly in a dynamic object, the
-     real symbol in the dynamic object is also put in the dynamic
-     symbols; we also must arrange for both symbols to point to the
-     same memory location.  We could handle the general case of symbol
-     aliasing, but a general symbol alias can only be generated in
-     assembler code, handling it correctly would be very time
-     consuming, and other ELF linkers don't handle general aliasing
-     either.  */
-  if (weaks != NULL)
-    {
-      struct elf_link_hash_entry **hpp;
-      struct elf_link_hash_entry **hppend;
-      struct elf_link_hash_entry **sorted_sym_hash;
-      struct elf_link_hash_entry *h;
-      size_t sym_count;
-
-      /* Since we have to search the whole symbol list for each weak
-	 defined symbol, search time for N weak defined symbols will be
-	 O(N^2). Binary search will cut it down to O(NlogN).  */
-      amt = extsymcount * sizeof (struct elf_link_hash_entry *);
-      sorted_sym_hash = bfd_malloc (amt);
-      if (sorted_sym_hash == NULL)
-	goto error_return;
-      sym_hash = sorted_sym_hash;
-      hpp = elf_sym_hashes (abfd);
-      hppend = hpp + extsymcount;
-      sym_count = 0;
-      for (; hpp < hppend; hpp++)
-	{
-	  h = *hpp;
-	  if (h != NULL
-	      && h->root.type == bfd_link_hash_defined
-	      && !bed->is_function_type (h->type))
-	    {
-	      *sym_hash = h;
-	      sym_hash++;
-	      sym_count++;
-	    }
-	}
-
-      qsort (sorted_sym_hash, sym_count,
-	     sizeof (struct elf_link_hash_entry *),
-	     elf_sort_symbol);
-
-      while (weaks != NULL)
-	{
-	  struct elf_link_hash_entry *hlook;
-	  asection *slook;
-	  bfd_vma vlook;
-	  long ilook;
-	  size_t i, j, idx;
-
-	  hlook = weaks;
-	  weaks = hlook->u.weakdef;
-	  hlook->u.weakdef = NULL;
-
-	  BFD_ASSERT (hlook->root.type == bfd_link_hash_defined
-		      || hlook->root.type == bfd_link_hash_defweak
-		      || hlook->root.type == bfd_link_hash_common
-		      || hlook->root.type == bfd_link_hash_indirect);
-	  slook = hlook->root.u.def.section;
-	  vlook = hlook->root.u.def.value;
-
-	  ilook = -1;
-	  i = 0;
-	  j = sym_count;
-	  while (i < j)
-	    {
-	      bfd_signed_vma vdiff;
-	      idx = (i + j) / 2;
-	      h = sorted_sym_hash [idx];
-	      vdiff = vlook - h->root.u.def.value;
-	      if (vdiff < 0)
-		j = idx;
-	      else if (vdiff > 0)
-		i = idx + 1;
-	      else
-		{
-		  long sdiff = slook->id - h->root.u.def.section->id;
-		  if (sdiff < 0)
-		    j = idx;
-		  else if (sdiff > 0)
-		    i = idx + 1;
-		  else
-		    {
-		      ilook = idx;
-		      break;
-		    }
-		}
-	    }
-
-	  /* We didn't find a value/section match.  */
-	  if (ilook == -1)
-	    continue;
-
-	  for (i = ilook; i < sym_count; i++)
-	    {
-	      h = sorted_sym_hash [i];
-
-	      /* Stop if value or section doesn't match.  */
-	      if (h->root.u.def.value != vlook
-		  || h->root.u.def.section != slook)
-		break;
-	      else if (h != hlook)
-		{
-		  hlook->u.weakdef = h;
-
-		  /* If the weak definition is in the list of dynamic
-		     symbols, make sure the real definition is put
-		     there as well.  */
-		  if (hlook->dynindx != -1 && h->dynindx == -1)
-		    {
-		      if (! bfd_elf_link_record_dynamic_symbol (info, h))
-			goto error_return;
-		    }
-
-		  /* If the real definition is in the list of dynamic
-		     symbols, make sure the weak definition is put
-		     there as well.  If we don't do this, then the
-		     dynamic loader might not merge the entries for the
-		     real definition and the weak definition.  */
-		  if (h->dynindx != -1 && hlook->dynindx == -1)
-		    {
-		      if (! bfd_elf_link_record_dynamic_symbol (info, hlook))
-			goto error_return;
-		    }
-		  break;
-		}
-	    }
-	}
-
-      free (sorted_sym_hash);
-    }
-
   if (bed->check_directives)
     (*bed->check_directives) (abfd, info);
 
@@ -5081,7 +4804,7 @@ elf_collect_hash_codes (struct elf_link_
 
   /* And store it in the struct so that we can put it in the hash table
      later.  */
-  h->u.elf_hash_value = ha;
+  h->elf_hash_value = ha;
 
   if (alc != NULL)
     free (alc);
@@ -8191,7 +7914,7 @@ elf_link_output_extsym (struct elf_link_
 	  size_t bucket;
 
 	  bucketcount = elf_hash_table (finfo->info)->bucketcount;
-	  bucket = h->u.elf_hash_value % bucketcount;
+	  bucket = h->elf_hash_value % bucketcount;
 
 	  hash_entry_size
 	    = elf_section_data (finfo->hash_sec)->this_hdr.sh_entsize;
--- bfd/elfxx-ia64.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elfxx-ia64.c	2007-07-16 14:04:43.000000000 -0700
@@ -3580,23 +3580,11 @@ allocate_dynrel_entries (dyn_i, data)
 static bfd_boolean
 elfNN_ia64_adjust_dynamic_symbol (info, h)
      struct bfd_link_info *info ATTRIBUTE_UNUSED;
-     struct elf_link_hash_entry *h;
+     struct elf_link_hash_entry *h ATTRIBUTE_UNUSED;
 {
   /* ??? Undefined symbols with PLT entries should be re-defined
      to be the PLT entry.  */
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-                  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* If this is a reference to a symbol defined by a dynamic object which
      is not a function, we might allocate the symbol in our .dynbss section
      and allocate a COPY dynamic relocation.
--- bfd/elfxx-mips.c.weakdef	2007-07-03 10:51:08.000000000 -0700
+++ bfd/elfxx-mips.c	2007-07-16 14:11:47.000000000 -0700
@@ -6934,7 +6934,6 @@ _bfd_mips_elf_adjust_dynamic_symbol (str
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -6997,18 +6996,6 @@ _bfd_mips_elf_adjust_dynamic_symbol (str
       return TRUE;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 
@@ -7033,7 +7020,6 @@ _bfd_mips_vxworks_adjust_dynamic_symbol 
   BFD_ASSERT (dynobj != NULL
 	      && (h->needs_plt
 		  || h->needs_copy
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -7113,18 +7099,6 @@ _bfd_mips_vxworks_adjust_dynamic_symbol 
       return TRUE;
     }
 
-  /* If this is a weak symbol, and there is a real definition, the
-     processor independent code will have arranged for us to see the
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
   if (info->shared)
--- bfd/elfxx-sparc.c.weakdef	2007-07-16 12:41:04.000000000 -0700
+++ bfd/elfxx-sparc.c	2007-07-16 14:16:41.000000000 -0700
@@ -1709,7 +1709,6 @@ _bfd_sparc_elf_adjust_dynamic_symbol (st
   /* Make sure we know what is going on here.  */
   BFD_ASSERT (htab->elf.dynobj != NULL
 	      && (h->needs_plt
-		  || h->u.weakdef != NULL
 		  || (h->def_dynamic
 		      && h->ref_regular
 		      && !h->def_regular)));
@@ -1749,18 +1748,6 @@ _bfd_sparc_elf_adjust_dynamic_symbol (st
   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
-     real definition first, and we can just use the same value.  */
-  if (h->u.weakdef != NULL)
-    {
-      BFD_ASSERT (h->u.weakdef->root.type == bfd_link_hash_defined
-		  || h->u.weakdef->root.type == bfd_link_hash_defweak);
-      h->root.u.def.section = h->u.weakdef->root.u.def.section;
-      h->root.u.def.value = h->u.weakdef->root.u.def.value;
-      return TRUE;
-    }
-
   /* This is a reference to a symbol defined by a dynamic object which
      is not a function.  */
 

Re: Fix assertion failure on aliases of dynamic weak symbols

[Ian Lance Taylor]

"H.J. Lu" <hjl@lucon.org> writes:

> That piece of code was added by Richard on May 3, 1999 with
> comments:

Richard added the entire binutils world on May 3, 1999.  I actually
wrote that code originally, in the Cygnus repository.

>   /* 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
>      dynamic objects, that's the only time we actually put anything on
>      the list WEAKS.  We need this information so that if a regular
>      object refers to a symbol defined weakly in a dynamic object, the
>      real symbol in the dynamic object is also put in the dynamic
>      symbols; we also must arrange for both symbols to point to the
>      same memory location.  We could handle the general case of symbol
>      aliasing, but a general symbol alias can only be generated in
>      assembler code, handling it correctly would be very time
>      consuming, and other ELF linkers don't handle general aliasing
>      either.  */
> 
> It no longer applies today since there is no difference between
> weak def and strong def in a shared library. Richard, do you
> remember if there is a testcase for your change? Can we remove
> the whole u.weakdef thing now? It will make linker simpler and
> faster.

The fact that there is no difference between weak defs and strong defs
is irrelevant here.  What is happening here is that the library
defines "environ" as a weak symbol.  "environ" is a weak alias for the
real symbol named "__environ".  The program refers to "environ".
Nothing outside the shared library refers to "__environ".  The shared
library refers only to "__environ", never to "environ" (other than
defining it as a weak alias).  If we only put "environ" in the dynamic
symbol table, then the dynamic linker will update the GOT entry for
"environ" so that all references to "environ" refer to the copy in the
main program.  If we don't put "__environ" in the dynamic symbol
table, then the GOT entry for "__environ" will continue to refer to
the copy in the dynamic object's data segment.  The effect is that the
dynamic object will set "__environ" at run time, and the program will
happily look at the uninitialized variable "environ".

Ian

Re: Fix assertion failure on aliases of dynamic weak symbols

[H.J. Lu]

On Mon, Jul 16, 2007 at 09:32:54PM -0700, Ian Lance Taylor wrote:
> "H.J. Lu" <hjl@lucon.org> writes:
> 
> > That piece of code was added by Richard on May 3, 1999 with
> > comments:
> 
> Richard added the entire binutils world on May 3, 1999.  I actually
> wrote that code originally, in the Cygnus repository.
> 
> >   /* 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
> >      dynamic objects, that's the only time we actually put anything on
> >      the list WEAKS.  We need this information so that if a regular
> >      object refers to a symbol defined weakly in a dynamic object, the
> >      real symbol in the dynamic object is also put in the dynamic
> >      symbols; we also must arrange for both symbols to point to the
> >      same memory location.  We could handle the general case of symbol
> >      aliasing, but a general symbol alias can only be generated in
> >      assembler code, handling it correctly would be very time
> >      consuming, and other ELF linkers don't handle general aliasing
> >      either.  */
> > 
> > It no longer applies today since there is no difference between
> > weak def and strong def in a shared library. Richard, do you
> > remember if there is a testcase for your change? Can we remove
> > the whole u.weakdef thing now? It will make linker simpler and
> > faster.
> 
> The fact that there is no difference between weak defs and strong defs
> is irrelevant here.  What is happening here is that the library
> defines "environ" as a weak symbol.  "environ" is a weak alias for the
> real symbol named "__environ".  The program refers to "environ".
> Nothing outside the shared library refers to "__environ".  The shared
> library refers only to "__environ", never to "environ" (other than
> defining it as a weak alias).  If we only put "environ" in the dynamic
> symbol table, then the dynamic linker will update the GOT entry for
> "environ" so that all references to "environ" refer to the copy in the
> main program.  If we don't put "__environ" in the dynamic symbol
> table, then the GOT entry for "__environ" will continue to refer to
> the copy in the dynamic object's data segment.  The effect is that the
> dynamic object will set "__environ" at run time, and the program will
> happily look at the uninitialized variable "environ".

It sounds familar. I looked at the binutils archive. Unfortunately,
it starts around May 10, 1999 :-(. I will add a testcase for this.

I think there is a small problem. We are making a typed weak def an
alias of a linker-created strong def without type.  I don't think we
want to do it.  Should we require symbols with aliass should be typed?


H.J.

Re: Fix assertion failure on aliases of dynamic weak symbols

[Ian Lance Taylor]

"H.J. Lu" <hjl@lucon.org> writes:

> It sounds familar. I looked at the binutils archive. Unfortunately,
> it starts around May 10, 1999 :-(. I will add a testcase for this.

By the way, a couple of months ago I asked Nick to ask Red Hat whether
it would be possible for them to release the internal archive from
before the inclusion into sourceware.  I could hook the old changes
into the existing CVS repository as part of a conversion to
subversion, just as I did for gcc (merging the FSF revision history
into the egcs history).  So far I think the request has vanished into
Red Hat's legal department.

Ian

Re: Fix assertion failure on aliases of dynamic weak symbols

[H.J. Lu]

On Tue, Jul 17, 2007 at 06:55:59AM -0700, Ian Lance Taylor wrote:
> "H.J. Lu" <hjl@lucon.org> writes:
> 
> > It sounds familar. I looked at the binutils archive. Unfortunately,
> > it starts around May 10, 1999 :-(. I will add a testcase for this.
> 
> By the way, a couple of months ago I asked Nick to ask Red Hat whether
> it would be possible for them to release the internal archive from
> before the inclusion into sourceware.  I could hook the old changes
> into the existing CVS repository as part of a conversion to
> subversion, just as I did for gcc (merging the FSF revision history
> into the egcs history).  So far I think the request has vanished into
> Red Hat's legal department.

That is too bad.

I think the proper way to fix this is

1. Select a different section for linker created symbol to avoid false
symbol alias in a shared library.
2. Match symbol types when creating symbol aliases in a shared library.

H.J.

Re: Fix assertion failure on aliases of dynamic weak symbols

[Richard Sandiford]

"H.J. Lu" <hjl@lucon.org> writes:
> I think the proper way to fix this is
>
> 1. Select a different section for linker created symbol to avoid false
> symbol alias in a shared library.

I disagree.  If the linker script defines a symbol in a section statement,
why shouldn't the symbol belong to that section?  I'm bet folk out there
are relying on that behaviour.

Richard

Re: Fix assertion failure on aliases of dynamic weak symbols

[H.J. Lu]

On Wed, Jul 18, 2007 at 04:58:42PM +0100, Richard Sandiford wrote:
> "H.J. Lu" <hjl@lucon.org> writes:
> > I think the proper way to fix this is
> >
> > 1. Select a different section for linker created symbol to avoid false
> > symbol alias in a shared library.
> 
> I disagree.  If the linker script defines a symbol in a section statement,
> why shouldn't the symbol belong to that section?  I'm bet folk out there
> are relying on that behaviour.
> 

I had an impression that linker tried to pick a section for linker
defined symbols. I was wrong.


H.J.

PATCH: Check symbol type for symbol alias

[H.J. Lu]

Here is a patch to add a testcase and check symbol type for symbol
alias.


H.J.
----
bfd/

2007-07-19  H.J. Lu  <hongjiu.lu@intel.com>

	* elflink.c (elf_link_add_object_symbols): Check symbol type
	for symbol alias in a dynamic object.

ld/testsuite/

2007-07-19  H.J. Lu  <hongjiu.lu@intel.com>

	* ld-elf/data2.c: New.
	* ld-elf/weakdef1.c: Likewise.

	* ld-elf/shared.exp: Add tests for libdata2 and weakdef1.

--- binutils/bfd/elflink.c.weakdef	2007-07-19 08:12:55.000000000 -0700
+++ binutils/bfd/elflink.c	2007-07-19 10:24:57.000000000 -0700
@@ -4585,6 +4585,7 @@ elf_link_add_object_symbols (bfd *abfd, 
 	  asection *slook;
 	  bfd_vma vlook;
 	  long ilook;
+	  int tlook;
 	  size_t i, j, idx;
 
 	  hlook = weaks;
@@ -4597,6 +4598,7 @@ elf_link_add_object_symbols (bfd *abfd, 
 		      || hlook->root.type == bfd_link_hash_indirect);
 	  slook = hlook->root.u.def.section;
 	  vlook = hlook->root.u.def.value;
+	  tlook = hlook->type;
 
 	  ilook = -1;
 	  i = 0;
@@ -4634,9 +4636,10 @@ elf_link_add_object_symbols (bfd *abfd, 
 	    {
 	      h = sorted_sym_hash [i];
 
-	      /* Stop if value or section doesn't match.  */
+	      /* Stop if value, section or type doesn't match.  */
 	      if (h->root.u.def.value != vlook
-		  || h->root.u.def.section != slook)
+		  || h->root.u.def.section != slook
+		  || h->type != tlook)
 		break;
 	      else if (h != hlook)
 		{
--- binutils/ld/testsuite/ld-elf/data2.c.weakdef	2007-07-19 10:22:33.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/data2.c	2007-07-19 10:16:33.000000000 -0700
@@ -0,0 +1,9 @@
+int foo = 0;
+extern int foo_alias __attribute__ ((weak, alias ("foo")));
+
+void
+bar (void)
+{
+  foo = -1;
+}
+
--- binutils/ld/testsuite/ld-elf/shared.exp.weakdef	2007-07-06 13:12:26.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/shared.exp	2007-07-19 10:20:15.000000000 -0700
@@ -123,6 +123,9 @@ set build_tests {
   {"Build libdata1.so"
    "-shared" "-fPIC"
    {data1.c} {} "libdata1.so"}
+  {"Build libdata2.so"
+   "-shared" "-fPIC"
+   {data2.c} {} "libdata2.so"}
 }
 
 set run_tests {
@@ -235,6 +238,9 @@ set run_tests {
     {"Run with libdata1.so"
      "tmpdir/libdata1.so" ""
      {dynbss1.c} "dynbss1" "pass.out"}
+    {"Run with libdata2.so"
+     "tmpdir/libdata2.so" ""
+     {weakdef1.c} "weakdef1" "pass.out"}
 }
 
 run_cc_link_tests $build_tests
--- binutils/ld/testsuite/ld-elf/weakdef1.c.weakdef	2007-07-19 10:21:52.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/weakdef1.c	2007-07-19 10:21:28.000000000 -0700
@@ -0,0 +1,15 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+extern int foo_alias;
+extern void bar (void);
+
+int
+main (void)
+{
+  bar ();
+  if (foo_alias != -1)
+    abort ();
+  printf ("PASS\n");
+  return 0;
+}

Re: PATCH: Check symbol type for symbol alias

[Richard Sandiford]

"H.J. Lu" <hjl@lucon.org> writes:
> Here is a patch to add a testcase and check symbol type for symbol
> alias.

In case anyone's in any doubt, this really doesn't fix my original
problem.  Although the strong symbol in the test's shared library was
defined by the linker script, that was mostly for test convenience.
We can still have a situation in which an object symbol "bar" in a
shared library is overridden by an assignment "PROVIDE (bar = .);" in
the linker script of an object being linked against the shared library.
(And as I said in the covering note, we do handle that situation
correctly; we use relocations against the original weak symbol instead.)
I can adjust the testcase in the obvious way if this patch goes in.

Richard

Re: PATCH: Check symbol type for symbol alias

[H.J. Lu]

On Fri, Jul 20, 2007 at 10:12:42AM +0100, Richard Sandiford wrote:
> "H.J. Lu" <hjl@lucon.org> writes:
> > Here is a patch to add a testcase and check symbol type for symbol
> > alias.
> 
> In case anyone's in any doubt, this really doesn't fix my original
> problem.  Although the strong symbol in the test's shared library was
> defined by the linker script, that was mostly for test convenience.
> We can still have a situation in which an object symbol "bar" in a
> shared library is overridden by an assignment "PROVIDE (bar = .);" in
> the linker script of an object being linked against the shared library.
> (And as I said in the covering note, we do handle that situation
> correctly; we use relocations against the original weak symbol instead.)
> I can adjust the testcase in the obvious way if this patch goes in.

The problem is linker creates a wrong symbol alias for a dynamic
object. We don't have a reliable way to tell if a symbol is an aliase
in a shared library. It can lead to many problems. Can we use
STT_SECTION for linker created symbols? Gabi says

STT_SECTION
    The symbol is associated with a section. Symbol table entries of
this type exist primarily for relocation and normally have STB_LOCAL
binding.

It doesn't forbid STB_GLOBAL binding. At least, we won't make a weak
symbol an alias of linker created symbols.


H.J.

Re: PATCH: Check symbol type for symbol alias

[Richard Sandiford]

"H.J. Lu" <hjl@lucon.org> writes:
> On Fri, Jul 20, 2007 at 10:12:42AM +0100, Richard Sandiford wrote:
>> "H.J. Lu" <hjl@lucon.org> writes:
>> > Here is a patch to add a testcase and check symbol type for symbol
>> > alias.
>> 
>> In case anyone's in any doubt, this really doesn't fix my original
>> problem.  Although the strong symbol in the test's shared library was
>> defined by the linker script, that was mostly for test convenience.
>> We can still have a situation in which an object symbol "bar" in a
>> shared library is overridden by an assignment "PROVIDE (bar = .);" in
>> the linker script of an object being linked against the shared library.
>> (And as I said in the covering note, we do handle that situation
>> correctly; we use relocations against the original weak symbol instead.)
>> I can adjust the testcase in the obvious way if this patch goes in.
>
> The problem is linker creates a wrong symbol alias for a dynamic
> object. We don't have a reliable way to tell if a symbol is an aliase
> in a shared library. It can lead to many problems. Can we use
> STT_SECTION for linker created symbols? Gabi says
>
> STT_SECTION
>     The symbol is associated with a section. Symbol table entries of
> this type exist primarily for relocation and normally have STB_LOCAL
> binding.
>
> It doesn't forbid STB_GLOBAL binding. At least, we won't make a weak
> symbol an alias of linker created symbols.

I think you're missing my point.  Please read what I said again.  You seem
to be talking about cases where the alias should not be considered valid
in isolation (i.e. by looking at the shared library and nothing else).
The bug I'm fixing happens _regardless of whether the alias would
normally be considered valid in isolation_.  Hence my comment about
changing my testcase.  Although the original testcase did use a linker
script to establish the alias, the test would still fail if that alias
were established in the normal environ/__environ way.

In my example -- where the shared library is involved in a second link
that redefines the strong symbol using a PROVIDE statement -- we _already_
correctly determine that the alias is _not_ valid, because the PROVIDEd
symbol overrides the shared library's symbol.  The problem is simply
that we have a bogus assert along the way.  (Recall that this is a
non-fatal assert.)  The code did not realise that a strongly-defined
PROVIDE symbol would actually have type bfd_link_hash_undefined
(rather than bfd_link_hash_defined or bfd_link_hash_defweak).

The bug I'm fixing is a corner case, and is simply an artifact of the
way PROVIDE symbols are handled.

Richard

Re: PATCH: Check symbol type for symbol alias

[H.J. Lu]

On Fri, Jul 20, 2007 at 02:06:31PM +0100, Richard Sandiford wrote:
> "H.J. Lu" <hjl@lucon.org> writes:
> > On Fri, Jul 20, 2007 at 10:12:42AM +0100, Richard Sandiford wrote:
> >> "H.J. Lu" <hjl@lucon.org> writes:
> >> > Here is a patch to add a testcase and check symbol type for symbol
> >> > alias.
> >> 
> >> In case anyone's in any doubt, this really doesn't fix my original
> >> problem.  Although the strong symbol in the test's shared library was
> >> defined by the linker script, that was mostly for test convenience.
> >> We can still have a situation in which an object symbol "bar" in a
> >> shared library is overridden by an assignment "PROVIDE (bar = .);" in
> >> the linker script of an object being linked against the shared library.
> >> (And as I said in the covering note, we do handle that situation
> >> correctly; we use relocations against the original weak symbol instead.)
> >> I can adjust the testcase in the obvious way if this patch goes in.
> >
> > The problem is linker creates a wrong symbol alias for a dynamic
> > object. We don't have a reliable way to tell if a symbol is an aliase
> > in a shared library. It can lead to many problems. Can we use
> > STT_SECTION for linker created symbols? Gabi says
> >
> > STT_SECTION
> >     The symbol is associated with a section. Symbol table entries of
> > this type exist primarily for relocation and normally have STB_LOCAL
> > binding.
> >
> > It doesn't forbid STB_GLOBAL binding. At least, we won't make a weak
> > symbol an alias of linker created symbols.
> 
> I think you're missing my point.  Please read what I said again.  You seem
> to be talking about cases where the alias should not be considered valid
> in isolation (i.e. by looking at the shared library and nothing else).
> The bug I'm fixing happens _regardless of whether the alias would
> normally be considered valid in isolation_.  Hence my comment about
> changing my testcase.  Although the original testcase did use a linker
> script to establish the alias, the test would still fail if that alias
> were established in the normal environ/__environ way.
> 
> In my example -- where the shared library is involved in a second link
> that redefines the strong symbol using a PROVIDE statement -- we _already_
> correctly determine that the alias is _not_ valid, because the PROVIDEd
> symbol overrides the shared library's symbol.  The problem is simply
> that we have a bogus assert along the way.  (Recall that this is a
> non-fatal assert.)  The code did not realise that a strongly-defined
> PROVIDE symbol would actually have type bfd_link_hash_undefined
> (rather than bfd_link_hash_defined or bfd_link_hash_defweak).
> 
> The bug I'm fixing is a corner case, and is simply an artifact of the
> way PROVIDE symbols are handled.
> 

We have 2 problems:

1. Linker may create wrong symbol alias.
2. Linker needs to handle the case where symbol alias in a shared
library is changed by linker created symbol.

For #2, I think linker should rediret symbol alias to the linker
created symbol. Does the linker do it correctly?


H.J.

Re: PATCH: Check symbol type for symbol alias

[Richard Sandiford]

"H.J. Lu" <hjl@lucon.org> writes:
> On Fri, Jul 20, 2007 at 02:06:31PM +0100, Richard Sandiford wrote:
>> "H.J. Lu" <hjl@lucon.org> writes:
>> > On Fri, Jul 20, 2007 at 10:12:42AM +0100, Richard Sandiford wrote:
>> >> "H.J. Lu" <hjl@lucon.org> writes:
>> >> > Here is a patch to add a testcase and check symbol type for symbol
>> >> > alias.
>> >> 
>> >> In case anyone's in any doubt, this really doesn't fix my original
>> >> problem.  Although the strong symbol in the test's shared library was
>> >> defined by the linker script, that was mostly for test convenience.
>> >> We can still have a situation in which an object symbol "bar" in a
>> >> shared library is overridden by an assignment "PROVIDE (bar = .);" in
>> >> the linker script of an object being linked against the shared library.
>> >> (And as I said in the covering note, we do handle that situation
>> >> correctly; we use relocations against the original weak symbol instead.)
>> >> I can adjust the testcase in the obvious way if this patch goes in.
>> >
>> > The problem is linker creates a wrong symbol alias for a dynamic
>> > object. We don't have a reliable way to tell if a symbol is an aliase
>> > in a shared library. It can lead to many problems. Can we use
>> > STT_SECTION for linker created symbols? Gabi says
>> >
>> > STT_SECTION
>> >     The symbol is associated with a section. Symbol table entries of
>> > this type exist primarily for relocation and normally have STB_LOCAL
>> > binding.
>> >
>> > It doesn't forbid STB_GLOBAL binding. At least, we won't make a weak
>> > symbol an alias of linker created symbols.
>> 
>> I think you're missing my point.  Please read what I said again.  You seem
>> to be talking about cases where the alias should not be considered valid
>> in isolation (i.e. by looking at the shared library and nothing else).
>> The bug I'm fixing happens _regardless of whether the alias would
>> normally be considered valid in isolation_.  Hence my comment about
>> changing my testcase.  Although the original testcase did use a linker
>> script to establish the alias, the test would still fail if that alias
>> were established in the normal environ/__environ way.
>> 
>> In my example -- where the shared library is involved in a second link
>> that redefines the strong symbol using a PROVIDE statement -- we _already_
>> correctly determine that the alias is _not_ valid, because the PROVIDEd
>> symbol overrides the shared library's symbol.  The problem is simply
>> that we have a bogus assert along the way.  (Recall that this is a
>> non-fatal assert.)  The code did not realise that a strongly-defined
>> PROVIDE symbol would actually have type bfd_link_hash_undefined
>> (rather than bfd_link_hash_defined or bfd_link_hash_defweak).
>> 
>> The bug I'm fixing is a corner case, and is simply an artifact of the
>> way PROVIDE symbols are handled.
>> 
>
> We have 2 problems:
>
> 1. Linker may create wrong symbol alias.
> 2. Linker needs to handle the case where symbol alias in a shared
> library is changed by linker created symbol.
>
> For #2, I think linker should rediret symbol alias to the linker
> created symbol. Does the linker do it correctly?

No, that's not the intended behaviour.  If a regular object (or linker
script) overrides the strong symbol, relocations against the weak symbol
are not redirected.  Quoting from elflink.c:

  /* If this is a weak definition, and we know a real definition, and
     the real symbol is not itself defined by a regular object file,
     then get a good value for the real definition.  We handle the
     real symbol first, for the convenience of the backend routine.

     Note that there is a confusing case here.  If the real definition
     is defined by a regular object file, we don't get the real symbol
     from the dynamic object, but we do get the weak symbol.  If the
     processor backend uses a COPY reloc, then if some routine in the
     dynamic object changes the real symbol, we will not see that
     change in the corresponding weak symbol.  This is the way other
     ELF linkers work as well, and seems to be a result of the shared
     library model.

     I will clarify this issue.  Most SVR4 shared libraries define the
     variable _timezone and define timezone as a weak synonym.  The
     tzset call changes _timezone.  If you write
       extern int timezone;
       int _timezone = 5;
       int main () { tzset (); printf ("%d %d\n", timezone, _timezone); }
     you might expect that, since timezone is a synonym for _timezone,
     the same number will print both times.  However, if the processor
     backend uses a COPY reloc, then actually timezone will be copied
     into your process image, and, since you define _timezone
     yourself, _timezone will not.  Thus timezone and _timezone will
     wind up at different memory locations.  The tzset call will set
     _timezone, leaving timezone unchanged.  */

This is the case I'm talking about, except that in my example,
"_timezone" is defined by a PROVIDE assignment.

Richard

Re: PATCH: Check symbol type for symbol alias

[H.J. Lu]

On Fri, Jul 20, 2007 at 09:17:00PM +0100, Richard Sandiford wrote:
> > We have 2 problems:
> >
> > 1. Linker may create wrong symbol alias.
> > 2. Linker needs to handle the case where symbol alias in a shared
> > library is changed by linker created symbol.
> >
> > For #2, I think linker should rediret symbol alias to the linker
> > created symbol. Does the linker do it correctly?
> 
> No, that's not the intended behaviour.  If a regular object (or linker
> script) overrides the strong symbol, relocations against the weak symbol
> are not redirected.  Quoting from elflink.c:
> 
>   /* If this is a weak definition, and we know a real definition, and
>      the real symbol is not itself defined by a regular object file,
>      then get a good value for the real definition.  We handle the
>      real symbol first, for the convenience of the backend routine.
> 
>      Note that there is a confusing case here.  If the real definition
>      is defined by a regular object file, we don't get the real symbol
>      from the dynamic object, but we do get the weak symbol.  If the
>      processor backend uses a COPY reloc, then if some routine in the
>      dynamic object changes the real symbol, we will not see that
>      change in the corresponding weak symbol.  This is the way other
>      ELF linkers work as well, and seems to be a result of the shared
>      library model.
> 
>      I will clarify this issue.  Most SVR4 shared libraries define the
>      variable _timezone and define timezone as a weak synonym.  The
>      tzset call changes _timezone.  If you write
>        extern int timezone;
>        int _timezone = 5;
>        int main () { tzset (); printf ("%d %d\n", timezone, _timezone); }
>      you might expect that, since timezone is a synonym for _timezone,
>      the same number will print both times.  However, if the processor
>      backend uses a COPY reloc, then actually timezone will be copied
>      into your process image, and, since you define _timezone
>      yourself, _timezone will not.  Thus timezone and _timezone will
>      wind up at different memory locations.  The tzset call will set
>      _timezone, leaving timezone unchanged.  */
> 
> This is the case I'm talking about, except that in my example,
> "_timezone" is defined by a PROVIDE assignment.
> 

You are right. Can you update your testcase to reflect it?

Thanks.


H.J.

Re: Fix assertion failure on aliases of dynamic weak symbols

[Alan Modra]

On Mon, Jul 16, 2007 at 06:10:10PM +0100, Richard Sandiford wrote:
> bfd/
> 	* elflink.c (_bfd_elf_fix_symbol_flags): Only assert the type
> 	of weakdef->root.type if weakdef has no regular definition.
> 
> ld/testsuite/
> 	* ld-elf/weak-dyn-1a.s, ld-elf/weak-dyn-1b.s, ld-elf/weak-dyn-1.ld,
> 	* ld-elf/weak-dyn-1.rd: New test.
> 	* ld-elf/elf.exp: Run it.

This is OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Fix assertion failure on aliases of dynamic weak symbols

[Richard Sandiford]

Alan Modra <amodra@bigpond.net.au> writes:
> On Mon, Jul 16, 2007 at 06:10:10PM +0100, Richard Sandiford wrote:
>> bfd/
>> 	* elflink.c (_bfd_elf_fix_symbol_flags): Only assert the type
>> 	of weakdef->root.type if weakdef has no regular definition.
>> 
>> ld/testsuite/
>> 	* ld-elf/weak-dyn-1a.s, ld-elf/weak-dyn-1b.s, ld-elf/weak-dyn-1.ld,
>> 	* ld-elf/weak-dyn-1.rd: New test.
>> 	* ld-elf/elf.exp: Run it.
>
> This is OK.

Thanks.  As HJ asked, I changed the test so that the shared library
uses a normal object definition of the strong symbol.  Here's what
I installed, after checking that the new test failed before the
patch and passes after it.

Richard


bfd/
	* elflink.c (_bfd_elf_fix_symbol_flags): Only assert the type
	of weakdef->root.type if weakdef has no regular definition.

ld/testsuite/
	* ld-elf/weak-dyn-1a.s, ld-elf/weak-dyn-1b.s, ld-elf/weak-dyn-1.ld,
	* ld-elf/weak-dyn-1.rd: New test.
	* ld-elf/elf.exp: Run it.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.269
diff -u -p -r1.269 elflink.c
--- bfd/elflink.c	10 Jul 2007 02:40:31 -0000	1.269
+++ bfd/elflink.c	23 Jul 2007 09:55:13 -0000
@@ -2492,8 +2492,6 @@ _bfd_elf_fix_symbol_flags (struct elf_li
 
       BFD_ASSERT (h->root.type == bfd_link_hash_defined
 		  || h->root.type == bfd_link_hash_defweak);
-      BFD_ASSERT (weakdef->root.type == bfd_link_hash_defined
-		  || weakdef->root.type == bfd_link_hash_defweak);
       BFD_ASSERT (weakdef->def_dynamic);
 
       /* If the real definition is defined by a regular object file,
@@ -2502,8 +2500,11 @@ _bfd_elf_fix_symbol_flags (struct elf_li
       if (weakdef->def_regular)
 	h->u.weakdef = NULL;
       else
-	(*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef,
-						  h);
+	{
+	  BFD_ASSERT (weakdef->root.type == bfd_link_hash_defined
+		      || weakdef->root.type == bfd_link_hash_defweak);
+	  (*bed->elf_backend_copy_indirect_symbol) (eif->info, weakdef, h);
+	}
     }
 
   return TRUE;
Index: ld/testsuite/ld-elf/weak-dyn-1a.s
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1a.s
diff -N ld/testsuite/ld-elf/weak-dyn-1a.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1a.s	23 Jul 2007 09:55:13 -0000
@@ -0,0 +1,13 @@
+	.globl	foo
+	.weak	foo
+	.type	foo,%object
+	.size	foo,1
+
+	.globl	bar
+	.type	bar,%object
+	.size	bar,1
+
+	.data
+foo:
+bar:
+	.byte	1
Index: ld/testsuite/ld-elf/weak-dyn-1b.s
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1b.s
diff -N ld/testsuite/ld-elf/weak-dyn-1b.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1b.s	23 Jul 2007 09:55:13 -0000
@@ -0,0 +1,2 @@
+	.data
+	.dc.a	foo
Index: ld/testsuite/ld-elf/weak-dyn-1.ld
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1.ld
diff -N ld/testsuite/ld-elf/weak-dyn-1.ld
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1.ld	23 Jul 2007 09:55:13 -0000
@@ -0,0 +1,8 @@
+SECTIONS
+{
+  . = 0x800000;
+  PROVIDE (bar = .);
+  .data : {
+    *(.data)
+  }
+}
Index: ld/testsuite/ld-elf/weak-dyn-1.rd
===================================================================
RCS file: ld/testsuite/ld-elf/weak-dyn-1.rd
diff -N ld/testsuite/ld-elf/weak-dyn-1.rd
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ld/testsuite/ld-elf/weak-dyn-1.rd	23 Jul 2007 09:55:13 -0000
@@ -0,0 +1,3 @@
+#...
+0+800000 .* foo.*
+#pass
Index: ld/testsuite/ld-elf/elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/elf.exp,v
retrieving revision 1.10
diff -u -p -r1.10 elf.exp
--- ld/testsuite/ld-elf/elf.exp	6 Jul 2007 14:09:43 -0000	1.10
+++ ld/testsuite/ld-elf/elf.exp	23 Jul 2007 09:55:13 -0000
@@ -36,6 +36,19 @@ foreach t $test_list {
     run_dump_test [file rootname $t]
 }
 
+if { [istarget *-*-linux*] } {
+    run_ld_link_tests {
+	{"Weak symbols in dynamic objects 1 (support)"
+	    "-shared" "" {weak-dyn-1a.s}
+	    {}
+	    "libweakdyn1a.so"}
+	{"Weak symbols in dynamic objects 1 (main test)"
+	    "-shared tmpdir/libweakdyn1a.so -Tweak-dyn-1.ld" "" {weak-dyn-1b.s}
+	    {{readelf {--relocs --wide} weak-dyn-1.rd}}
+	    "libweakdyn1b.so"}
+    }
+}
+
 # The following tests require running the executable generated by ld.
 if ![isnative] {
     return