This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: ld/3351: aborting at elflink.c line 6778 in elf_link_check_versioned_symbol


On Thu, Oct 19, 2006 at 10:34:53AM +0930, Alan Modra wrote:
> On Tue, Oct 17, 2006 at 05:57:12PM -0700, H. J. Lu wrote:
> > On Tue, Oct 17, 2006 at 04:31:05PM -0700, H. J. Lu wrote:
> > > When a shared library references a forced local symbol in executable,
> > > linker tries to issue an error. But it doesn't check indirect symbols.
> > > Also, when we update real symbol reference flags, we fail to update
> > > indirect symbol reference flags. This patch fixes both.
> > > 
> > 
> > We have to be very careful about indiret symbol created by symbol
> > versioning. This patch only updates some indirect symbol flags Should
> > _bfd_elf_merge_symbol and elf_link_add_object_symbols update all
> > indirect symbol flags at the same time when we update real symbol
> > flags?
> 
> The usual practice in the linker is to copy flags from the indirect
> symbol to the real one at the time the indirect symbol is created.
> >From that point on test/modify only the real symbol flags.  I don't
> think you should copy anything back to the indirect symbol unless
> there is a very good reason to do so.
> 

That is true in general. However, the indirect symbol dynamic flags 
are used to detect errors. If we don't set them properly,
elf_link_output_extsym won't handle indirect symbols correctly.

Another problem is when the indirect symbol is forced local, we
can't make the real symbol dynamic just because the indirect symbol is
referenced by a shared library.

Here is an updated patch with a few testcases. I only set dynamic flags
for indirect symbols when necessary.


H.J.
-----
bfd/

2006-10-17  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/3351
	* elflink.c (_bfd_elf_update_dynamic_flags): New.
	(_bfd_elf_merge_symbol): Update both real and indirect symbol
	dynamic flags.
	(_bfd_elf_add_default_symbol): Make the real symbol dynamic if
	the indirect symbol is defined in a shared library.
	(elf_link_add_object_symbols): Likewise.  If the indirect
	symbol has been forced local, don't make the real symbol
	dynamic.
	(elf_link_check_versioned_symbol): Check indirect symbol.
	(elf_link_output_extsym): Use real symbol definition when
	reporting indirect symbol error.  Check version info for
	dynamic versioned symbol.

ld/testsuite/

2006-10-17  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/3351
	* ld-elf/indirect.exp: New file.
	* ld-elf/indirect1a.c: Likewise.
	* ld-elf/indirect1b.c: Likewise.
	* ld-elf/indirect1c.c: Likewise.
	* ld-elf/indirect2.c: Likewise.
	* ld-elf/indirect3.out: Likewise.
	* ld-elf/indirect3a.c: Likewise.
	* ld-elf/indirect3b.c: Likewise.
	* ld-elf/indirect3c.c: Likewise.
	* ld-elf/indirect4.out: Likewise.
	* ld-elf/indirect4a.c: Likewise.
	* ld-elf/indirect4b.c: Likewise.
	* ld-elf/indirect4c.c: Likewise.

--- binutils/bfd/elflink.c.indirect	2006-10-18 09:25:03.000000000 -0700
+++ binutils/bfd/elflink.c	2006-10-18 17:50:21.000000000 -0700
@@ -801,6 +801,33 @@ _bfd_elf_link_renumber_dynsyms (bfd *out
   return dynsymcount;
 }
 
