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@v1 The symbol resolution rules for versioned symbols are: * foo@@v1 (default version) resolves both undefined foo and foo@v1 * foo@v1 (non-default version) resolves undefined foo@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@v1. We don't do the same for object files, so foo@@v1 defined in one object file incorrectly does not resolve a foo@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@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@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@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@v1 and foo@@v1 should be merged, so redirect foo@v1 to foo@@v1. + map.try_emplace(sym, maybeDefault); + // If both foo@v1 and foo@@v1 are defined and non-weak, report a duplicate + // definition error. + maybeDefault->resolve(*sym); + // Eliminate foo@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@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@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@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@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@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@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@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@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@plt> +# W5DIS-NEXT: callq 0x1350 <foo@plt> # W5DIS-COUNT-3: int3 -# W5DIS-NEXT: callq 0x1390 <foo@plt> +# W5DIS-NEXT: callq 0x1350 <foo@plt> #--- ver v1 {}; _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
