[clang-repl] Implement LoadDynamicLibrary for clang-repl wasm use cases #133037
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Anutosh Bhat (anutosh491) ChangesFull diff: https://github.com/llvm/llvm-project/pull/133037.diff 1 Files Affected:
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index fa4c1439c9261..b4c68f340abc0 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -18,6 +18,7 @@
#include "llvm/Support/VirtualFileSystem.h"
#ifdef __EMSCRIPTEN__
#include "Wasm.h"
+#include <dlfcn.h>
#endif // __EMSCRIPTEN__
#include "clang/AST/ASTConsumer.h"
@@ -711,6 +712,14 @@ llvm::Error Interpreter::Undo(unsigned N) {
}
llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
+#ifdef __EMSCRIPTEN__
+ void* handle = dlopen(name, RTLD_NOW | RTLD_GLOBAL);
+ if (!handle) {
+ llvm::errs() << dlerror() << '\n';
+ return llvm::make_error<llvm::StringError>(
+ "Failed to load dynamic library", llvm::inconvertibleErrorCode());
+ }
+#else
auto EE = getExecutionEngine();
if (!EE)
return EE.takeError();
@@ -722,6 +731,7 @@ llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
EE->getMainJITDylib().addGenerator(std::move(*DLSG));
else
return DLSG.takeError();
+#endif
return llvm::Error::success();
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
For anyone concerned, Such a feature would be tested out in downstream projects like cppinterop and xeus-cpp as currently llvm doesn't have a concrete structure to test clang-repl in the browser. |
| return llvm::make_error<llvm::StringError>("Failed to load dynamic library", | ||
| llvm::inconvertibleErrorCode()); | ||
| } | ||
| #else |
There was a problem hiding this comment.
Why we can’t improve llvm::orc::DynamicLibrarySearchGenerator::Load?
cc: @lhames
There was a problem hiding this comment.
Hi, thanks for the suggestion.
I did some surface level introspection on DynamicLibrarySearchGenerator::Load and I see it calls dlopen internally but I think the handle is wrapped in a DynamicLibrarySearchGenerator, which lets the ORC JIT symbol resolution system look inside the .so for symbols during later getSymbolAddress(...) calls.
So even if DynamicLibrarySearchGenerator::Load(...) works under WASM (which it might), it:
i) Creates a JITDylib search generator
ii) Which requires an ORC execution engine
iii) Which we don't make use of correct ? (we're using WasmIncrementalExecutor instead)
So as per what I see DynamicLibrarySearchGenerator infrastructure creates symbol generators for ORC JIT Dylibs, which isn’t used in the wasm execution model. So while the underlying dlopen(...) mechanism is similar, DynamicLibrarySearchGenerator wouldn’t integrate meaningfully unless the wasm executor also moved to using ORC JIT infrastructure, which it currently doesn’t.
There was a problem hiding this comment.
Additionally, path resolution isn't a concern in the wasm case for a few reasons (which is why a simple combination of dlopen + dlsym should do the job isn't it ?)
-
All dynamic libraries are preloaded into MEMFS at known paths during compilation or initialization (e.g., via Emscripten’s --preload-file flag). This means the library’s location is fully deterministic at runtime.
-
No concept of system-level search paths like LD_LIBRARY_PATH in the browser — we can’t rely on an OS-level dynamic loader. Instead, we always load by the full path (e.g., dlopen("/libsymengine.so")), which resolves directly in the virtual MEMFS.
-
Because users control the preload path, and because dlopen in Emscripten expects an exact match in the in-memory filesystem, we don’t need infrastructure for path lookup, search path prioritization, or canonicalization. A simple, direct path-based dlopen is all that’s required.
There was a problem hiding this comment.
Check how we can use the changes here in cppinterop to get dynamic library loaded and symbol addresses fetched at runtime (just needs some changes to LoadLibrary and no change to GetFunctionAddress)
compiler-research/CppInterOp@main...anutosh491:CppInterOp:shared_libs
Everything boils down to preload as we know where in the MEMFS our shared lib exists !
The tests would pass as such
test 1
Start 1: cppinterop-DynamicLibraryManagerTests
........ logs .....
.......... logs .....
1: End of search list.
1: [ OK ] DynamicLibraryManagerTest.BasicSymbolLookup (577 ms)
1: [----------] 2 tests from DynamicLibraryManagerTest (580 ms total)
1:
1: [----------] Global test environment tear-down
1: [==========] 2 tests from 1 test suite ran. (591 ms total)
1: [ PASSED ] 1 test.
1: [ SKIPPED ] 1 test, listed below:
1: [ SKIPPED ] DynamicLibraryManagerTest.Sanity
anutosh491
force-pushed
the
dynamic_libs
branch
from
b717060 to
bc41b71
Compare
|
We really need to figure out how to add in-tree tests. |
Yupp sadly I was only able to test this through cppinterop and xeus-cpp as I've commented above (#133037 (comment)) where it works as expected :\ I shall take out some time in the near future to ideate on this. |
I am wondering if we can add such CI for llvm, too... maybe you can discuss that at the llvm discord's infrastructure channel... |
Absolutely, I shall do that and get back to you on this (I think there are people at llvm looking into llvm+emscripten use cases and should support something like this) Shall keep you updated. In the meantime would be great to have your views on the above ... and possibly if you could give me the green flag here :) |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/15260 Here is the relevant piece of the build log for the reference |
|
The above failure reported doesn't look related. Cherry picking commit for 20.1.3 /cherry-pick 8f56394 |
Error: Command failed due to missing milestone. |
…es (llvm#133037) **Currently we don't make use of the JIT for the wasm use cases so the approach using the execution engine won't work in these cases.** Rather if we use dlopen. We should be able to do the following (demonstrating through a toy project) 1) Make use of LoadDynamicLibrary through the given implementation ``` extern "C" EMSCRIPTEN_KEEPALIVE int load_library(const char *name) { auto Err = Interp->LoadDynamicLibrary(name); if (Err) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "load_library error: "); return -1; } return 0; } ``` 2) Add a button to call load_library once the library has been added in our MEMFS (currently we have symengine built as a SIDE MODULE and we are loading it)
Currently we don't make use of the JIT for the wasm use cases so the approach using the execution engine won't work in these cases.
Rather if we use dlopen. We should be able to do the following (demonstrating through a toy project)
Screen.Recording.2025-03-26.at.11.00.30.AM.mov