Re: [PATCH] PR ld/16467: Check incompatible existing default symbol definition

["H.J. Lu" <hjl dot tools at gmail dot com>]

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.
>
>> +       && (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;
>

You are right.  There is nothing linker can do when a type mismatched
default definition are made dynamic by info->shared, info->export_dynamic
or h->ref_dynamic. But we do want to avoid exporting it when building PIE.
I checked in this patch to remove those checks.


Thanks.

-- 
H.J.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 6e1e334..5923bc3 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+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
+	adding the default version.
+
 2014-01-16  Alan Modra  <amodra@gmail.com>
 
 	* elfxx-mips.c (mips_elf_record_got_page_entry): Pass in a
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 7dcafd6..d0006da 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1087,12 +1087,8 @@ _bfd_elf_merge_symbol (bfd *abfd,
 
   /* When we try to create a default indirect symbol from the dynamic
      definition with the default version, we skip it if its type and
-     the type of existing regular definition mismatch.  We only do it
-     if the existing regular definition won't be dynamic.  */
+     the type of existing regular definition mismatch.  */
   if (pold_alignment == NULL
-      && !info->shared
-      && !info->export_dynamic
-      && !h->ref_dynamic
       && newdyn
       && newdef
       && !olddyn
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 84729b6..ac65a3a 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+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.
+
 2014-01-20  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR ld/2404
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index afc262c..bbfd464 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -453,3 +453,29 @@ set run_cxx_tests {
 
 run_cc_link_tests $build_cxx_tests
 run_ld_link_exec_tests [] $run_cxx_tests
+
+if { [istarget *-*-linux*]
+     || [istarget *-*-nacl*]
+     || [istarget *-*-gnu*] } {
+    run_cc_link_tests [list \
+	[list \
+	    "Build libpr2404b.a with PIE" \
+	    "" \
+	    "-fPIE" \
+	    { pr2404b.c } \
+	    {} \
+	    "libpr2404b.a" \
+	] \
+    ]
+    run_ld_link_exec_tests [] [list \
+	[list \
+	    "Run pr2404 with PIE" \
+	    "-pie tmpdir/pr2404b.o tmpdir/libpr2404a.so" \
+	    "" \
+	    { dummy.c } \
+	    "pr2404pie" \
+	    "pr2404.out" \
+	    "-fPIE" \
+	] \
+    ]
+}