+/* Mark if a symbol has a definition in a dynamic object or is
+   weak in all dynamic objects.  */
+
+static void
+_bfd_elf_mark_dynamic_def_weak (struct elf_link_hash_entry *h,
+				asection *sec, int bind)
+{
+  if (!h->dynamic_def)
+    {
+      if (!bfd_is_und_section (sec))
+	h->dynamic_def = 1;
+      else
+	{
+	  /* Check if this symbol is weak in all dynamic objects. If it
+	     is the first time we see it in a dynamic object, we mark
+	     if it is weak. Otherwise, we clear it.  */
+	  if (!h->ref_dynamic)
+	    {
+	      if (bind == STB_WEAK)
+		h->dynamic_weak = 1;
+	    }
+	  else if (bind != STB_WEAK)
+	    h->dynamic_weak = 0;
+	}
+    }
+}
+
 /* This function is called when we want to define a new symbol.  It
    handles the various cases which arise when we find a definition in
    a dynamic object, or when there is already a definition in a
@@ -829,6 +856,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
 {
   asection *sec, *oldsec;
   struct elf_link_hash_entry *h;
+  struct elf_link_hash_entry *hi;
   struct elf_link_hash_entry *flip;
   int bind;
   bfd *oldbfd;
@@ -865,8 +893,9 @@ _bfd_elf_merge_symbol (bfd *abfd,
   if (info->hash->creator != abfd->xvec)
     return TRUE;
 
-  /* For merging, we only care about real symbols.  */
-
+  /* For merging, we only care about real symbols.  But we need to make
+     sure that indirect symbol dynamic flags are updated.  */
+  hi = h;
   while (h->root.type == bfd_link_hash_indirect
 	 || h->root.type == bfd_link_hash_warning)
     h = (struct elf_link_hash_entry *) h->root.u.i.link;
