[ELF] - Give a symbol version extracted from name a priority over version set by script.

[grimar]

Summary

This fixes PR33712.

Imagine following script and code:

VER1 { global: foo; local: *; };
VER2 { global: foo; };

.global bar
bar:
.symver bar, foo@VER1

.global zed
zed:
.symver zed, foo@@VER2

We add foo@@VER2 as foo to symbol table, because have to resolve references to
foo for default symbols.
Later we are trying to assign symbol versions from script. For that we are searching for 'foo'
again. Here it is placed under VER1 and VER2 at the same time, we find it twice and trying to
set version again both times, hence LLD shows a warning.
Though sample code is correct: we have 2 different versions of foo.

Probably easiest way is to give .symver a priority over version script and do not reassign versions.
Looks bfd and gold do the same here.

[ruiu]

ELF/SymbolTable.cpp

234 ↗

(On Diff #105866)

Got is in general not a good name in the linker as it could mean Global Offset Table.

ELF/Symbols.h

366–370 ↗

(On Diff #105866)

Do you really need this? You could just call parseSymbolVersion after processing version scripts, no?

[grimar]

• Addressed review comments.

ELF/Symbols.h

366–370 ↗

(On Diff #105866)

I can drop this flag with a cost of one more lookup of '@' (updated the diff to demonstrate).

Without that it will not work so simple becaue of next:
When we have symbol foo@@VER2 we drop version and add it as foo to SymbolTable.
One of steps proccessing version scripts is a call of assignExactVersion and it do:

std::vector<SymbolBody *> Syms = findByVersion(Ver);

For this call Ver.Name is foo for VER1 and VER2. Hence it will try to assign version to the same
symbol twice and will report warning.

[ruiu]

ELF/SymbolTable.cpp

705 ↗

(On Diff #105984)

Can you replace this with

if (Sym->VersionId != Config->DefaultSymbolVersion)

to remove Symbol::InVersionScript?

756 ↗

(On Diff #105984)

Indentation

[grimar]

• Rebased.
• Fixed identation.

[ruiu]

LGTM

ELF/SymbolTable.cpp

698–699 ↗

(On Diff #106171)

// Skip symbols containing version info because symbol versions specified by symbol names take precedence over version scripts. See parseSymbolVersion().

lld/trunk/ELF/SymbolTable.cpp

if (Syms.empty()) {
     if (Config->NoUndefinedVersion)
       error("version script assignment of '" + VersionName + "' to symbol '" +
             Ver.Name + "' failed: symbol not defined");
     return;
   }
 
   // Assign the version.
   for (SymbolBody *B : Syms) {
+    // Skip symbols containing version info because symbol versions
+    // specified by symbol names take precedence over version scripts.
+    // See parseSymbolVersion().
+    if (B->getName().find('@') != StringRef::npos)
+      continue;
+
     Symbol *Sym = B->symbol();
     if (Sym->VersionId != Config->DefaultSymbolVersion)
       warn("duplicate symbol '" + Ver.Name + "' in version script");
     Sym->VersionId = VersionId;
   }
 }
 
 template <class ELFT>
 void SymbolTable<ELFT>::assignWildcardVersion(SymbolVersion Ver,
                                               uint16_t VersionId) {
   if (!Ver.HasWildcard)
     return;
 
   // Exact matching takes precendence over fuzzy matching,
   // so we set a version to a symbol only if no version has been assigned
   // to the symbol. This behavior is compatible with GNU.
   for (SymbolBody *B : findAllByVersion(Ver))
     if (B->symbol()->VersionId == Config->DefaultSymbolVersion)
       B->symbol()->VersionId = VersionId;
 }
 
 // This function processes version scripts by updating VersionId
 // member of symbols.
 template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
-  // Symbol themselves might know their versions because symbols
-  // can contain versions in the form of <name>@<version>.
-  // Let them parse and update their names to exclude version suffix.
-  for (Symbol *Sym : SymVector)
-    Sym->body()->parseSymbolVersion();
-
   // Handle edge cases first.
   handleAnonymousVersion();
 
-  if (Config->VersionDefinitions.empty())
-    return;
 
   // Now we have version definitions, so we need to set version ids to symbols.
   // Each version definition has a glob pattern, and all symbols that match
   // with the pattern get that version.
 
   // First, we assign versions to exact matching symbols,
   // i.e. version definitions not containing any glob meta-characters.
   for (VersionDefinition &V : Config->VersionDefinitions)
     for (SymbolVersion &Ver : V.Globals)
       assignExactVersion(Ver, V.Id, V.Name);
 
   // Next, we assign versions to fuzzy matching symbols,
   // i.e. version definitions containing glob meta-characters.
   // Note that because the last match takes precedence over previous matches,
   // we iterate over the definitions in the reverse order.
   for (VersionDefinition &V : llvm::reverse(Config->VersionDefinitions))
     for (SymbolVersion &Ver : V.Globals)
       assignWildcardVersion(Ver, V.Id);
+
+  // Symbol themselves might know their versions because symbols
+  // can contain versions in the form of <name>@<version>.
+  // Let them parse and update their names to exclude version suffix.
+  for (Symbol *Sym : SymVector)
+    Sym->body()->parseSymbolVersion();
 }
 
 template class elf::SymbolTable<ELF32LE>;
 template class elf::SymbolTable<ELF32BE>;
 template class elf::SymbolTable<ELF64LE>;
 template class elf::SymbolTable<ELF64BE>;

lld/trunk/test/ELF/version-script-symver2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "VER1 { global: foo; local: *; }; VER2 { global: foo; }; VER3 { global: foo; };" > %t.map
+# RUN: ld.lld -shared %t.o --version-script %t.map -o %t.so --fatal-warnings
+# RUN: llvm-readobj -V %t.so | FileCheck %s
+
+# CHECK:      Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Version: 0
+# CHECK-NEXT:     Name: @
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Version: 3
+# CHECK-NEXT:     Name: foo@@VER2
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Version: 2
+# CHECK-NEXT:     Name: foo@VER1
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.global bar
+bar:
+.symver bar, foo@VER1
+
+.global zed
+zed:
+.symver zed, foo@@VER2