[ELF] SymbolTable::insert: keep @@ in the name

* Avoid the name truncation quirk in SymbolTable::insert: the truncated name will be replaced by @@ again.
* Allow foo and foo@@v1 in different files to be diagnosed as duplicate definition error (GNU ld behavior)
* Avoid potential redundant strlen on symbol name due to StringRefZ in ObjFile<ELFT>::initializeSymbols

lld/ELF/InputFiles.cpp

@@ -1103,7 +1103,6 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
     uint8_t type = eSym.getType();
     uint64_t value = eSym.st_value;
     uint64_t size = eSym.st_size;
-    StringRefZ name = this->stringTable.data() + eSym.st_name;
 
     if (eSym.st_shndx == SHN_UNDEF) {
       undefineds.push_back(i);
@@ -1111,9 +1110,10 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
     }
 
     Symbol *sym = this->symbols[i];
+    const StringRef name = sym->getName();
     if (eSym.st_shndx == SHN_COMMON) {
       if (value == 0 || value >= UINT32_MAX)
-        fatal(toString(this) + ": common symbol '" + StringRef(name.data) +
+        fatal(toString(this) + ": common symbol '" + name +
               "' has invalid alignment: " + Twine(value));
       sym->resolve(
           CommonSymbol{this, name, binding, stOther, type, value, size});
@@ -1809,7 +1809,8 @@ template <class ELFT> void LazyObjFile::parse() {
     // Get existing symbols or insert placeholder symbols.
     for (size_t i = firstGlobal, end = eSyms.size(); i != end; ++i)
       if (eSyms[i].st_shndx != SHN_UNDEF)
-        this->symbols[i] = symtab->insert(CHECK(eSyms[i].getName(strtab), this));
+        this->symbols[i] =
+            symtab->insert(CHECK(eSyms[i].getName(strtab), this));
 
     // Replace existing symbols with LazyObject symbols.
     //

lld/ELF/SymbolTable.cpp

@@ -64,16 +64,21 @@ Symbol *SymbolTable::insert(StringRef name) {
   // Since this is a hot path, the following string search code is
   // optimized for speed. StringRef::find(char) is much faster than
   // StringRef::find(StringRef).
+  StringRef stem = name;
   size_t pos = name.find('@');
   if (pos != StringRef::npos && pos + 1 < name.size() && name[pos + 1] == '@')
-    name = name.take_front(pos);
+    stem = name.take_front(pos);
 
-  auto p = symMap.insert({CachedHashStringRef(name), (int)symVector.size()});
+  auto p = symMap.insert({CachedHashStringRef(stem), (int)symVector.size()});
   int &symIndex = p.first->second;
   bool isNew = p.second;
 
-  if (!isNew)
-    return symVector[symIndex];
+  if (!isNew) {
+    Symbol *sym = symVector[symIndex];
+    if (stem.size() != name.size())
+      sym->setName(name);
+    return sym;
+  }
 
   Symbol *sym = reinterpret_cast<Symbol *>(make<SymbolUnion>());
   symVector.push_back(sym);

lld/ELF/Symbols.cpp

@@ -561,22 +561,6 @@ void Symbol::resolveUndefined(const Undefined &other) {
   }
 }
 
-// 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.
-static int compareVersion(StringRef a, StringRef b) {
-  bool x = a.contains("@@");
-  bool y = b.contains("@@");
-  if (!x && y)
-    return 1;
-  if (x && !y)
-    return -1;
-  return 0;
-}
-
 // Compare two symbols. Return 1 if the new symbol should win, -1 if
 // the new symbol should lose, or 0 if there is a conflict.
 int Symbol::compare(const Symbol *other) const {
@@ -585,8 +569,16 @@ int Symbol::compare(const Symbol *other) const {
   if (!isDefined() && !isCommon())
     return 1;
 
-  if (int cmp = compareVersion(getName(), other->getName()))
-    return cmp;
+  // .symver foo,foo@@VER unfortunately creates two defined symbols: foo and
+  // foo@@VER. In GNU ld, if foo and foo@@VER are in the same file, foo is
+  // ignored. In our implementation, when this is foo, this->getName() may still
+  // contain @@, return 1 in this case as well.
+  if (file == other->file) {
+    if (other->getName().contains("@@"))
+      return 1;
+    if (getName().contains("@@"))
+      return -1;
+  }
 
   if (other->isWeak())
     return -1;

lld/test/ELF/symver-archive.s

@@ -6,6 +6,15 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/symver-archive2.s -o %t3.o
 # RUN: ld.lld -o /dev/null %t2.o %t3.o %t.a
 
+# RUN: not ld.lld -o /dev/null %t2.o %t3.o %t1 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR:      error: duplicate symbol: x
+
+## If defined xx and xx@@VER are in different files, report a duplicate definition error like GNU ld.
+# ERR:      error: duplicate symbol: xx
+# ERR-NEXT: >>> defined at {{.*}}2.o:(.text+0x0)
+# ERR-NEXT: >>> defined at {{.*}}1:(.text+0x0)
+
 .text
 .globl x
 .type x, @function