From 27c9a6641ef40569ad34d2f8dd99e140f0afae2b Mon Sep 17 00:00:00 2001 From: Javier Lopez-Gomez Date: Tue, 30 May 2023 11:29:59 +0200 Subject: [PATCH] [cling] Restore symbol lookup in child interpreters Prior to the upgrade to LLVM13, child interpreters used to also lookup symbols in their parent. This commit introduces a `DefinitionGenerator` that allows for symbol lookup across different `IncrementalJIT` instances, which restores the old behavior. This change fixes the following tests: - CodeUnloading/AtExit.C - MultipleInterpreters/MultipleInterpreters.C Fixes #12455. --- .../lib/Interpreter/IncrementalExecutor.cpp | 7 +++- .../lib/Interpreter/IncrementalExecutor.h | 12 +++---- .../cling/lib/Interpreter/IncrementalJIT.cpp | 32 +++++++++++++++++++ .../cling/lib/Interpreter/IncrementalJIT.h | 6 ++++ .../cling/lib/Interpreter/Interpreter.cpp | 3 +- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp index 25f01de4d0565..5da617a3c2796 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp @@ -36,7 +36,7 @@ namespace cling { IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& /*diags*/, const clang::CompilerInstance& CI, void *ExtraLibHandle, bool Verbose): - m_Callbacks(nullptr), m_externalIncrementalExecutor(nullptr) + m_Callbacks(nullptr) #if 0 : m_Diags(diags) #endif @@ -60,6 +60,11 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& /*diags*/, IncrementalExecutor::~IncrementalExecutor() {} +void IncrementalExecutor::registerExternalIncrementalExecutor( + IncrementalExecutor& IE) { + m_JIT->addGenerator(IE.m_JIT->getGenerator()); +} + void IncrementalExecutor::runAtExitFuncs() { // It is legal to register an atexit handler from within another atexit // handler and furthor-more the standard says they need to run in reverse diff --git a/interpreter/cling/lib/Interpreter/IncrementalExecutor.h b/interpreter/cling/lib/Interpreter/IncrementalExecutor.h index e50f20bb15da1..428331c4a5f4a 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalExecutor.h +++ b/interpreter/cling/lib/Interpreter/IncrementalExecutor.h @@ -64,10 +64,6 @@ namespace cling { ///\brief Whom to call upon invocation of user code. InterpreterCallbacks* m_Callbacks; - ///\brief A pointer to the IncrementalExecutor of the parent Interpreter. - /// - IncrementalExecutor* m_externalIncrementalExecutor; - ///\brief Helper that manages when the destructor of an object to be called. /// /// The object is registered first as an CXAAtExitElement and then cling @@ -151,9 +147,11 @@ namespace cling { ~IncrementalExecutor(); - void setExternalIncrementalExecutor(IncrementalExecutor *extIncrExec) { - m_externalIncrementalExecutor = extIncrExec; - } + /// Register a different `IncrementalExecutor` object that can provide + /// addresses for external symbols. This is used by child interpreters to + /// lookup symbols defined in the parent. + void registerExternalIncrementalExecutor(IncrementalExecutor& IE); + void setCallbacks(InterpreterCallbacks* callbacks); const DynamicLibraryManager& getDynamicLibraryManager() const { diff --git a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp index 04bebf0558aa0..e98cb9e46684c 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalJIT.cpp @@ -403,6 +403,33 @@ Error RTDynamicLibrarySearchGenerator::tryToGenerate( return JD.define(absoluteSymbols(std::move(NewSymbols)), CurrentRT()); } +/// A definition generator that calls a user-provided function that is +/// responsible for providing symbol addresses. +/// This is used by `IncrementalJIT::getGenerator()` to yield a generator that +/// resolves symbols defined in the IncrementalJIT object on which the function +/// is called, which in turn may be used to provide lookup across different +/// IncrementalJIT instances. +class DelegateGenerator : public DefinitionGenerator { + using LookupFunc = std::function(StringRef)>; + LookupFunc lookup; + +public: + DelegateGenerator(LookupFunc lookup) : lookup(lookup) {} + + Error tryToGenerate(LookupState& LS, LookupKind K, JITDylib& JD, + JITDylibLookupFlags JDLookupFlags, + const SymbolLookupSet& LookupSet) override { + SymbolMap Symbols; + for (auto& KV : LookupSet) { + if (auto Addr = lookup(*KV.first)) + Symbols[KV.first] = Addr.get(); + } + if (Symbols.empty()) + return Error::success(); + return JD.define(absoluteSymbols(std::move(Symbols))); + } +}; + static bool UseJITLink(const Triple& TT) { bool jitLink = false; // Default to JITLink on macOS and RISC-V, as done in (recent) LLVM by @@ -603,6 +630,11 @@ IncrementalJIT::IncrementalJIT( Jit->getExecutionSession().setErrorReporter(ErrorReporter); } +std::unique_ptr IncrementalJIT::getGenerator() { + return std::make_unique( + [&](StringRef UnmangledName) { return Jit->lookup(UnmangledName); }); +} + void IncrementalJIT::addModule(Transaction& T) { ResourceTrackerSP RT = Jit->getMainJITDylib().createResourceTracker(); m_ResourceTrackers[&T] = RT; diff --git a/interpreter/cling/lib/Interpreter/IncrementalJIT.h b/interpreter/cling/lib/Interpreter/IncrementalJIT.h index 170cc403209c7..7fd712603e5a8 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalJIT.h +++ b/interpreter/cling/lib/Interpreter/IncrementalJIT.h @@ -66,6 +66,12 @@ class IncrementalJIT { Jit->getMainJITDylib().addGenerator(std::move(G)); } + /// Return a `DefinitionGenerator` that can provide addresses for symbols + /// reachable from this IncrementalJIT object. This function can be used in + /// conjunction with `addGenerator()` to provide symbol resolution across + /// diferent IncrementalJIT instances. + std::unique_ptr getGenerator(); + // FIXME: Accept a LLVMContext as well, e.g. the one that was used for the // particular module in Interpreter, CIFactory or BackendPasses (would be // more efficient) diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 8b0e48e0cfc28..34ce68f555df1 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -372,7 +372,8 @@ namespace cling { // Give my IncrementalExecutor a pointer to the Incremental executor of the // parent Interpreter. - m_Executor->setExternalIncrementalExecutor(parentInterpreter.m_Executor.get()); + m_Executor->registerExternalIncrementalExecutor( + *parentInterpreter.m_Executor); if (auto C = parentInterpreter.m_IncrParser->getDiagnosticConsumer()) m_IncrParser->setDiagnosticConsumer(C, /*Own=*/false);