[lld] 2bdad16 - [ELF] SymbolTable::insert: keep @@ in the name
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 15 15:19:40 PST 2021
Author: Fangrui Song
Date: 2021-12-15T15:19:35-08:00
New Revision: 2bdad16303f4f0d716ebc2c12beb24ea50649639
URL: https://github.com/llvm/llvm-project/commit/2bdad16303f4f0d716ebc2c12beb24ea50649639
DIFF: https://github.com/llvm/llvm-project/commit/2bdad16303f4f0d716ebc2c12beb24ea50649639.diff
LOG: [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
Added:
Modified:
lld/ELF/InputFiles.cpp
lld/ELF/SymbolTable.cpp
lld/ELF/Symbols.cpp
lld/test/ELF/symver-archive.s
Removed:
################################################################################
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 5319db195a4b..4cfd51905ac5 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/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.
//
diff --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index 406dca295650..9bb2e7ddaf51 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/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);
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index 01a3598f8d60..93609fc1f283 100644
--- a/lld/ELF/Symbols.cpp
+++ b/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;
diff --git a/lld/test/ELF/symver-archive.s b/lld/test/ELF/symver-archive.s
index 3e22dd24b66d..c823be4769e1 100644
--- a/lld/test/ELF/symver-archive.s
+++ b/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
diff erent 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
More information about the llvm-commits
mailing list