Use symbols with the default version to resolve unversioned symbols.

[ruiu]

Summary

This patch is made based on https://reviews.llvm.org/D33680.

Symbols can contain symbol versions in themselves. If a symbol name
is in the form of "foo@bar", foo's version is "bar".

There's one special version -- the default version. If a symbol is in
the form of "foo@@bar", it is used not only to resolve "foo" in
version "bar" but also any unversioned "foo". Thus, it is called the
default.

This patch implements the behavior.

Fixes https://bugs.llvm.org/show_bug.cgi?id=33383.

[silvas]

If it is based on D33680 I presume it fixes https://bugs.llvm.org/show_bug.cgi?id=28414 also; would be good to mention that in the description too.

[grimar]

Thanks for taking a look on that ! Really.

Honestly I think D33680 is more clear for my eye. Let me do one more update for it,
I have one change in mind I did not include ealier, we can discuss which way fits better after that.

My conserns about this patch are inline.

lld/ELF/SymbolTable.cpp

32

I would inline it.

755

So you are mutating arguments here. Generally you are asking to avoid that in reviews.
And I think I agree with that strategy.
Mutating arguments really made me be confusing, please see next comment.

780

So Version can have valid 0 value it seems ?

But below you do check for 0:

if (Ver != 0)
  Sym->VersionId = IsDefault ? Ver : (Ver | VERSYM_HIDDEN);

That become visible and confusing for me mostly because of mutating arguments.
(I was a bit lost in what this method do in general).

[ruiu]

lld/ELF/SymbolTable.cpp

32

I wouldn't -- I tried both and it seems this is slightly clearner.

755

Well, I tried to use a tuple or a struct to return a value, but turned out this seems the best option I have.

780

I could fix it.

[grimar]

lld/ELF/SymbolTable.cpp

755

I have one idea in mind for D33680, I would like to show you, sorry again for delay here. I'll polish and update it tomorrow to demonstrate and we can choose which approach fits better then..

[ruiu]

lld/ELF/SymbolTable.cpp

780

Take your time. No reason to rush.

lld/ELF/SymbolTable.h

public:
   SymbolBody *find(StringRef Name);
   SymbolBody *findInCurrentDSO(StringRef Name);
 
   void trace(StringRef Name);
 
 private:
   std::vector<SymbolBody *> findByVersion(SymbolVersion Ver);
   std::vector<SymbolBody *> findAllByVersion(SymbolVersion Ver);
