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] PR ld/16467: Check incompatible existing default symbol definition


On Mon, Jan 20, 2014 at 8:40 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Jan 20, 2014 at 05:30:20AM -0800, H.J. Lu wrote:
>>    if (pold_alignment == NULL
>> -      && !info->shared
>> -      && !info->export_dynamic
>> -      && !h->ref_dynamic
>>        && newdyn
>>        && newdef
>>        && !olddyn
>> -      && (olddef || h->root.type == bfd_link_hash_common)
>> -      && ELF_ST_TYPE (sym->st_info) != h->type
>> -      && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
>> -      && h->type != STT_NOTYPE
>> -      && !(newfunc && oldfunc))
>> +      && ((!info->shared
>> +       && !info->export_dynamic
>> +       && !h->ref_dynamic
>
> I'd like to understand why you kept the above three lines.  I'm
> inclined to think they ought to disappear.  If it is correct to omit
> the default symbol for an executable, why not a shared library, and
> why is it important to test ref_dynamic?  Try linking your pr2404
> testcase as a pie.
>

Here is the updated patch with those 3 tests removed.  OK
for trunk?

Thanks.

-- 
H.J.
From 78f197b88d6c6b0939a43af4f0381c959ea7e70d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 19 Jan 2014 17:25:00 -0800
Subject: [PATCH] Check incompatible existing default symbol definition

After resolving a versioned reference, foo@VER1, to a default versioned
definition, foo@@VER1, from a shared object, we also merge it with
the existing regular default symbol definition, foo.  When foo is IFUNC
and foo@@VER1 aren't, we will merge 2 incompatible definitions.  This
patch avoids merging foo@@VER1 definition with foo definition if
one is IFUNC and the other isn't.
---
 bfd/ChangeLog                      |  8 +++++++
 bfd/elflink.c                      | 13 ++++++-----
 ld/testsuite/ChangeLog             | 14 ++++++++++++
 ld/testsuite/ld-ifunc/dummy.c      |  1 +
 ld/testsuite/ld-ifunc/ifunc.exp    | 45 ++++++++++++++++++++++++++++++++++++++
 ld/testsuite/ld-ifunc/pr16467.out  |  1 +
 ld/testsuite/ld-ifunc/pr16467a.c   |  5 +++++
 ld/testsuite/ld-ifunc/pr16467a.map |  4 ++++
 ld/testsuite/ld-ifunc/pr16467b.c   |  7 ++++++
 ld/testsuite/ld-ifunc/pr16467b.map |  4 ++++
 ld/testsuite/ld-ifunc/pr16467c.c   |  9 ++++++++
 11 files changed, 106 insertions(+), 5 deletions(-)
 create mode 100644 ld/testsuite/ld-ifunc/dummy.c
 create mode 100644 ld/testsuite/ld-ifunc/pr16467.out
 create mode 100644 ld/testsuite/ld-ifunc/pr16467a.c
 create mode 100644 ld/testsuite/ld-ifunc/pr16467a.map
 create mode 100644 ld/testsuite/ld-ifunc/pr16467b.c
 create mode 100644 ld/testsuite/ld-ifunc/pr16467b.map
 create mode 100644 ld/testsuite/ld-ifunc/pr16467c.c

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 5923bc3..c70a7db 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,5 +1,13 @@
 2014-01-21  H.J. Lu  <hongjiu.lu@intel.com>
 
+	PR ld/16467
+	* elflink.c (_bfd_elf_merge_symbol): When types of the existing
+	regular default symbol definition and the versioned dynamic
+	symbol definition mismatch, skip the default symbol definition
+	if one of them is IFUNC.
+
+2014-01-21  H.J. Lu  <hongjiu.lu@intel.com>
+
 	PR ld/2404
 	* elflink.c (_bfd_elf_merge_symbol): Don't check info->shared,
 	info->export_dynamic nor h->ref_dynamic for type mismatch when
diff --git a/bfd/elflink.c b/bfd/elflink.c
index d0006da..792e14e 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1092,11 +1092,14 @@ _bfd_elf_merge_symbol (bfd *abfd,
       && newdyn
       && newdef
       && !olddyn
-      && (olddef || h->root.type == bfd_link_hash_common)
-      && ELF_ST_TYPE (sym->st_info) != h->type
-      && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
-      && h->type != STT_NOTYPE
-      && !(newfunc && oldfunc))
+      && (((olddef || h->root.type == bfd_link_hash_common)
+	   && ELF_ST_TYPE (sym->st_info) != h->type
+	   && ELF_ST_TYPE (sym->st_info) != STT_NOTYPE
+	   && h->type != STT_NOTYPE
+	   && !(newfunc && oldfunc))
+	  || (olddef
+	      && ((h->type == STT_GNU_IFUNC)
+		  != (ELF_ST_TYPE (sym->st_info) == STT_GNU_IFUNC)))))
     {
       *skip = TRUE;
       return TRUE;
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index ac65a3a..a092428 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,19 @@
 2014-01-21  H.J. Lu  <hongjiu.lu@intel.com>
 
+	PR ld/16467
+	* ld-ifunc/dummy.c: New file.
+	* ld-ifunc/pr16467.out: Likewise.
+	* ld-ifunc/pr16467a.c: Likewise.
+	* ld-ifunc/pr16467a.map: Likewise.
+	* ld-ifunc/pr16467b.c: Likewise.
+	* ld-ifunc/pr16467b.map: Likewise.
+	* ld-ifunc/pr16467c.c: Likewise.
+
+	* ld-ifunc/ifunc.exp (run_cc_link_tests): New.
+	(run_ld_link_exec_tests): Run pr16467.
+
+2014-01-21  H.J. Lu  <hongjiu.lu@intel.com>
+
 	PR ld/2404
 	* ld-elf/shared.exp: Add a PIE test for PR ld/2404.
 
diff --git a/ld/testsuite/ld-ifunc/dummy.c b/ld/testsuite/ld-ifunc/dummy.c
new file mode 100644
index 0000000..5c03287
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/dummy.c
@@ -0,0 +1 @@
+/* An empty file.  */
diff --git a/ld/testsuite/ld-ifunc/ifunc.exp b/ld/testsuite/ld-ifunc/ifunc.exp
index fb106c6..d7ff445 100644
--- a/ld/testsuite/ld-ifunc/ifunc.exp
+++ b/ld/testsuite/ld-ifunc/ifunc.exp
@@ -349,6 +349,42 @@ if { $verbose < 1 } {
     remote_file host delete "tmpdir/static_nonifunc_prog"
 }
 
+run_cc_link_tests [list \
+    [list \
+	"Build libpr16467a.so" \
+	"-shared -Wl,--version-script=pr16467a.map" \
+	"-fPIC" \
+	{ pr16467a.c } \
+	{} \
+	"libpr16467a.so" \
+    ] \
+    [list \
+	"Build libpr16467b.a" \
+	"" \
+	"-fPIC" \
+	{ pr16467b.c } \
+	{} \
+	"libpr16467b.a" \
+    ] \
+    [list \
+	"Build libpr16467b.so" \
+	"-shared tmpdir/pr16467b.o tmpdir/libpr16467a.so \
+	 -Wl,--version-script=pr16467b.map" \
+	"-fPIC" \
+	{ dummy.c } \
+	{} \
+	"libpr16467b.so" \
+    ] \
+    [list \
+	"Build libpr16467c.a" \
+	"" \
+	"" \
+	{ pr16467c.c } \
+	{} \
+	"libpr16467c.a" \
+    ] \
+]
+
 run_ld_link_exec_tests [] [list \
     [list \
 	"Common symbol override ifunc test 1a" \
@@ -368,6 +404,15 @@ run_ld_link_exec_tests [] [list \
 	"ifunc-common-1.out" \
 	"-g" \
     ] \
+    [list \
+	"Run pr16467" \
+	"tmpdir/pr16467c.o tmpdir/libpr16467b.so tmpdir/libpr16467a.so" \
+	"" \
+	{ dummy.c } \
+	"pr16467" \
+	"pr16467.out" \
+	"" \
+    ] \
 ]
 
 set test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
diff --git a/ld/testsuite/ld-ifunc/pr16467.out b/ld/testsuite/ld-ifunc/pr16467.out
new file mode 100644
index 0000000..d86bac9
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467.out
@@ -0,0 +1 @@
+OK
diff --git a/ld/testsuite/ld-ifunc/pr16467a.c b/ld/testsuite/ld-ifunc/pr16467a.c
new file mode 100644
index 0000000..ae3f084
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467a.c
@@ -0,0 +1,5 @@
+const char * 
+sd_get_seats(void)
+{
+  return "OK";
+}
diff --git a/ld/testsuite/ld-ifunc/pr16467a.map b/ld/testsuite/ld-ifunc/pr16467a.map
new file mode 100644
index 0000000..d677f37
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467a.map
@@ -0,0 +1,4 @@
+LIBSYSTEMD_209 {
+global:
+        sd_get_seats;
+};
diff --git a/ld/testsuite/ld-ifunc/pr16467b.c b/ld/testsuite/ld-ifunc/pr16467b.c
new file mode 100644
index 0000000..264f6cf
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467b.c
@@ -0,0 +1,7 @@
+void new_sd_get_seats(void);
+__asm__(".symver new_sd_get_seats,sd_get_seats@LIBSYSTEMD_209");
+void (*resolve_sd_get_seats(void)) (void) __asm__ ("sd_get_seats");
+void (*resolve_sd_get_seats(void)) (void) {
+        return new_sd_get_seats;
+}
+__asm__(".type sd_get_seats, %gnu_indirect_function");
diff --git a/ld/testsuite/ld-ifunc/pr16467b.map b/ld/testsuite/ld-ifunc/pr16467b.map
new file mode 100644
index 0000000..1f263de
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467b.map
@@ -0,0 +1,4 @@
+LIBSYSTEMD_208 {
+global:
+        sd_get_seats;
+};
diff --git a/ld/testsuite/ld-ifunc/pr16467c.c b/ld/testsuite/ld-ifunc/pr16467c.c
new file mode 100644
index 0000000..e2a901c
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr16467c.c
@@ -0,0 +1,9 @@
+#include <stdio.h>
+const char* sd_get_seats(void);
+
+int
+main (int argc, char **argv)
+{
+  printf("%s\n", sd_get_seats());
+  return 0;
+}
-- 
1.8.4.2


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