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]

Undo dynamic symbol state after regular object sym type mismatch


This fixes a generic ELF bug I noticed when looking at
https://sourceware.org/ml/binutils/2017-04/msg00137.html

We already handle the case of an object file first defining a symbol
that a later shared library also defines where the symbol types are
incompatible.  This patch handles the reverse, when a later object
file defines an incompatible symbol defined by an earlier shared
library.

The new testcases expose bugs in a number of ELF backends, the ones I
saw being aarch64, arm, mips, mips64, nios2 and sparc64.  There are
likely more that I wouldn't see due to not having cross-compilers
installed.

bfd/
	* elflink.c (_bfd_elf_merge_symbol): Undo dynamic linking
	state when a regular object file defines a symbol with
	incompatible type to that defined by an earlier shared lib.
ld/
	* testsuite/ld-elf/indirect5a.c,
	* testsuite/ld-elf/indirect5b.c,
	* testsuite/ld-elf/indirect5.map,
	* testsuite/ld-elf/indirect5.out: New test.
	* testsuite/ld-elf/indirect6a.c: Likewise.
	* testsuite/ld-elf/indirect.exp (check_dynamic_syms): New proc.
	Run new tests and check dynsyms.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index f26882f..b113a8b 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2017-04-17  Alan Modra  <amodra@gmail.com>
+
+	* elflink.c (_bfd_elf_merge_symbol): Undo dynamic linking
+	state when a regular object file defines a symbol with
+	incompatible type to that defined by an earlier shared lib.
+
 2017-04-13  Alan Modra  <amodra@gmail.com>
 
 	* coffcode.h: Wrap some overly long _bfd_error_handler args.
