PR26978, Inconsistency for strong foo@v1 and weak foo@@v1
authorAlan Modra <amodra@gmail.com>
Wed, 2 Dec 2020 02:33:23 +0000 (13:03 +1030)
committerAlan Modra <amodra@gmail.com>
Fri, 4 Dec 2020 00:36:52 +0000 (11:06 +1030)
Prior to this patch
  ld -shared --version-script=pr26979.ver pr26978a.o pr26978b.o
results in
  ld: pr26978b.o: in function `foo_v1':
  (.text+0x0): multiple definition of `foo@v1'
  ld: pr26978b.o:(*IND*+0x0): multiple definition of `foo'
while
  ld -shared --version-script=pr26979.ver pr26978b.o pr26978a.o
results in no error, but some odd dynamic symbols.
  ... 0 NOTYPE  GLOBAL DEFAULT    7 foo@v1
  ... 0 NOTYPE  WEAK   DEFAULT    7 foo@@v1

When linking an undecorated reference to foo against such a shared
library, ld complains about multiple definitions of foo@v1 while gold
creates a dynamic reference to foo@v1.  That results in foo@v1 being
used at runtime.

While we could error in both cases, it is reasonable to say foo@v1 and
foo@@v1 are in fact the same symbol.  (Same name, same version.  The
only real difference is that foo@@v1 satisfies a reference to plain
foo, while foo@v1 does not.)  Just as merging a weak undecorated sym
with a strong sym results in the strong sym prevailing, so should the
strong foo@v1 prevail.  And since there is a definition that satisfies
plain foo, the foo@@v1 variety of dynamic symbol should be emitted at
the foo@v1 value.  That makes the testcase that currently links
continue to produce a shared library, and that shared library can now
be used by both ld and gold with the same runtime behaviour as when
using gold with the odd dynamic symbol library.

bfd/
PR 26978
* elflink.c (_bfd_elf_add_default_symbol): Handle the case where
a new weak sym@@ver should be overridden by an existing sym@ver.
(elf_link_add_object_symbols): Don't _bfd_elf_add_default_symbol
for a new weak sym@ver when sym@@ver already exists.
* linker.c (link_action): Choose MIND for previous indirect,
current def, rather than MDEF.
(_bfd_generic_link_add_one_symbol <MIND>): Handle redefinition of
weak indirect symbol.
ld/
* testsuite/ld-elf/pr26978a.d,
* testsuite/ld-elf/pr26978a.s,
* testsuite/ld-elf/pr26978b.d,
* testsuite/ld-elf/pr26978b.s: New tests.

bfd/ChangeLog
bfd/elflink.c
bfd/linker.c
ld/ChangeLog
ld/testsuite/ld-elf/pr26978a.d [new file with mode: 0644]
ld/testsuite/ld-elf/pr26978a.s [new file with mode: 0644]
ld/testsuite/ld-elf/pr26978b.d [new file with mode: 0644]
ld/testsuite/ld-elf/pr26978b.s [new file with mode: 0644]

index b3bcf9ea1e675e686a9ea71f9c40358a5f23b640..fa29df22676b81fde81c191c4cb940308dea9bab 100644 (file)
@@ -1,3 +1,15 @@
+2020-12-04  Alan Modra  <amodra@gmail.com>
+
+       PR 26978
+       * elflink.c (_bfd_elf_add_default_symbol): Handle the case where
+       a new weak sym@@ver should be overridden by an existing sym@ver.
+       (elf_link_add_object_symbols): Don't _bfd_elf_add_default_symbol
+       for a new weak sym@ver when sym@@ver already exists.
+       * linker.c (link_action): Choose MIND for previous indirect,
+       current def, rather than MDEF.
+       (_bfd_generic_link_add_one_symbol <MIND>): Handle redefinition of
+       weak indirect symbol.
+
 2020-12-01  Nelson Chu  <nelson.chu@sifive.com>
 
        * elfxx-riscv.c (riscv_parse_prefixed_ext): Use riscv_compare_subsets
index 512c5044b3b7e0834c9c910253cc9bca3bd6a680..29b46f1568db10cda14dff9175c18a5bbade3f49 100644 (file)
@@ -2078,9 +2078,26 @@ _bfd_elf_add_default_symbol (bfd *abfd,
     return FALSE;
 
   if (skip)
-    return TRUE;
-
-  if (override)
+    {
+      if (!dynamic
+         && h->root.type == bfd_link_hash_defweak
+         && hi->root.type == bfd_link_hash_defined)
+       {
+         /* We are handling a weak sym@@ver and attempting to define
+            a weak sym@ver, but _bfd_elf_merge_symbol said to skip the
+            new weak sym@ver because there is already a strong sym@ver.
+            However, sym@ver and sym@@ver are really the same symbol.
+            The existing strong sym@ver ought to override sym@@ver.  */
+         h->root.type = bfd_link_hash_defined;
+         h->root.u.def.section = hi->root.u.def.section;
+         h->root.u.def.value = hi->root.u.def.value;
+         hi->root.type = bfd_link_hash_indirect;
+         hi->root.u.i.link = &h->root;
+       }
+      else
+       return TRUE;
+    }
+  else if (override)
     {
       /* Here SHORTNAME is a versioned name, so we don't expect to see
         the type of override we do in the case above unless it is
@@ -2091,6 +2108,7 @@ _bfd_elf_add_default_symbol (bfd *abfd,
          /* xgettext:c-format */
          (_("%pB: unexpected redefinition of indirect versioned symbol `%s'"),
           abfd, shortname);
