Skip to content

[clang-repl] Implement LoadDynamicLibrary for clang-repl wasm use cases #133037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 1, 2025
Merged

[clang-repl] Implement LoadDynamicLibrary for clang-repl wasm use cases #133037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7ccd0e
Implement LoadDynamicLibrary for clang-repl wasm use cases
anutosh491 Mar 26, 2025
bc41b71
Override getSymbolAddress
anutosh491 Mar 27, 2025
60d4622
Update Wasm.h
anutosh491 Mar 27, 2025
5037017
fix clang format
anutosh491 Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/lib/Interpreter/IncrementalExecutor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class IncrementalExecutor {
virtual llvm::Error removeModule(PartialTranslationUnit &PTU);
virtual llvm::Error runCtors() const;
virtual llvm::Error cleanUp();
llvm::Expected<llvm::orc::ExecutorAddr>
virtual llvm::Expected<llvm::orc::ExecutorAddr>
getSymbolAddress(llvm::StringRef Name, SymbolNameKind NameKind) const;

llvm::orc::LLJIT &GetExecutionEngine() { return *Jit; }
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Interpreter/Interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "llvm/Support/VirtualFileSystem.h"
#ifdef __EMSCRIPTEN__
#include "Wasm.h"
#include <dlfcn.h>
#endif // __EMSCRIPTEN__

#include "clang/AST/ASTConsumer.h"
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we can’t improve llvm::orc::DynamicLibrarySearchGenerator::Load?

cc: @lhames

Copy link
Member Author

@anutosh491 anutosh491 Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?)

  1. 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.

  2. 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.

  3. 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.

Copy link
Member Author

@anutosh491 anutosh491 Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

auto EE = getExecutionEngine();
if (!EE)
return EE.takeError();
Expand All @@ -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();
}
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/Interpreter/Wasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,19 @@ llvm::Error WasmIncrementalExecutor::cleanUp() {
return llvm::Error::success();
}

llvm::Expected<llvm::orc::ExecutorAddr>
WasmIncrementalExecutor::getSymbolAddress(llvm::StringRef Name,
SymbolNameKind NameKind) const {
void *Sym = dlsym(RTLD_DEFAULT, Name.str().c_str());
if (!Sym) {
return llvm::make_error<llvm::StringError>("dlsym failed for symbol: " +
Name.str(),
llvm::inconvertibleErrorCode());
}

return llvm::orc::ExecutorAddr::fromPtr(Sym);
}

WasmIncrementalExecutor::~WasmIncrementalExecutor() = default;

} // namespace clang
3 changes: 3 additions & 0 deletions clang/lib/Interpreter/Wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class WasmIncrementalExecutor : public IncrementalExecutor {
llvm::Error removeModule(PartialTranslationUnit &PTU) override;
llvm::Error runCtors() const override;
llvm::Error cleanUp() override;
llvm::Expected<llvm::orc::ExecutorAddr>
getSymbolAddress(llvm::StringRef Name,
SymbolNameKind NameKind) const override;

~WasmIncrementalExecutor() override;
};
Expand Down