diff --git a/bfd/elflink.c b/bfd/elflink.c
index dfcc51e..ec2032b 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1242,23 +1242,50 @@ _bfd_elf_merge_symbol (bfd *abfd,
   oldfunc = (h->type != STT_NOTYPE
 	     && bed->is_function_type (h->type));
 
-  /* If creating a default indirect symbol ("foo" or "foo@") from a
-     dynamic versioned definition ("foo@@") skip doing so if there is
-     an existing regular definition with a different type.  We don't
-     want, for example, a "time" variable in the executable overriding
-     a "time" function in a shared library.  */
-  if (pold_alignment == NULL
-      && newdyn
-      && newdef
-      && !olddyn
-      && (olddef || h->root.type == bfd_link_hash_common)
+  if (!(newfunc && oldfunc)
       && ELF_ST_TYPE (sym->st_info) != h->type
       && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
       && h->type != STT_NOTYPE
-      && !(newfunc && oldfunc))
+      && (newdef || bfd_is_com_section (sec))
+      && (olddef || h->root.type == bfd_link_hash_common))
     {
-      *skip = TRUE;
-      return TRUE;
+      /* If creating a default indirect symbol ("foo" or "foo@") from
+	 a dynamic versioned definition ("foo@@") skip doing so if
+	 there is an existing regular definition with a different
+	 type.  We don't want, for example, a "time" variable in the
+	 executable overriding a "time" function in a shared library.  */
+      if (newdyn
+	  && !olddyn)
+	{
+	  *skip = TRUE;
+	  return TRUE;
+	}
+
+      /* When adding a symbol from a regular object file after we have
+	 created indirect symbols, undo the indirection and any
+	 dynamic state.  */
+      if (hi != h
+	  && !newdyn
+	  && olddyn)
+	{
+	  h = hi;
+	  (*bed->elf_backend_hide_symbol) (info, h, TRUE);
+	  h->forced_local = 0;
+	  h->ref_dynamic = 0;
+	  h->def_dynamic = 0;
+	  h->dynamic_def = 0;
+	  if (h->root.u.undef.next || info->hash->undefs_tail == &h->root)
+	    {
+	      h->root.type = bfd_link_hash_undefined;
+	      h->root.u.undef.abfd = abfd;
+	    }
+	  else
+	    {
+	      h->root.type = bfd_link_hash_new;
+	      h->root.u.undef.abfd = NULL;
+	    }
+	  return TRUE;
+	}
     }
 
   /* Check TLS symbols.  We don't check undefined symbols introduced
diff --git a/ld/ChangeLog b/ld/ChangeLog
index b55c436..793847d 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,13 @@
+2017-04-17  Alan Modra  <amodra@gmail.com>
+
+	* testsuite/ld-elf/indirect5a.c,
+	* testsuite/ld-elf/indirect5b.c,
+	* testsuite/ld-elf/indirect5.map,
+	* testsuite/ld-elf/indirect5.out: New test.
+	* testsuite/ld-elf/indirect6a.c: Likewise.
+	* testsuite/ld-elf/indirect.exp (check_dynamic_syms): New proc.
+	Run new tests and check dynsyms.
+
 2017-04-11  Alan Modra  <amodra@gmail.com>
 
 	PR 21274
diff --git a/ld/testsuite/ld-elf/indirect.exp b/ld/testsuite/ld-elf/indirect.exp
index 5c5382c..128d4a7 100644
--- a/ld/testsuite/ld-elf/indirect.exp
+++ b/ld/testsuite/ld-elf/indirect.exp
@@ -85,6 +85,9 @@ set build_tests {
   {"Build libindirect4c.so"
    "-shared" "-fPIC"
    {indirect4c.c} {} "libindirect4c.so"}
+  {"Build libindirect5.so"
+   "-shared -Wl,--version-script=indirect5.map" "-fPIC"
+   {indirect5b.c} {} "libindirect5.so"}
   {"Build libpr18720c.so"
    "-shared" "-fPIC"
    {pr18720c.c} {} "libpr18720c.so"}
@@ -151,6 +154,18 @@ set run_tests {
     {"Run with libindirect4c.so 4"
      "-Wl,--no-as-needed tmpdir/libindirect4c.so tmpdir/indirect4b.o tmpdir/indirect4a.o" ""
      {dummy.c} "indirect4d" "indirect4.out"}
+    {"Run indirect5 1"
+     "-Wl,--no-as-needed tmpdir/libindirect5.so" ""
+     {indirect5a.c} "indirect5a" "indirect5.out"}
+    {"Run indirect5 2"
+     "-Wl,--no-as-needed tmpdir/indirect5a.o tmpdir/libindirect5.so" ""
+     {dummy.c} "indirect5b" "indirect5.out"}
+    {"Run indirect6 1"
+     "-Wl,--no-as-needed tmpdir/libindirect5.so" ""
+     {indirect6a.c} "indirect6a" "indirect5.out"}
+    {"Run indirect6 2"
+     "-Wl,--no-as-needed tmpdir/indirect6a.o tmpdir/libindirect5.so" ""
+     {dummy.c} "indirect6b" "indirect5.out"}
     {"Run with libpr18720c.so 1"
      "-Wl,--no-as-needed tmpdir/pr18720a.o tmpdir/pr18720b.o tmpdir/libpr18720c.so" ""
      {check-ptr-eq.c} "pr18720a" "pr18720.out"}
@@ -178,3 +193,62 @@ set run_tests {
 }
 
 run_ld_link_exec_tests $run_tests
+
+# Check that "bar" is not dynamic in the executable
+proc check_dynamic_syms { test } {
+    global nm
+    set cmd "$nm -D $test > dump.out"
+    send_log "$cmd\n"
+    catch "exec $cmd" comp_output
+    if ![string match "" $comp_output] then {
+	send_log "$comp_output\n"
+	return 0
+    }
+    if { [string match "* bar\n*" [file_contents "dump.out"]] } then {
+	verbose "output is [file_contents "dump.out"]"
+	return 0
+    }
+    return 1
+}
+
+foreach t [list indirect5a indirect5b indirect6a indirect6b] {
+    set testname [concat $t "dynsym"]
+    if { [check_dynamic_syms tmpdir/$t] } {
+	pass $testname
+    } else {
+	fail $testname
+    }
+}
+
+send_log "$CC -fPIE -pie $srcdir/$subdir/main.c -o tmpdir/pie"
+catch "exec $CC -fPIE -pie $srcdir/$subdir/main.c -o tmpdir/pie" exec_output
+send_log "$exec_output"
+if { ! [string match "" $exec_output] } {
+    return
+}
+
+set pie_tests {
+    {"Run indirect5 3"
+     "-pie -Wl,--no-as-needed tmpdir/libindirect5.so" ""
+     {indirect5a.c} "indirect5c" "indirect5.out" "-fPIE"}
+    {"Run indirect5 4"
+     "-pie -Wl,--no-as-needed tmpdir/indirect5a.o tmpdir/libindirect5.so" ""
+     {dummy.c} "indirect5d" "indirect5.out" "-fPIE"}
+    {"Run indirect6 3"
+     "-pie -Wl,--no-as-needed tmpdir/libindirect5.so" ""
+     {indirect6a.c} "indirect6c" "indirect5.out" "-fPIE"}
+    {"Run indirect6 4"
+     "-pie -Wl,--no-as-needed tmpdir/indirect6a.o tmpdir/libindirect5.so" "-fPIE"
+     {dummy.c} "indirect6d" "indirect5.out" "-fPIE"}
+}
+
+run_ld_link_exec_tests $pie_tests
+
+foreach t [list indirect5c indirect5d indirect6c indirect6d] {
+    set testname [concat $t "dynsym"]
+    if { [check_dynamic_syms tmpdir/$t] } {
+	pass $testname
+    } else {
+	fail $testname
+    }
+}
diff --git a/ld/testsuite/ld-elf/indirect5.map b/ld/testsuite/ld-elf/indirect5.map
new file mode 100644
index 0000000..f62b414
--- /dev/null
+++ b/ld/testsuite/ld-elf/indirect5.map
@@ -0,0 +1,6 @@
+FOO
+{
+global:
+  foo;
+  bar;
+};
diff --git a/ld/testsuite/ld-elf/indirect5.out b/ld/testsuite/ld-elf/indirect5.out
new file mode 100644
index 0000000..3bd1f0e
--- /dev/null
+++ b/ld/testsuite/ld-elf/indirect5.out
@@ -0,0 +1,2 @@
+foo
+bar
diff --git a/ld/testsuite/ld-elf/indirect5a.c b/ld/testsuite/ld-elf/indirect5a.c
new file mode 100644
index 0000000..9e150fe
--- /dev/null
+++ b/ld/testsuite/ld-elf/indirect5a.c
@@ -0,0 +1,9 @@
+extern void foo (long *);
+long bar;
+
+int
+main (void)
+{
+  foo (&bar);
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/indirect5b.c b/ld/testsuite/ld-elf/indirect5b.c
new file mode 100644
index 0000000..7659971
--- /dev/null
+++ b/ld/testsuite/ld-elf/indirect5b.c
@@ -0,0 +1,15 @@
+#include <stdio.h>
+
+void
+bar (void)
+{
+  printf ("bar\n");
+}
+
+void
+foo (long *x)
+{
+  (void) x;
+  printf ("foo\n");
+  bar ();
+}
diff --git a/ld/testsuite/ld-elf/indirect6a.c b/ld/testsuite/ld-elf/indirect6a.c
new file mode 100644
index 0000000..1545db1
--- /dev/null
+++ b/ld/testsuite/ld-elf/indirect6a.c
@@ -0,0 +1,9 @@
+extern void foo (long *);
+long bar = 1;
+
+int
+main (void)
+{
+  foo (&bar);
+  return 0;
+}

-- 
Alan Modra
Australia Development Lab, IBM


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