+      return TRUE;
     }
   else
     {
@@ -2100,37 +2118,36 @@ _bfd_elf_add_default_symbol (bfd *abfd,
              bfd_ind_section_ptr, 0, name, FALSE, collect, &bh)))
        return FALSE;
       hi = (struct elf_link_hash_entry *) bh;
+    }
 
-      /* If there is a duplicate definition somewhere, then HI may not
-        point to an indirect symbol.  We will have reported an error
-        to the user in that case.  */
-
-      if (hi->root.type == bfd_link_hash_indirect)
-       {
-         (*bed->elf_backend_copy_indirect_symbol) (info, h, hi);
-         h->ref_dynamic_nonweak |= hi->ref_dynamic_nonweak;
-         hi->dynamic_def |= h->dynamic_def;
+  /* If there is a duplicate definition somewhere, then HI may not
+     point to an indirect symbol.  We will have reported an error
+     to the user in that case.  */
+  if (hi->root.type == bfd_link_hash_indirect)
+    {
+      (*bed->elf_backend_copy_indirect_symbol) (info, h, hi);
+      h->ref_dynamic_nonweak |= hi->ref_dynamic_nonweak;
+      hi->dynamic_def |= h->dynamic_def;
 
-         /* If we first saw a reference to @VER symbol with
-            non-default visibility, merge that visibility to the
-            @@VER symbol.  */
-         elf_merge_st_other (abfd, h, hi->other, sec, TRUE, dynamic);
+      /* If we first saw a reference to @VER symbol with
+        non-default visibility, merge that visibility to the
+        @@VER symbol.  */
+      elf_merge_st_other (abfd, h, hi->other, sec, TRUE, dynamic);
 
-         /* See if the new flags lead us to realize that the symbol
-            must be dynamic.  */
-         if (! *dynsym)
+      /* See if the new flags lead us to realize that the symbol
+        must be dynamic.  */
+      if (! *dynsym)
+       {
+         if (! dynamic)
            {
-             if (! dynamic)
-               {
-                 if (! bfd_link_executable (info)
-                     || hi->ref_dynamic)
-                   *dynsym = TRUE;
-               }
-             else
-               {
-                 if (hi->ref_regular)
-                   *dynsym = TRUE;
-               }
+             if (! bfd_link_executable (info)
+                 || hi->ref_dynamic)
+               *dynsym = TRUE;
+           }
+         else
+           {
+             if (hi->ref_regular)
+               *dynsym = TRUE;
            }
        }
     }
@@ -5060,8 +5077,10 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 
          /* Check to see if we need to add an indirect symbol for
             the default name.  */
-         if (definition
-             || (!override && h->root.type == bfd_link_hash_common))
+         if ((definition
+              || (!override && h->root.type == bfd_link_hash_common))
+             && !(hi != h
+                  && hi->versioned == versioned_hidden))
            if (!_bfd_elf_add_default_symbol (abfd, info, h, name, isym,
                                              sec, value, &old_bfd, &dynsym))
              goto error_free_vers;
index 1357168456867148ac8b8ef47fe64f90f13a0c21..6008a4452ea0c0a31a9f00f4ef33584023312f5e 100644 (file)
@@ -1301,7 +1301,7 @@ static const enum link_action link_action[8][8] =
   /* current\prev    new    undef  undefw def    defw   com    indr   warn  */
   /* UNDEF_ROW */  {UND,   NOACT, UND,   REF,   REF,   NOACT, REFC,  WARNC },
   /* UNDEFW_ROW        */  {WEAK,  NOACT, NOACT, REF,   REF,   NOACT, REFC,  WARNC },
-  /* DEF_ROW   */  {DEF,   DEF,   DEF,   MDEF,  DEF,   CDEF,  MDEF,  CYCLE },
+  /* DEF_ROW   */  {DEF,   DEF,   DEF,   MDEF,  DEF,   CDEF,  MIND,  CYCLE },
   /* DEFW_ROW  */  {DEFW,  DEFW,  DEFW,  NOACT, NOACT, NOACT, NOACT, CYCLE },
   /* COMMON_ROW        */  {COM,   COM,   COM,   CREF,  COM,   BIG,   REFC,  WARNC },
   /* INDR_ROW  */  {IND,   IND,   IND,   MDEF,  IND,   CIND,  MIND,  CYCLE },
@@ -1672,6 +1672,17 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
        case MIND:
          /* Multiple indirect symbols.  This is OK if they both point
             to the same symbol.  */
+         if (h->u.i.link->type == bfd_link_hash_defweak)
+           {
+             /* It is also OK to redefine a symbol that indirects to
+                a weak definition.  So for sym@ver -> sym@@ver where
+                sym@@ver is weak and we have a new strong sym@ver,
+                redefine sym@@ver.  Of course if there exists
+                sym -> sym@@ver then this also redefines sym.  */
+             h = h->u.i.link;
+             cycle = TRUE;
+             break;
+           }
          if (strcmp (h->u.i.link->root.string, string) == 0)
            break;
          /* Fall through.  */
index 4c5f0090d09403eee6f6e9b2b4486219b30795c1..0bbb576c56286698c2e30d0cd28ac399a8655abf 100644 (file)
@@ -1,3 +1,10 @@
+2020-12-04  Alan Modra  <amodra@gmail.com>
+
+       * testsuite/ld-elf/pr26978a.d,
+       * testsuite/ld-elf/pr26978a.s,
+       * testsuite/ld-elf/pr26978b.d,
+       * testsuite/ld-elf/pr26978b.s: New tests.
+
 2020-12-03  Andreas Krebbel  <krebbel@linux.ibm.com>
 
        * testsuite/ld-s390/tlsbin_64.dd: The newly added jgnop mnemonic
diff --git a/ld/testsuite/ld-elf/pr26978a.d b/ld/testsuite/ld-elf/pr26978a.d
new file mode 100644 (file)
index 0000000..969f34b
--- /dev/null
@@ -0,0 +1,11 @@
+#source: pr26978a.s
+#source: pr26978b.s
+#target: [check_shared_lib_support]
+#as:
+#ld: -shared --version-script=pr26979.ver
+#readelf: -sW
+
+#failif
+#...
+.*foo@v1
+#pass
diff --git a/ld/testsuite/ld-elf/pr26978a.s b/ld/testsuite/ld-elf/pr26978a.s
new file mode 100644 (file)
index 0000000..abb035d
--- /dev/null
@@ -0,0 +1,4 @@
+ .weak foo
+ .symver foo, foo@@@v1
+foo:
+ .octa 0
diff --git a/ld/testsuite/ld-elf/pr26978b.d b/ld/testsuite/ld-elf/pr26978b.d
new file mode 100644 (file)
index 0000000..fa29086
--- /dev/null
@@ -0,0 +1,11 @@
+#source: pr26978b.s
+#source: pr26978a.s
+#target: [check_shared_lib_support]
+#as:
+#ld: -shared --version-script=pr26979.ver
+#readelf: -sW
+
+#failif
+#...
+.*foo@v1
+#pass
diff --git a/ld/testsuite/ld-elf/pr26978b.s b/ld/testsuite/ld-elf/pr26978b.s
new file mode 100644 (file)
index 0000000..59b6ac2
--- /dev/null
@@ -0,0 +1,4 @@
+ .globl foo_v1
+ .symver foo_v1, foo@v1
+foo_v1:
+ .octa 0