[ELF] Combine foo@v1 and foo with the same versionId if both are defined

Due to an assembler design flaw (IMO), `.symver foo,foo@v1` produces two symbols `foo` and `foo@v1` if `foo` is defined.

* `v1 {};` produces both `foo` and `foo@v1`, but GNU ld only produces `foo@v1`
* `v1 { foo; };` produces both `foo@@v1` and `foo@v1`, but GNU ld only produces `foo@v1`
* `v2 { foo; };` produces both `foo@@v2` and `foo@v1`, matching GNU ld. (Tested by symver.s)

This patch implements the GNU ld behavior by reusing the symbol redirection mechanism
in D92259. The new test symver-non-default.s checks the first two cases.

Without the patch, the second case will produce `foo@v1` and `foo@@v1` which
looks weird and makes foo unnecessarily default versioned.

Note: `.symver foo,foo@v1,remove` exists but the unfortunate `foo` will not go
away anytime soon.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D107235

Author: MaskRay

lld/ELF/Driver.cpp

@@ -2071,23 +2071,37 @@ static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
     if (suffix1[0] != '@' || suffix1[1] == '@')
       continue;
 
-    // Check whether the default version foo@@v1 exists. If it exists, the
-    // symbol can be found by the name "foo" in the symbol table.
-    Symbol *maybeDefault = symtab->find(name);
-    if (!maybeDefault)
-      continue;
-    const char *suffix2 = maybeDefault->getVersionSuffix();
-    if (suffix2[0] != '@' || suffix2[1] != '@' ||
-        strcmp(suffix1 + 1, suffix2 + 2) != 0)
+    // Check the existing symbol foo. We have two special cases to handle:
+    //
+    // * There is a definition of foo@v1 and foo@@v1.
+    // * There is a definition of foo@v1 and foo.
+    Defined *sym2 = dyn_cast_or_null<Defined>(symtab->find(name));
+    if (!sym2)
       continue;
-
-    // foo@v1 and foo@@v1 should be merged, so redirect foo@v1 to foo@@v1.
-    map.try_emplace(sym, maybeDefault);
-    // If both foo@v1 and foo@@v1 are defined and non-weak, report a duplicate
-    // definition error.
-    maybeDefault->resolve(*sym);
-    // Eliminate foo@v1 from the symbol table.
-    sym->symbolKind = Symbol::PlaceholderKind;
+    const char *suffix2 = sym2->getVersionSuffix();
+    if (suffix2[0] == '@' && suffix2[1] == '@' &&
+        strcmp(suffix1 + 1, suffix2 + 2) == 0) {
+      // foo@v1 and foo@@v1 should be merged, so redirect foo@v1 to foo@@v1.
+      map.try_emplace(sym, sym2);
+      // If both foo@v1 and foo@@v1 are defined and non-weak, report a duplicate
+      // definition error.
+      sym2->resolve(*sym);
+      // Eliminate foo@v1 from the symbol table.
+      sym->symbolKind = Symbol::PlaceholderKind;
+    } else if (auto *sym1 = dyn_cast<Defined>(sym)) {
+      if (sym2->versionId > VER_NDX_GLOBAL
+              ? config->versionDefinitions[sym2->versionId].name == suffix1 + 1
+              : sym1->section == sym2->section && sym1->value == sym2->value) {
+        // Due to an assembler design flaw, if foo is defined, .symver foo,
+        // foo@v1 defines both foo and foo@v1. Unless foo is bound to a
+        // different version, GNU ld makes foo@v1 canonical and elimiates foo.
+        // Emulate its behavior, otherwise we would have foo or foo@@v1 beside
+        // foo@v1. foo@v1 and foo combining does not apply if they are not
+        // defined in the same place.
+        map.try_emplace(sym2, sym);
+        sym2->symbolKind = Symbol::PlaceholderKind;
+      }
+    }
   }
 
   if (map.empty())

lld/test/ELF/symver-non-default.s

