[ORC] Add Executor Resolver Utility #143654
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
@lhames, this looks reasonable to me. I'd like move forward with these changes. Can you take a look?
b3ac7cb to
c8febe2
Compare
| ExecutorAddr Lookup; | ||
| ExecutorAddr Resolve; |
There was a problem hiding this comment.
Is Lookup still used after this patch? I'm not opposed to replacing raw dylib handles with Resolver objects (as long as it doesn't change the dylib lifetimes, which I don't think it does), but if we are doing that then we can drop Lookup entirely and clean up the lookup wrappers and implementations in the target process library.
c8febe2 to
701f14a
Compare
|
Previously, we were returning an error for a required symbol in As you can see below, the error is being silently consumed: JITBuilder->setPrePlatformSetup([](llvm::orc::LLJIT &J) {
// Try to enable debugging of JIT'd code (only works with JITLink for
// ELF and MachO).
consumeError(llvm::orc::enableDebuggerSupport(J));
return llvm::Error::success();
});The comment here shows that debugger support is only implemented for ELF and Mach-O. That explains why the assertion fails on Windows: no valid address is returned in this case. Instead of asserting, we could return an error when the |
This patch adds the new executor-side resolver API as suggested by @lhames. It introduces a
DylibSymbolResolverthat helps resolve symbols for each loaded dylib.Previously, we returned a
DylibHandleto the controller. Now, we wrap the native handle insideDylibSymbolResolverand return aResolverHandleinstead. This makes the code cleaner and separates the symbol resolution logic from raw handle management.