[llvm-branch-commits] [lld] 941e933 - [ELF] Make foo@@v1 resolve undefined foo at v1
Fangrui Song via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Dec 1 12:09:15 PST 2020
Author: Fangrui Song
Date: 2020-12-01T08:54:01-08:00
New Revision: 941e9336d092f0ccef35e0f425d97f7def5ed1b0
URL: https://github.com/llvm/llvm-project/commit/941e9336d092f0ccef35e0f425d97f7def5ed1b0
DIFF: https://github.com/llvm/llvm-project/commit/941e9336d092f0ccef35e0f425d97f7def5ed1b0.diff
LOG: [ELF] Make foo@@v1 resolve undefined foo at v1
The symbol resolution rules for versioned symbols are:
* foo@@v1 (default version) resolves both undefined foo and foo at v1
* foo at v1 (non-default version) resolves undefined foo at v1
Note, foo@@v1 must be defined (the assembler errors if attempting to
create an undefined foo@@v1).
For defined foo@@v1 in a shared object, we call `SymbolTable::addSymbol` twice,
one for foo and the other for foo at v1. We don't do the same for object files, so
foo@@v1 defined in one object file incorrectly does not resolve a foo at v1
reference in another object file.
This patch fixes the issue by reusing the --wrap code to redirect symbols in
object files. This has to be done after processing input files because
foo and foo at v1 are two separate symbols if we haven't seen foo@@v1.
Add a helper `Symbol::getVersionSuffix` to retrieve the optional trailing
`@...` or `@@...` from the possibly truncated symbol name.
Depends on D92258
Reviewed By: jhenderson
Differential Revision: https://reviews.llvm.org/D92259
Added:
Modified:
lld/ELF/Driver.cpp
lld/ELF/Symbols.cpp
lld/ELF/Symbols.h
lld/test/ELF/symver.s
Removed:
################################################################################
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index aa6fed652a93..860c74fa0d20 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1919,17 +1919,46 @@ static std::vector<WrappedSymbol> addWrappedSymbols(opt::InputArgList &args) {
return v;
}
-// Do renaming for -wrap by updating pointers to symbols.
+// Do renaming for -wrap and foo at v1 by updating pointers to symbols.
//
// When this function is executed, only InputFiles and symbol table
// contain pointers to symbol objects. We visit them to replace pointers,
// so that wrapped symbols are swapped as instructed by the command line.
-static void wrapSymbols(ArrayRef<WrappedSymbol> wrapped) {
+static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
+ llvm::TimeTraceScope timeScope("Redirect symbols");
DenseMap<Symbol *, Symbol *> map;
for (const WrappedSymbol &w : wrapped) {
map[w.sym] = w.wrap;
map[w.real] = w.sym;
}
+ for (Symbol *sym : symtab->symbols()) {
+ // Enumerate symbols with a non-default version (foo at v1).
+ StringRef name = sym->getName();
+ const char *suffix1 = sym->getVersionSuffix();
+ if (suffix1[0] != '@' || suffix1[1] == '@')
+ continue;
+
+ // Check whether the default version foo@@v1 exists. If it exists, the
+ // symbol can be found by the name "foo" in the symbol table.
+ Symbol *maybeDefault = symtab->find(name);
+ if (!maybeDefault)
+ continue;
+ const char *suffix2 = maybeDefault->getVersionSuffix();
+ if (suffix2[0] != '@' || suffix2[1] != '@' ||
+ strcmp(suffix1 + 1, suffix2 + 2) != 0)
+ continue;
+
+ // foo at v1 and foo@@v1 should be merged, so redirect foo at v1 to foo@@v1.
+ map.try_emplace(sym, maybeDefault);
+ // If both foo at v1 and foo@@v1 are defined and non-weak, report a duplicate
+ // definition error.
+ maybeDefault->resolve(*sym);
+ // Eliminate foo at v1 from the symbol table.
+ sym->symbolKind = Symbol::PlaceholderKind;
+ }
+
+ if (map.empty())
+ return;
// Update pointers in input files.
parallelForEach(objectFiles, [&](InputFile *file) {
@@ -2158,9 +2187,8 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
!config->thinLTOModulesToCompile.empty())
return;
- // Apply symbol renames for -wrap.
- if (!wrapped.empty())
- wrapSymbols(wrapped);
+ // Apply symbol renames for -wrap and combine foo at v1 and foo@@v1.
+ redirectSymbols(wrapped);
{
llvm::TimeTraceScope timeScope("Aggregate sections");
diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp
index f38d9fab4500..a2153da2e8d8 100644
--- a/lld/ELF/Symbols.cpp
+++ b/lld/ELF/Symbols.cpp
@@ -37,11 +37,9 @@ std::string lld::toString(const elf::Symbol &sym) {
StringRef name = sym.getName();
std::string ret = demangle(name);
- // If sym has a non-default version, its name may have been truncated at '@'
- // by Symbol::parseSymbolVersion(). Add the trailing part. This check is safe
- // because every symbol name ends with '\0'.
- if (name.data()[name.size()] == '@')
- ret += name.data() + name.size();
+ const char *suffix = sym.getVersionSuffix();
+ if (*suffix == '@')
+ ret += suffix;
return ret;
}
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index a8d056a32f98..48428c24f9c8 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -178,6 +178,15 @@ class Symbol {
void parseSymbolVersion();
+ // Get the NUL-terminated version suffix ("", "@...", or "@@...").
+ //
+ // For @@, the name has been truncated by insert(). For @, the name has been
+ // truncated by Symbol::parseSymbolVersion().
+ const char *getVersionSuffix() const {
+ assert(nameSize != (uint32_t)-1);
+ return nameData + nameSize;
+ }
+
bool isInGot() const { return gotIndex != -1U; }
bool isInPlt() const { return pltIndex != -1U; }
diff --git a/lld/test/ELF/symver.s b/lld/test/ELF/symver.s
index 7495805bfe4d..2858e835622a 100644
--- a/lld/test/ELF/symver.s
+++ b/lld/test/ELF/symver.s
@@ -15,29 +15,33 @@
# RUN: ld.lld -shared --soname=def1.so --version-script=%t/ver %t/def1.o -o %t/def1.so
# RUN: ld.lld -shared --soname=hid1.so --version-script=%t/ver %t/hid1.o -o %t/hid1.so
-## TODO Report a duplicate definition error for foo at v1 and foo@@v1.
-# RUN: ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null
-
+## Report a duplicate definition error for foo at v1 and foo@@v1.
+# RUN: not ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null 2>&1 | \
+# RUN: FileCheck %s --check-prefix=DUP
# RUN: ld.lld -shared --version-script=%t/ver %t/def1w.o %t/hid1.o -o /dev/null
# RUN: ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1w.o -o /dev/null
# RUN: ld.lld -shared --version-script=%t/ver %t/def1w.o %t/hid1w.o -o /dev/null
-## TODO foo@@v1 should resolve undefined foo at v1.
-# RUN: not ld.lld -shared --version-script=%t/ver %t/ref1p.o %t/def1.o -o /dev/null
+# DUP: error: duplicate symbol: foo@@v1
+# DUP-NEXT: >>> defined at {{.*}}/def1{{w?}}.o:(.text+0x0)
+# DUP-NEXT: >>> defined at {{.*}}/hid1.o:(.text+0x0)
+
+## Protected undefined foo at v1 makes the output symbol protected.
+# RUN: ld.lld -shared --version-script=%t/ver %t/ref1p.o %t/def1.o -o %t.protected
+# RUN: llvm-readelf --dyn-syms %t.protected | FileCheck %s --check-prefix=PROTECTED
+
+# PROTECTED: NOTYPE GLOBAL PROTECTED [[#]] foo@@v1
-## TODO
## foo@@v1 resolves both undefined foo and foo at v1. There is one single definition.
## Note: set soname so that the name string referenced by .gnu.version_d is fixed.
# RUN: ld.lld -shared --soname=t --version-script=%t/ver %t/ref.o %t/ref1.o %t/def1.o -o %t1
# RUN: llvm-readelf -r --dyn-syms %t1 | FileCheck %s
-# CHECK: Relocation section '.rela.plt' at offset {{.*}} contains 2 entries:
+# CHECK: Relocation section '.rela.plt' at offset {{.*}} contains 1 entries:
# CHECK-NEXT: {{.*}} Type {{.*}}
# CHECK-NEXT: {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo@@v1 + 0
-# CHECK-NEXT: {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo + 0
-# CHECK: 1: {{.*}} NOTYPE GLOBAL DEFAULT UND foo{{$}}
-# CHECK-NEXT: 2: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@@v1
+# CHECK: 1: {{.*}} NOTYPE GLOBAL DEFAULT [[#]] foo@@v1
# CHECK-EMPTY:
## foo@@v2 does not resolve undefined foo at v1.
@@ -96,10 +100,9 @@
# RUN: llvm-readobj -r %t.w1 | FileCheck %s --check-prefix=W1REL
# RUN: llvm-objdump -d --no-show-raw-insn %t.w1 | FileCheck %s --check-prefix=W1DIS
-## TODO foo should be foo@@v1.
# W1REL: .rela.plt {
# W1REL-NEXT: R_X86_64_JUMP_SLOT __wrap_foo 0x0
-# W1REL-NEXT: R_X86_64_JUMP_SLOT foo 0x0
+# W1REL-NEXT: R_X86_64_JUMP_SLOT foo@@v1 0x0
# W1REL-NEXT: }
# W1DIS-LABEL: <.text>:
@@ -150,7 +153,6 @@
# W4DIS-COUNT-3: int3
# W4DIS-NEXT: callq {{.*}} <__wrap_foo at plt>
-## TODO The two callq should jump to the same address.
## Note: GNU ld errors "no symbol version section for versioned symbol `__wrap_foo@@v1'".
# RUN: ld.lld -shared --soname=t --version-script=%t/ver --wrap=foo@@v1 %t/ref.o %t/ref1.o %t/def1.o %t/wrap.o -o %t.w5
# RUN: llvm-readobj -r %t.w5 | FileCheck %s --check-prefix=W5REL
@@ -158,13 +160,12 @@
# W5REL: .rela.plt {
# W5REL-NEXT: R_X86_64_JUMP_SLOT foo@@v1 0x0
-# W5REL-NEXT: R_X86_64_JUMP_SLOT foo 0x0
# W5REL-NEXT: }
# W5DIS-LABEL: <.text>:
-# W5DIS-NEXT: callq 0x1380 <foo at plt>
+# W5DIS-NEXT: callq 0x1350 <foo at plt>
# W5DIS-COUNT-3: int3
-# W5DIS-NEXT: callq 0x1390 <foo at plt>
+# W5DIS-NEXT: callq 0x1350 <foo at plt>
#--- ver
v1 {};
More information about the llvm-branch-commits
mailing list