From 3b64052a2572e69355969a59a0c4c8aba4fee887 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Fri, 4 Sep 2020 12:27:40 -0700 Subject: [PATCH] [ORC] Fix some bugs in TPCDynamicLibrarySearchGenerator, use in llvm-jitlink. TPCDynamicLibrarySearchGenerator was generating errors on missing symbols, but that doesn't fit the DefinitionGenerator contract: A symbol that isn't generated by a particular generator should not cause an error. This commit fixes the error by using SymbolLookupFlags::WeaklyReferencedSymbol for all elements of the lookup, and switches llvm-jitlink to use TPCDynamicLibrarySearchGenerator. --- .../Orc/TPCDynamicLibrarySearchGenerator.h | 15 +++++++--- .../ExecutionEngine/Orc/TargetProcessControl.h | 5 +++- .../Orc/TPCDynamicLibrarySearchGenerator.cpp | 32 +++++++++++++++++----- .../ExecutionEngine/Orc/TargetProcessControl.cpp | 6 ++-- llvm/tools/llvm-jitlink/llvm-jitlink.cpp | 12 ++------ 5 files changed, 46 insertions(+), 24 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h b/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h index d35c8ab..7c1b72b 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h @@ -14,6 +14,7 @@ #ifndef LLVM_EXECUTIONENGINE_ORC_TPCDYNAMICLIBRARYSEARCHGENERATOR_H #define LLVM_EXECUTIONENGINE_ORC_TPCDYNAMICLIBRARYSEARCHGENERATOR_H +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ExecutionEngine/Orc/TargetProcessControl.h" namespace llvm { @@ -21,6 +22,8 @@ namespace orc { class TPCDynamicLibrarySearchGenerator : public JITDylib::DefinitionGenerator { public: + using SymbolPredicate = unique_function; + /// Create a DynamicLibrarySearchGenerator that searches for symbols in the /// library with the given handle. /// @@ -28,19 +31,22 @@ public: /// will be searched for. If the predicate is not given then all symbols will /// be searched for. TPCDynamicLibrarySearchGenerator(TargetProcessControl &TPC, - TargetProcessControl::DylibHandle H) - : TPC(TPC), H(H) {} + TargetProcessControl::DylibHandle H, + SymbolPredicate Allow = SymbolPredicate()) + : TPC(TPC), H(H), Allow(std::move(Allow)) {} /// Permanently loads the library at the given path and, on success, returns /// a DynamicLibrarySearchGenerator that will search it for symbol definitions /// in the library. On failure returns the reason the library failed to load. static Expected> - Load(TargetProcessControl &TPC, const char *LibraryPath); + Load(TargetProcessControl &TPC, const char *LibraryPath, + SymbolPredicate Allow = SymbolPredicate()); /// Creates a TPCDynamicLibrarySearchGenerator that searches for symbols in /// the target process. static Expected> - GetForTargetProcess(TargetProcessControl &TPC) { + GetForTargetProcess(TargetProcessControl &TPC, + SymbolPredicate Allow = SymbolPredicate()) { return Load(TPC, nullptr); } @@ -51,6 +57,7 @@ public: private: TargetProcessControl &TPC; TargetProcessControl::DylibHandle H; + SymbolPredicate Allow; }; } // end namespace orc diff --git a/llvm/include/llvm/ExecutionEngine/Orc/TargetProcessControl.h b/llvm/include/llvm/ExecutionEngine/Orc/TargetProcessControl.h index 159b6e8..d334975 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/TargetProcessControl.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/TargetProcessControl.h @@ -149,8 +149,11 @@ public: virtual Expected loadDylib(const char *DylibPath) = 0; /// Search for symbols in the target process. + /// /// The result of the lookup is a 2-dimentional array of target addresses - /// that correspond to the lookup order. + /// that correspond to the lookup order. If a required symbol is not + /// found then this method will return an error. If a weakly referenced + /// symbol is not found then it be assigned a '0' value in the result. virtual Expected lookupSymbols(LookupRequest Request) = 0; protected: diff --git a/llvm/lib/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.cpp index 18de5b6..d85f3c3 100644 --- a/llvm/lib/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.cpp +++ b/llvm/lib/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.cpp @@ -13,12 +13,14 @@ namespace orc { Expected> TPCDynamicLibrarySearchGenerator::Load(TargetProcessControl &TPC, - const char *LibraryPath) { + const char *LibraryPath, + SymbolPredicate Allow) { auto Handle = TPC.loadDylib(LibraryPath); if (!Handle) return Handle.takeError(); - return std::make_unique(TPC, *Handle); + return std::make_unique(TPC, *Handle, + std::move(Allow)); } Error TPCDynamicLibrarySearchGenerator::tryToGenerate( @@ -28,22 +30,38 @@ Error TPCDynamicLibrarySearchGenerator::tryToGenerate( if (Symbols.empty()) return Error::success(); + SymbolLookupSet LookupSymbols; + + for (auto &KV : Symbols) { + // Skip symbols that don't match the filter. + if (Allow && !Allow(KV.first)) + continue; + LookupSymbols.add(KV.first, SymbolLookupFlags::WeaklyReferencedSymbol); + } + SymbolMap NewSymbols; - TargetProcessControl::LookupRequestElement Request(H, Symbols); + TargetProcessControl::LookupRequestElement Request(H, LookupSymbols); auto Result = TPC.lookupSymbols(Request); if (!Result) return Result.takeError(); assert(Result->size() == 1 && "Results for more than one library returned"); - assert(Result->front().size() == Symbols.size() && + assert(Result->front().size() == LookupSymbols.size() && "Result has incorrect number of elements"); + SymbolNameVector MissingSymbols; auto ResultI = Result->front().begin(); - for (auto &KV : Symbols) - NewSymbols[KV.first] = - JITEvaluatedSymbol(*ResultI++, JITSymbolFlags::Exported); + for (auto &KV : LookupSymbols) + if (*ResultI) + NewSymbols[KV.first] = + JITEvaluatedSymbol(*ResultI++, JITSymbolFlags::Exported); + + // If there were no resolved symbols bail out. + if (NewSymbols.empty()) + return Error::success(); + // Define resolved symbols. return JD.define(absoluteSymbols(std::move(NewSymbols))); } diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcessControl.cpp b/llvm/lib/ExecutionEngine/Orc/TargetProcessControl.cpp index 59c9ce2..1e7736d 100644 --- a/llvm/lib/ExecutionEngine/Orc/TargetProcessControl.cpp +++ b/llvm/lib/ExecutionEngine/Orc/TargetProcessControl.cpp @@ -78,14 +78,14 @@ SelfTargetProcessControl::lookupSymbols(LookupRequest Request) { auto &Sym = KV.first; std::string Tmp((*Sym).data() + !!GlobalManglingPrefix, (*Sym).size() - !!GlobalManglingPrefix); - if (void *Addr = Dylib->getAddressOfSymbol(Tmp.c_str())) - R.back().push_back(pointerToJITTargetAddress(Addr)); - else if (KV.second == SymbolLookupFlags::RequiredSymbol) { + void *Addr = Dylib->getAddressOfSymbol(Tmp.c_str()); + if (!Addr && KV.second == SymbolLookupFlags::RequiredSymbol) { // FIXME: Collect all failing symbols before erroring out. SymbolNameVector MissingSymbols; MissingSymbols.push_back(Sym); return make_error(std::move(MissingSymbols)); } + R.back().push_back(pointerToJITTargetAddress(Addr)); } } diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp index f1cc1f2..a848bf0 100644 --- a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp +++ b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp @@ -17,6 +17,7 @@ #include "llvm/BinaryFormat/Magic.h" #include "llvm/ExecutionEngine/JITLink/EHFrameSupport.h" #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h" +#include "llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h" #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCDisassembler/MCDisassembler.h" @@ -30,7 +31,6 @@ #include "llvm/Object/ObjectFile.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" -#include "llvm/Support/DynamicLibrary.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Process.h" @@ -802,19 +802,13 @@ Error sanitizeArguments(const Session &S) { } Error loadProcessSymbols(Session &S) { - std::string ErrMsg; - if (sys::DynamicLibrary::LoadLibraryPermanently(nullptr, &ErrMsg)) - return make_error(std::move(ErrMsg), inconvertibleErrorCode()); - - char GlobalPrefix = - S.TPC->getTargetTriple().getObjectFormat() == Triple::MachO ? '_' : '\0'; auto InternedEntryPointName = S.ES.intern(EntryPointName); auto FilterMainEntryPoint = [InternedEntryPointName](SymbolStringPtr Name) { return Name != InternedEntryPointName; }; S.MainJD->addGenerator( - ExitOnErr(orc::DynamicLibrarySearchGenerator::GetForCurrentProcess( - GlobalPrefix, FilterMainEntryPoint))); + ExitOnErr(orc::TPCDynamicLibrarySearchGenerator::GetForTargetProcess( + *S.TPC, std::move(FilterMainEntryPoint)))); return Error::success(); } -- 2.7.4