+  void scanSymbolVersions();
 
   llvm::StringMap<std::vector<SymbolBody *>> &getDemangledSyms();
   void handleAnonymousVersion();
   void assignExactVersion(SymbolVersion Ver, uint16_t VersionId,
                           StringRef VersionName);
   void assignWildcardVersion(SymbolVersion Ver, uint16_t VersionId);
 
   struct SymIndex {

lld/ELF/SymbolTable.cpp

 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
 
 using namespace lld;
 using namespace lld::elf;
 
+static void copy(Symbol *A, Symbol *B) {
+  memcpy(A->Body.buffer, B->Body.buffer, sizeof(Symbol::Body));
+}
+
 // All input object files must be for the same architecture
 // (e.g. it does not make sense to link x86 object files with
 // MIPS object files.) This function checks for that error.
 template <class ELFT> static bool isCompatible(InputFile *F) {
   if (!isa<ELFFileBase<ELFT>>(F) && !isa<BitcodeFile>(F))
     return true;
 
   if (F->EKind == Config->EKind && F->EMachine == Config->EMachine) {
template <class ELFT> void SymbolTable<ELFT>::applySymbolRenames() {
   for (auto &KV : Config->RenamedSymbols) {
     Symbol *Sym = KV.first;
     Symbol *Rename = KV.second.Target;
     Sym->Binding = KV.second.OrigBinding;
 
     // We rename symbols by replacing the old symbol's SymbolBody with the new
     // symbol's SymbolBody. This causes all SymbolBody pointers referring to the
     // old symbol to instead refer to the new symbol.
-    memcpy(Sym->Body.buffer, Rename->Body.buffer, sizeof(Sym->Body));
+    copy(Sym, Rename);
   }
 }
 
 static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VA == STV_DEFAULT)
     return VB;
   if (VB == STV_DEFAULT)
     return VA;
void SymbolTable<ELFT>::assignWildcardVersion(SymbolVersion Ver,
   for (SymbolBody *B : Syms)
     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 their names.
   if (!Config->VersionDefinitions.empty())
-    for (Symbol *Sym : SymVector)
-      Sym->body()->parseSymbolVersion();
+    scanSymbolVersions();
 
   // Handle edge cases first.
   handleAnonymousVersion();
 
   if (Config->VersionDefinitions.empty())
     return;
 
   // Now we have version definitions, so we need to set version ids to symbols.
template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
   // 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);
 }
 
+// Parse a symbol name as a versioned symbol. It recognizes
+// <name>@<version> or <name>@@<version>.
+static void parseSymbolVersion(const SymbolBody &B, uint16_t &Version,
+                               bool &IsDefault, StringRef &NewName) {
+  StringRef S = B.getName();
+  size_t Pos = S.find('@');
+  if (Pos == 0 || Pos == StringRef::npos)
+    return;
+  StringRef Verstr = S.substr(Pos + 1);
+  if (Verstr.empty())
+    return;
+
+  // Truncate the symbol name so that it doesn't include the version string.
+  NewName = S.substr(0, Pos);
+
+  // If this is not in this DSO, it is not a definition.
+  if (!B.isInCurrentDSO())
+    return;
+
+  // '@@' in a symbol name means the default version.
+  // It is usually the most recent one.
+  IsDefault = (Verstr[0] == '@');
+  if (IsDefault)
+    Verstr = Verstr.substr(1);
+
+  for (VersionDefinition &Ver : Config->VersionDefinitions) {
+    if (Ver.Name == Verstr) {
+      Version = Ver.Id;
+      return;
+    }
+  }
+
+  // It is an error if the specified version is not defined.
+  error(toString(B.File) + ": symbol " + S + " has undefined version " +
+        Verstr);
+}
+
+// There is a way to specify sybmol versions besides version scripts.
+// If a symbol name is in the form of <name>@<version> or <name>@@<version>,
+// the strings after '@' specifies versions.
+template <class ELFT> void SymbolTable<ELFT>::scanSymbolVersions() {
+  for (Symbol *Sym : SymVector) {
+    SymbolBody *B = Sym->body();
+    uint16_t Ver = 0;
+    bool IsDefault = false;
+    StringRef NewName;
+
+    // Parse the symbol name.
+    parseSymbolVersion(*B, Ver, IsDefault, NewName);
+    if (!NewName.empty())
+      B->setName(NewName);
+    if (Ver != 0)
+      Sym->VersionId = IsDefault ? Ver : (Ver | VERSYM_HIDDEN);
+
+    // If versioned symbols are default ones (which contains "@@" as opposed
+    // to "@"), they are used to resolved remaining unversioned symbols.
+    if (IsDefault)
+      if (auto *U = dyn_cast_or_null<Undefined>(find(NewName)))
+        copy(U->symbol(), Sym);
+  }
+}
+
 template class elf::SymbolTable<ELF32LE>;
 template class elf::SymbolTable<ELF32BE>;
 template class elf::SymbolTable<ELF64LE>;
 template class elf::SymbolTable<ELF64BE>;

lld/ELF/Symbols.h

public:
   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; }
+  void setName(StringRef S) { Name = {S.data(), S.size()}; }
   uint8_t getVisibility() const { return StOther & 0x3; }
-  void 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;

lld/ELF/Symbols.cpp

if (isa<DefinedCommon>(this)) {
     if (Config->DefineCommon)
       return InX::Common->getParent();
     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() {
-  StringRef S = getName();
-  size_t Pos = S.find('@');
-  if (Pos == 0 || Pos == StringRef::npos)
-    return;
-  StringRef Verstr = S.substr(Pos + 1);
-  if (Verstr.empty())
-    return;
-
-  // 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;
-
-  // '@@' 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;
-  }
-
-  // It is an error if the specified version is not defined.
-  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
-}
-
 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;
   if (!Section || !isFunc())
     return false;

lld/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:

lld/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