Skip to content

Commit

Permalink
[cling] Restore symbol lookup in child interpreters
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jalopezg-git committed May 31, 2023
1 parent 0084c1f commit 88f5fa0
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 9 deletions.
7 changes: 6 additions & 1 deletion interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 5 additions & 7 deletions interpreter/cling/lib/Interpreter/IncrementalExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions interpreter/cling/lib/Interpreter/IncrementalJIT.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,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<Expected<JITEvaluatedSymbol>(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 std::unique_ptr<TargetMachine>
CreateTargetMachine(const clang::CompilerInstance& CI) {
CodeGenOpt::Level OptLevel = CodeGenOpt::Default;
Expand Down Expand Up @@ -444,6 +471,11 @@ IncrementalJIT::IncrementalJIT(
Jit->getExecutionSession().setErrorReporter(ErrorReporter);
}

std::unique_ptr<llvm::orc::DefinitionGenerator> IncrementalJIT::getGenerator() {
return std::make_unique<DelegateGenerator>(
[&](StringRef UnmangledName) { return Jit->lookup(UnmangledName); });
}

void IncrementalJIT::addModule(Transaction& T) {
ResourceTrackerSP RT = Jit->getMainJITDylib().createResourceTracker();
m_ResourceTrackers[&T] = RT;
Expand Down
6 changes: 6 additions & 0 deletions interpreter/cling/lib/Interpreter/IncrementalJIT.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<llvm::orc::DefinitionGenerator> 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)
Expand Down
3 changes: 2 additions & 1 deletion interpreter/cling/lib/Interpreter/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 88f5fa0

Please sign in to comment.