Given preference to @@ symbols

[• rafael]

Summary

This is an alternative to https://reviews.llvm.org/D35595.

It is a lot more verbose, but it is conceptually simpler: we prefer symbols with a default version (with @@ in the name).

There is hope that we can transition the few users of @@ to @@@:

https://reviews.freebsd.org/D11653#

in which case most of the complication in here can be reverted.

[grimar]

LGTM with a nit

ELF/SymbolTable.cpp

339

May be move this code to separate static function to incapsulate this hack ?

[ruiu]

LGTM

[smeenai]

r308492

ELF/SymbolTable.cpp

static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {
   if (VB == STV_DEFAULT)
     return VA;
   return std::min(VA, VB);
 }
 
 // Find an existing symbol or create and insert a new one.
 template <class ELFT>
 std::pair<Symbol *, bool> SymbolTable<ELFT>::insert(StringRef Name) {
+  // <name>@@<version> means the symbol is the default version. In that
+  // case symbol <name> must exist and <name>@@<version> will be used to
+  // resolve references to <name>.
+  size_t Pos = Name.find("@@");
+  if (Pos != StringRef::npos)
+    Name = Name.take_front(Pos);
+
   auto P = Symtab.insert(
       {CachedHashStringRef(Name), SymIndex((int)SymVector.size(), false)});
   SymIndex &V = P.first->second;
   bool IsNew = P.second;
 
   if (V.Idx == -1) {
     IsNew = true;
     V = SymIndex((int)SymVector.size(), true);
else if (InputFile *F = L->fetch())
       addFile(F);
   }
   return S;
 }
 
 // We have a new defined symbol with the specified binding. Return 1 if the new
 // symbol should win, -1 if the new symbol should lose, or 0 if both symbols are
 // strong defined symbols.
-static int compareDefined(Symbol *S, bool WasInserted, uint8_t Binding) {
+static int compareDefined(Symbol *S, bool WasInserted, uint8_t Binding,
+                          StringRef Name) {
   if (WasInserted)
     return 1;
   SymbolBody *Body = S->body();
   if (!Body->isInCurrentDSO())
     return 1;
+
+  // Using .symver foo,foo@@VER unfortunately creates two symbols: foo and
+  // foo@@VER. We want to effectively ignore foo, so give precedence to
+  // foo@@VER.
+  // FIXME: If users can transition to using
+  // .symver foo,foo@@@VER
+  // we can delete this hack.
+  if (Name.find("@@") != StringRef::npos &&
+      S->body()->getName().find("@@") == StringRef::npos)
+    return 1;
+  if (Name.find("@@") == StringRef::npos &&
+      S->body()->getName().find("@@") != StringRef::npos)
+    return -1;
+
   if (Binding == STB_WEAK)
     return -1;
   if (S->isWeak())
     return 1;
   return 0;
 }
 
 // We have a new non-common defined symbol with the specified binding. Return 1
 // if the new symbol should win, -1 if the new symbol should lose, or 0 if there
 // is a conflict. If the new symbol wins, also update the binding.
 template <typename ELFT>
 static int compareDefinedNonCommon(Symbol *S, bool WasInserted, uint8_t Binding,
-                                   bool IsAbsolute, typename ELFT::uint Value) {
-  if (int Cmp = compareDefined(S, WasInserted, Binding)) {
+                                   bool IsAbsolute, typename ELFT::uint Value,
+                                   StringRef Name) {
+  if (int Cmp = compareDefined(S, WasInserted, Binding, Name)) {
     if (Cmp > 0)
       S->Binding = Binding;
     return Cmp;
   }
   SymbolBody *B = S->body();
   if (isa<DefinedCommon>(B)) {
     // Non-common symbols take precedence over common symbols.
     if (Config->WarnCommon)

 Symbol *SymbolTable<ELFT>::addCommon(StringRef N, uint64_t Size,
                                      uint32_t Alignment, uint8_t Binding,
                                      uint8_t StOther, uint8_t Type,
                                      InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N, Type, getVisibility(StOther),
                                     /*CanOmitFromDynSym*/ false, File);
-  int Cmp = compareDefined(S, WasInserted, Binding);
+  int Cmp = compareDefined(S, WasInserted, Binding, N);
   if (Cmp > 0) {
     S->Binding = Binding;
     replaceBody<DefinedCommon>(S, N, Size, Alignment, StOther, Type, File);
   } else if (Cmp == 0) {
     auto *C = dyn_cast<DefinedCommon>(S->body());
     if (!C) {
       // Non-common symbols take precedence over common symbols.
       if (Config->WarnCommon)
Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t StOther,
                                       uint8_t Type, uint64_t Value,
                                       uint64_t Size, uint8_t Binding,
                                       SectionBase *Section, InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, Type, getVisibility(StOther),
                                     /*CanOmitFromDynSym*/ false, File);
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Binding,
-                                          Section == nullptr, Value);
+                                          Section == nullptr, Value, Name);
   if (Cmp > 0)
     replaceBody<DefinedRegular>(S, Name, /*IsLocal=*/false, StOther, Type,
                                 Value, Size, Section, File);
   else if (Cmp == 0)
     reportDuplicate<ELFT>(S->body(),
                           dyn_cast_or_null<InputSectionBase>(Section), Value);
   return S;
 }

 Symbol *SymbolTable<ELFT>::addBitcode(StringRef Name, uint8_t Binding,
                                       uint8_t StOther, uint8_t Type,
                                       bool CanOmitFromDynSym, BitcodeFile *F) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(Name, Type, getVisibility(StOther), CanOmitFromDynSym, F);
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Binding,
-                                          /*IsAbs*/ false, /*Value*/ 0);
+                                          /*IsAbs*/ false, /*Value*/ 0, Name);
   if (Cmp > 0)
     replaceBody<DefinedRegular>(S, Name, /*IsLocal=*/false, StOther, Type, 0, 0,
                                 nullptr, F);
   else if (Cmp == 0)
     reportDuplicate(S->body(), F);
   return S;
 }
 
void SymbolTable<ELFT>::assignWildcardVersion(SymbolVersion Ver,
   // 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;
 }
 
-static bool isDefaultVersion(SymbolBody *B) {
-  return B->isInCurrentDSO() && B->getName().find("@@") != StringRef::npos;
-}
-
 // 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) {
-    SymbolBody *Body = Sym->body();
-    bool IsDefault = isDefaultVersion(Body);
-    Body->parseSymbolVersion();
-
-    if (!IsDefault)
-      continue;
-
-    // <name>@@<version> means the symbol is the default version. If that's the
-    // case, the symbol is not used only to resolve <name> of version <version>
-    // but also undefined unversioned symbols with name <name>.
-    SymbolBody *S = find(Body->getName());
-    if (S && S->isUndefined())
-      S->copy(Body);
-  }
-
   // Handle edge cases first.
   handleAnonymousVersion();
 
   // 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,

test/ELF/Inputs/symver-archive1.s

+.text
+.globl x
+.type x, @function
+x:
+
+.symver x, xx@@VER

test/ELF/Inputs/symver-archive2.s

+call xx@PLT

test/ELF/symver-archive.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1
+# RUN: rm -f %t.a
+# RUN: llvm-ar rcs %t.a %t1
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symver-archive1.s -o %t2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symver-archive2.s -o %t3.o
+# RUN: ld.lld -o %t.out %t2.o %t3.o %t.a
+
+.text
+.globl x
+.type x, @function
+x:
+
+.globl xx
+xx = x