@@ -1018,23 +1047,11 @@ _bfd_elf_merge_symbol (bfd *abfd,
   /* We need to remember if a symbol has a definition in a dynamic
      object or is weak in all dynamic objects. Internal and hidden
      visibility will make it unavailable to dynamic objects.  */
-  if (newdyn && !h->dynamic_def)
+  if (newdyn)
     {
-      if (!bfd_is_und_section (sec))
-	h->dynamic_def = 1;
-      else
-	{
-	  /* Check if this symbol is weak in all dynamic objects. If it
-	     is the first time we see it in a dynamic object, we mark
-	     if it is weak. Otherwise, we clear it.  */
-	  if (!h->ref_dynamic)
-	    {
-	      if (bind == STB_WEAK)
-		h->dynamic_weak = 1;
-	    }
-	  else if (bind != STB_WEAK)
-	    h->dynamic_weak = 0;
-	}
+      _bfd_elf_mark_dynamic_def_weak (h, sec, bind);
+      if (h != hi)
+	_bfd_elf_mark_dynamic_def_weak (hi, sec, bind);
     }
 
   /* If the old symbol has non-default visibility, we ignore the new
@@ -1046,6 +1063,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
       *skip = TRUE;
       /* Make sure this symbol is dynamic.  */
       h->ref_dynamic = 1;
+      hi->ref_dynamic = 1;
       /* A protected symbol has external availability. Make sure it is
 	 recorded as dynamic.
 
@@ -1572,6 +1590,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
 	  if (! dynamic)
 	    {
 	      if (info->shared
+		  || hi->def_dynamic
 		  || hi->ref_dynamic)
 		*dynsym = TRUE;
 	    }
@@ -3644,6 +3663,7 @@ elf_link_add_object_symbols (bfd *abfd, 
       flagword flags;
       const char *name;
       struct elf_link_hash_entry *h;
+      struct elf_link_hash_entry *hi;
       bfd_boolean definition;
       bfd_boolean size_change_ok;
       bfd_boolean type_change_ok;
@@ -3934,6 +3954,9 @@ elf_link_add_object_symbols (bfd *abfd, 
 	goto error_free_vers;
 
       h = *sym_hash;
+      /* We need to make sure that indirect symbol dynamic flags are
+	 updated.  */
+      hi = h;
       while (h->root.type == bfd_link_hash_indirect
 	     || h->root.type == bfd_link_hash_warning)
 	h = (struct elf_link_hash_entry *) h->root.u.i.link;
@@ -4137,22 +4160,36 @@ elf_link_add_object_symbols (bfd *abfd, 
 		}
 	      else
 		h->def_regular = 1;
-	      if (! info->executable
-		  || h->def_dynamic
-		  || h->ref_dynamic)
+
+	      /* If the indirect symbol has been forced local, don't
+		 make the real symbol dynamic.  */
+	      if ((h == hi || !hi->forced_local)
+		  && (! info->executable
+		      || h->def_dynamic
+		      || h->ref_dynamic))
 		dynsym = TRUE;
 	    }
 	  else
 	    {
 	      if (! definition)
-		h->ref_dynamic = 1;
+		{
+		  h->ref_dynamic = 1;
+		  hi->ref_dynamic = 1;
+		}
 	      else
-		h->def_dynamic = 1;
-	      if (h->def_regular
-		  || h->ref_regular
-		  || (h->u.weakdef != NULL
-		      && ! new_weakdef
-		      && h->u.weakdef->dynindx != -1))
+		{
+		  h->def_dynamic = 1;
+		  hi->def_dynamic = 1;
+		}
+
+	      /* If the indirect symbol has been forced local, don't
+		 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)))
 		dynsym = TRUE;
 	    }
 
@@ -6729,6 +6766,10 @@ elf_link_check_versioned_symbol (struct 
   if (!is_elf_hash_table (info->hash))
     return FALSE;
 
+  /* Check indirect symbol.  */
+  while (h->root.type == bfd_link_hash_indirect)
+    h = (struct elf_link_hash_entry *) h->root.u.i.link;
+
   switch (h->root.type)
     {
     default:
@@ -6937,11 +6978,17 @@ elf_link_output_extsym (struct elf_link_
       && !h->dynamic_weak
       && ! elf_link_check_versioned_symbol (finfo->info, bed, h))
     {
+      struct elf_link_hash_entry *hi = h;
+
+      /* Check indirect symbol.  */
+      while (hi->root.type == bfd_link_hash_indirect)
+	hi = (struct elf_link_hash_entry *) hi->root.u.i.link;
+
       (*_bfd_error_handler)
 	(_("%B: %s symbol `%s' in %B is referenced by DSO"),
 	 finfo->output_bfd,
-	 h->root.u.def.section == bfd_abs_section_ptr
-	 ? finfo->output_bfd : h->root.u.def.section->owner,
+	 hi->root.u.def.section == bfd_abs_section_ptr
+	 ? finfo->output_bfd : hi->root.u.def.section->owner,
 	 ELF_ST_VISIBILITY (h->other) == STV_INTERNAL
 	 ? "internal"
 	 : ELF_ST_VISIBILITY (h->other) == STV_HIDDEN
@@ -7137,6 +7184,23 @@ elf_link_output_extsym (struct elf_link_
     {
       bfd_byte *esym;
 
+      /* Since there is no version information in the dynamic string,
+	 if there is no version info in symbol version section, we will
+	 have a run-time problem.  */
+      if (h->verinfo.verdef == NULL)
+	{
+	  char *p = strrchr (h->root.root.string, ELF_VER_CHR);
+
+	  if (p && p [1] != '\0')
+	    {
+	      (*_bfd_error_handler)
+		(_("%B: No symbol version section for versioned symbol `%s'"),
+		 finfo->output_bfd, h->root.root.string);
+	      eoinfo->failed = TRUE;
+	      return FALSE;
+	    }
+	}
+
       sym.st_name = h->dynstr_index;
       esym = finfo->dynsym_sec->contents + h->dynindx * bed->s->sizeof_sym;
       if (! check_dynsym (finfo->output_bfd, &sym))
--- binutils/ld/testsuite/ld-elf/indirect.exp.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect.exp	2006-10-18 16:13:52.000000000 -0700
@@ -0,0 +1,126 @@
+# Expect script for various indirect symbol tests.
+#   Copyright 2006 Free Software Foundation, Inc.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA.
+#
+
+#
+# Written by H.J. Lu (hongjiu.lu@intel.com)
+#
+
+# Exclude non-ELF targets.
+
+if ![is_elf_format] {
+    return
+}
+
+# Check if compiler works
+if { [which $CC] == 0 } {
+    return
+}
+
+proc check_link_message { cmd string testname } {
+    send_log "$cmd\n"
+    verbose "$cmd"
+    catch "exec $cmd" exec_output
+    send_log "$exec_output\n"
+    verbose "$exec_output"
+
+    foreach str $string {
+	if [string match "*$str*" $exec_output] {
+	    pass "$testname: $str"
+	} else {
+	    fail "$testname: $str"
+	}
+    }
+}
+
+if { ![ld_compile $CC $srcdir/$subdir/indirect1a.c tmpdir/indirect1a.o]
+     || ![ld_compile $CC $srcdir/$subdir/indirect1b.c tmpdir/indirect1b.o]
+     || ![ld_compile "$CC -fPIC" $srcdir/$subdir/indirect2.c tmpdir/indirect2.o]
+     || ![ld_compile $CC $srcdir/$subdir/indirect3a.c tmpdir/indirect3a.o]
+     || ![ld_compile $CC $srcdir/$subdir/indirect3b.c tmpdir/indirect3b.o]
+     || ![ld_compile $CC $srcdir/$subdir/indirect4a.c tmpdir/indirect4a.o]
+     || ![ld_compile $CC $srcdir/$subdir/indirect4b.c tmpdir/indirect4b.o] } {
+    unresolved "Indirect symbol tests"
+    return
+}
+
+set build_tests {
+  {"Build libindirect1c.so"
+   "-shared" "-fPIC"
+   {indirect1c.c} {} "libindirect1c.so"}
+  {"Build libindirect3c.so"
+   "-shared" "-fPIC"
+   {indirect3c.c} {} "libindirect3c.so"}
+  {"Build libindirect4c.so"
+   "-shared" "-fPIC"
+   {indirect4c.c} {} "libindirect4c.so"}
+}
+
+run_cc_link_tests $build_tests
+
+global ld
+
+set string ": final link failed: Nonrepresentable section on output"
+
+set string1 ": local symbol \`foo\' in tmpdir/indirect1b.o is referenced by DSO"
+
+set testname "Indirect symbol 1a"
+set cmd "$ld -e start -o tmpdir/indirect1 tmpdir/indirect1a.o tmpdir/indirect1b.o tmpdir/libindirect1c.so"
+check_link_message "$cmd" [list $string1 $string] "$testname"
+
+set testname "Indirect symbol 1b"
+set cmd "$ld -e start -o tmpdir/indirect1 tmpdir/indirect1a.o tmpdir/libindirect1c.so tmpdir/indirect1b.o"
+check_link_message "$cmd" [list $string1 $string] "$testname"
+
+set string2 ": No symbol version section for versioned symbol \`foo@FOO\'"
+set testname "Indirect symbol 2"
+set cmd "$ld -shared  -o tmpdir/indirect2.so tmpdir/indirect2.o"
+check_link_message "$cmd" [list $string2 $string] "$testname"
+
+# The following tests require running the executable generated by ld.
+if ![isnative] {
+    return
+}
+
+set run_tests {
+    {"Run with libindirect3c.so 1"
+     "tmpdir/indirect3a.o tmpdir/indirect3b.o tmpdir/libindirect3c.so" ""
+     {dummy.c} "indirect3a" "indirect3.out"}
+    {"Run with libindirect3c.so 2"
+     "tmpdir/indirect3a.o tmpdir/libindirect3c.so tmpdir/indirect3b.o" ""
+     {dummy.c} "indirect3b" "indirect3.out"}
+    {"Run with libindirect3c.so 3"
+     "tmpdir/indirect3b.o tmpdir/libindirect3c.so tmpdir/indirect3a.o" ""
+     {dummy.c} "indirect3c" "indirect3.out"}
+    {"Run with libindirect3c.so 4"
+     "tmpdir/libindirect3c.so tmpdir/indirect3b.o tmpdir/indirect3a.o" ""
+     {dummy.c} "indirect3d" "indirect3.out"}
+    {"Run with libindirect4c.so 1"
+     "tmpdir/indirect4a.o tmpdir/indirect4b.o tmpdir/libindirect4c.so" ""
+     {dummy.c} "indirect4a" "indirect4.out"}
+    {"Run with libindirect4c.so 2"
+     "tmpdir/indirect4a.o tmpdir/libindirect4c.so tmpdir/indirect4b.o" ""
+     {dummy.c} "indirect4b" "indirect4.out"}
+    {"Run with libindirect4c.so 3"
+     "tmpdir/indirect4b.o tmpdir/libindirect4c.so tmpdir/indirect4a.o" ""
+     {dummy.c} "indirect4c" "indirect4.out"}
+    {"Run with libindirect4c.so 4"
+     "tmpdir/libindirect4c.so tmpdir/indirect4b.o tmpdir/indirect4a.o" ""
+     {dummy.c} "indirect4d" "indirect4.out"}
+}
+
+run_ld_link_exec_tests [] $run_tests
--- binutils/ld/testsuite/ld-elf/indirect1a.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect1a.c	2006-10-18 11:31:17.000000000 -0700
@@ -0,0 +1,8 @@
+extern void bar (void);
+
+int
+start (void)
+{
+  bar ();
+  return 0;
+}
--- binutils/ld/testsuite/ld-elf/indirect1b.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect1b.c	2006-10-18 11:31:28.000000000 -0700
@@ -0,0 +1,6 @@
+void
+foo (void)
+{
+}
+
+asm (".symver foo,foo@FOO");
--- binutils/ld/testsuite/ld-elf/indirect1c.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect1c.c	2006-10-18 11:31:11.000000000 -0700
@@ -0,0 +1,7 @@
+extern void foo (void);
+
+void
+bar (void)
+{
+  foo ();
+}
--- binutils/ld/testsuite/ld-elf/indirect2.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect2.c	2006-10-18 14:42:01.000000000 -0700
@@ -0,0 +1,9 @@
+extern void foo (void);
+
+asm (".symver foo,foo@@@FOO");
+
+void
+bar (void)
+{
+  foo ();
+}
--- binutils/ld/testsuite/ld-elf/indirect3.out.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect3.out	2006-10-18 15:43:56.000000000 -0700
@@ -0,0 +1,2 @@
+MAIN
+DSO
--- binutils/ld/testsuite/ld-elf/indirect3a.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect3a.c	2006-10-18 15:41:03.000000000 -0700
@@ -0,0 +1,10 @@
+extern void bar (void);
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  bar ();
+  return 0;
+}
--- binutils/ld/testsuite/ld-elf/indirect3b.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect3b.c	2006-10-18 15:41:11.000000000 -0700
@@ -0,0 +1,9 @@
+#include <stdio.h>
+
+void
+foo (void)
+{
+  printf ("MAIN\n");
+}
+
+asm (".symver foo,foo@FOO");
--- binutils/ld/testsuite/ld-elf/indirect3c.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect3c.c	2006-10-18 15:41:17.000000000 -0700
@@ -0,0 +1,15 @@
+#include <stdio.h>
+
+extern void foo (void);
+
+void
+foo (void)
+{
+  printf ("DSO\n");
+}
+
+void
+bar (void)
+{
+  foo ();
+}
--- binutils/ld/testsuite/ld-elf/indirect4.out.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect4.out	2006-10-18 16:14:07.000000000 -0700
@@ -0,0 +1,2 @@
+MAIN2
+MAIN2
--- binutils/ld/testsuite/ld-elf/indirect4a.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect4a.c	2006-10-18 16:11:52.000000000 -0700
@@ -0,0 +1,10 @@
+extern void bar (void);
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  bar ();
+  return 0;
+}
--- binutils/ld/testsuite/ld-elf/indirect4b.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect4b.c	2006-10-18 16:12:05.000000000 -0700
@@ -0,0 +1,17 @@
+#include <stdio.h>
+
+void
+foo2 (void)
+{
+  printf ("MAIN2\n");
+}
+
+asm (".symver foo2,foo@@FOO2");
+
+void
+foo1 (void)
+{
+  printf ("MAIN1\n");
+}
+
+asm (".symver foo1,foo@FOO1");
--- binutils/ld/testsuite/ld-elf/indirect4c.c.indirect	2006-10-18 17:53:28.000000000 -0700
+++ binutils/ld/testsuite/ld-elf/indirect4c.c	2006-10-18 16:12:11.000000000 -0700
@@ -0,0 +1,15 @@
+#include <stdio.h>
+
+extern void foo (void);
+
+void
+foo (void)
+{
+  printf ("DSO\n");
+}
+
+void
+bar (void)
+{
+  foo ();
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]