Re: Revert pr16467 change

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

On Wed, Jun 1, 2016 at 6:51 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jun 1, 2016 at 6:28 AM, Alan Modra <amodra@gmail.com> wrote:
>> This reverts the pr16467 change, which was incorrect due to faulty
>> analysis of the pr16467 testcase.  The failure was not due to a
>> mismatch in symbol type (ifunc/non-ifunc) but due to a symbol loop
>> being set up.  I've added a pr16467d.c that shows the same symbol
>> loop without ifunc being involved.
>
> I agree.  PR 16467 is an odd case which moves a versoned symbol
> from one DSO to another.
>
>> I'm not 100% happy with this fix, due to not always having symbol
>> versions around early enough.  See the comment.  However, comparing
>> versions directly addresses the problem exposed by the testcase, and I
>> don't see any other approach that will work in all cases.
>>
>> The strtab save/restore change is necessary due to
>> _bfd_elf_add_default_symbol now calling elf_backend_hide_symbol, which
>> decrements dynstr strtab refcounts.  Previously we only added new
>> dynamic symbols.
>>
>> I'll commit this in a few days if no more problems like the strtab
>> refcounting one appear.
>
> Can you check your change into a branch?  I'd like to add a few
> more testcases on top of your change.
>

There is one case:

hjl@gnu-6 pr16467c]$ gcc -B./  -o x pr16467c.o pr16467b.o
libpr16467a.so -Wl,-R,.
[hjl@gnu-6 pr16467c]$ ./x
./x: symbol lookup error: ./x: undefined symbol:
[hjl@gnu-6 pr16467c]$ readelf --dyn-sym x

Symbol table '.dynsym' contains 11 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND puts@GLIBC_2.2.5 (2)
     2: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND
__libc_start_main@GLIBC_2.2.5 (2)
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     4: 0000000000000000 0x20eb268 NOTYPE  LOCAL  DEFAULT  UND <corrupt>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Linker should refuse to link with something like

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 93e7dd2..58f2554 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -9396,6 +9408,15 @@ elf_link_output_extsym (struct bfd_hash_entry
*bh, void *data)
  foo is used with no version, then we add an indirect symbol
  foo which points to foo@@GNU_1.2.  We ignore these symbols,
  since the indirected symbol is already in the hash table.  */
+      if (h->dynindx != -1)
+ {
+  (*_bfd_error_handler)
+    (_("%B: incompatible versioned symbol `%s'"),
+     flinfo->output_bfd, h->root.root.string);
+  bfd_set_error (bfd_error_bad_value);
+  eoinfo->failed = TRUE;
+  return FALSE;
+ }
       return TRUE;
     }



     5: 0000000000600a7c     0 NOTYPE  GLOBAL DEFAULT   24 _edata
     6: 0000000000600a80     0 NOTYPE  GLOBAL DEFAULT   25 _end
     7: 0000000000400639    13 IFUNC   GLOBAL DEFAULT   13 sd_get_seats
     8: 0000000000600a7c     0 NOTYPE  GLOBAL DEFAULT   25 __bss_start
     9: 00000000004004c0     0 FUNC    GLOBAL DEFAULT   11 _init
    10: 00000000004006c4     0 FUNC    GLOBAL DEFAULT   14 _fini
[hjl@gnu-6 pr16467c]$


-- 
H.J.