@@ -0,0 +1,69 @@
+# REQUIRES: x86
+## Test symbol resolution related to .symver produced non-default version symbols.
+
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/ref.s -o %t/ref.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/def1.s -o %t/def1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/def2.s -o %t/def2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/def3.s -o %t/def3.o
+
+## foo@v1 & foo defined at the same location are combined.
+# RUN: ld.lld -shared --version-script=%t/ver1 %t/def1.o %t/ref.o -o %t1
+# RUN: llvm-readelf --dyn-syms %t1 | FileCheck %s --check-prefix=CHECK1
+# RUN: ld.lld -shared --version-script=%t/ver2 %t/def1.o %t/ref.o -o %t1
+# RUN: llvm-readelf --dyn-syms %t1 | FileCheck %s --check-prefix=CHECK1
+
+# CHECK1:       1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
+# CHECK1-EMPTY:
+
+## def2.o doesn't define foo. foo@v1 & undefined foo are unrelated.
+# RUN: ld.lld -shared --version-script=%t/ver1 %t/def2.o %t/ref.o -o %t2
+# RUN: llvm-readelf -r --dyn-syms %t2 | FileCheck %s --check-prefix=CHECK2
+
+# CHECK2:       R_X86_64_JUMP_SLOT {{.*}} foo + 0
+# CHECK2:       1: {{.*}} NOTYPE GLOBAL DEFAULT UND   foo{{$}}
+# CHECK2-NEXT:  2: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
+# CHECK2-EMPTY:
+
+## def2.o doesn't define foo. foo@v1 & defined foo are unrelated.
+# RUN: ld.lld -shared --version-script=%t/ver1 %t/def2.o %t/def3.o %t/ref.o -o %t3
+# RUN: llvm-readelf -r --dyn-syms %t3 | FileCheck %s --check-prefix=CHECK3
+
+# CHECK3:       R_X86_64_JUMP_SLOT {{.*}} foo + 0
+# CHECK3:       1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
+# CHECK3-NEXT:  2: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo{{$}}
+# CHECK3-EMPTY:
+
+## foo@v1 overrides the defined foo which is affected by a version script.
+# RUN: ld.lld -shared --version-script=%t/ver2 %t/def2.o %t/def3.o %t/ref.o -o %t4
+# RUN: llvm-readelf -r --dyn-syms %t4 | FileCheck %s --check-prefix=CHECK4
+
+# CHECK4:       R_X86_64_JUMP_SLOT {{.*}} foo@v1 + 0
+# CHECK4:       1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@v1
+# CHECK4-EMPTY:
+
+#--- ver1
+v1 {};
+
+#--- ver2
+v1 { foo; };
+
+#--- ref.s
+call foo
+
+#--- def1.s
+.globl foo
+.symver foo, foo@v1
+foo:
+  ret
+
+#--- def2.s
+.globl foo_v1
+.symver foo_v1, foo@v1, remove
+foo_v1:
+  ret
+
+#--- def3.s
+.globl foo
+foo:
+  ret

lld/test/ELF/version-script-symver.s

@@ -1,12 +1,13 @@
 # REQUIRES: x86
 ## Test how .symver interacts with --version-script.
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: echo 'call foo3; call foo4' > %tref.s
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %tref.s -o %tref.o
 
 # RUN: echo 'v1 { local: foo1; }; v2 { local: foo2; };' > %t1.script
 # RUN: ld.lld --version-script %t1.script -shared %t.o -o %t1.so
 # RUN: llvm-readelf --dyn-syms %t1.so | FileCheck --check-prefix=EXACT %s
 # EXACT:      UND
-# EXACT-NEXT: [[#]] foo3{{$}}
 # EXACT-NEXT: [[#]] foo4@@v2
 # EXACT-NEXT: [[#]] _start{{$}}
 # EXACT-NEXT: [[#]] foo3@v1
@@ -34,11 +35,16 @@
 # MIX2:      UND
 # MIX2-NEXT: [[#]] foo1@@v1
 # MIX2-NEXT: [[#]] foo2@@v1
-# MIX2-NEXT: [[#]] foo3@@v1
 # MIX2-NEXT: [[#]] foo4@@v2
 # MIX2-NEXT: [[#]] foo3@v1
 # MIX2-NOT:  {{.}}
 
+# RUN: ld.lld --version-script %t4.script -shared %t.o %tref.o -o %t5.so
+# RUN: llvm-readelf -r %t5.so | FileCheck --check-prefix=RELOC %s
+
+# RELOC: R_X86_64_JUMP_SLOT {{.*}} foo3@v1 + 0
+# RELOC: R_X86_64_JUMP_SLOT {{.*}} foo4@@v2 + 0
+
 .globl foo1; foo1: ret
 .globl foo2; foo2: ret
 .globl foo3; .symver foo3,foo3@v1; foo3: ret

lld/test/ELF/version-symbol-undef.s

@@ -5,7 +5,7 @@
 // RUN:       .quad \"basename@FBSD_1.1\" " > %t.s
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %t.s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t2.o
-// RUN: echo "FBSD_1.0 { global: basename; local: *; }; FBSD_1.1 { };" > %t2.ver
+// RUN: echo "FBSD_1.0 { global: basename; local: *; }; FBSD_1.1 { basename; };" > %t2.ver
 // RUN: ld.lld --shared --version-script %t2.ver %t2.o -o %t2.so
 // RUN: echo "FBSD_1.0 { }; FBSD_1.1 { }; LIBPKG_1.3 { };" > %t.ver
 // RUN: ld.lld --shared %t.o --version-script %t.ver %t2.so -o %t.so