[ELF] - Resolve references properly when using .symver directive (2)

[grimar]

Summary

This is variation of D33680.

Previously LLD was unable to link following:
(failed with undefined symbol bar)

Version script:

SOME_VERSION { global: *; };

.global _start
.global bar
.symver _start, bar@@SOME_VERSION
_start:
  jmp bar

Manual has next description:

.symver name, name2@@nodename
In this case, the symbol name must exist and be defined within the file being assembled. It is similar to name2@nodename.
The difference is name2@@nodename will also be used to resolve references to name2 by the linker
https://sourceware.org/binutils/docs/as/Symver.html

Patch implements that. If we have name@@ver symbol and name is undefined, name@@ver is used to resolve references to name.
If name is defined then multiple definition error is emited, that is consistent with what bfd do.

[ruiu]

I don't think we should return a bool value from a parser function unless it really improves performance. Essentially it is a speed hack, so it needs to be proven that it is effective.

[grimar]

So looks we agreed to go with D33680 version for now.

ELF/SymbolTable.cpp

 }
 
 // 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 their names.
-  if (!Config->VersionDefinitions.empty())
-    for (Symbol *Sym : SymVector)
-      Sym->body()->parseSymbolVersion();
+  // Below we resolve references to symbols that have default version. We use
+  // additional temp vector because addRegular() call may change SymVector.
+  std::vector<DefinedRegular *> DefaultV;
+    if (Sym->body()->parseSymbolVersion())
+      DefaultV.push_back(cast<DefinedRegular>(Sym->body()));
+
+  // <name>@@<version> means symbol has 'default' version. We should use it to
+  // resolve references to <name>.
+  for (DefinedRegular *D : DefaultV)
+    if (find(D->getName()))
+      addRegular(D->getName(), D->StOther, D->Type, D->Value, D->Size,
+                 D->symbol()->Binding, D->Section, D->File);
 
   // Handle edge cases first.
   handleAnonymousVersion();
 
   if (Config->VersionDefinitions.empty())
     return;
 
   // Now we have version definitions, so we need to set version ids to symbols.

ELF/Symbols.h

bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isInCurrentDSO() const { return !isUndefined() && !isShared(); }
   bool isLocal() const { return IsLocal; }
   bool isPreemptible() const;
   StringRef getName() const { return Name; }
   uint8_t getVisibility() const { return StOther & 0x3; }
-  void parseSymbolVersion();
+  bool parseSymbolVersion();
 
   bool isInGot() const { return GotIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
 
   uint64_t getVA(int64_t Addend = 0) const;
 
   uint64_t getGotOffset() const;
   uint64_t getGotVA() const;

ELF/Symbols.cpp

if (isa<DefinedCommon>(this)) {
     return nullptr;
   }
 
   return nullptr;
 }
 
 // If a symbol name contains '@', the characters after that is
 // a symbol version name. This function parses that.
-void SymbolBody::parseSymbolVersion() {
+bool SymbolBody::parseSymbolVersion() {
   StringRef S = getName();
   size_t Pos = S.find('@');
   if (Pos == 0 || Pos == StringRef::npos)
-    return;
+    return false;
   StringRef Verstr = S.substr(Pos + 1);
   if (Verstr.empty())
-    return;
+    return false;
 
   // Truncate the symbol name so that it doesn't include the version string.
   Name = {S.data(), Pos};
 
   // If this is not in this DSO, it is not a definition.
   if (!isInCurrentDSO())
-    return;
+    return false;
 
   // '@@' in a symbol name means the default version.
   // It is usually the most recent one.
   bool IsDefault = (Verstr[0] == '@');
   if (IsDefault)
     Verstr = Verstr.substr(1);
 
   for (VersionDefinition &Ver : Config->VersionDefinitions) {
     if (Ver.Name != Verstr)
       continue;
 
     if (IsDefault)
       symbol()->VersionId = Ver.Id;
     else
       symbol()->VersionId = Ver.Id | VERSYM_HIDDEN;
-    return;
+    return IsDefault;
   }
 
   // It is an error if the specified version is not defined.
-  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
+  if (Config->Shared)
+    error(toString(File) + ": symbol " + S + " has undefined version " +
+          Verstr);
+  return IsDefault;
 }
 
 Defined::Defined(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
                  uint8_t Type)
     : SymbolBody(K, Name, IsLocal, StOther, Type) {}
 
 template <class ELFT> bool DefinedRegular::isMipsPIC() const {
   typedef typename ELFT::Ehdr Elf_Ehdr;

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

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
+# CHECK:      error: duplicate symbol: bar
+# CHECK-NEXT:   >>> defined at {{.*}}.o
+# CHECK-NEXT:   >>> defined at {{.*}}.o
+
+.global _start
+.global bar
+.symver _start, bar@@VERSION
+_start:
+  jmp bar
+
+bar:

test/ELF/version-script-symver.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+
+.global _start
+.global bar
+.symver _start, bar@@VERSION
+_start:
+  jmp bar