[clang-repl] Use default visibility for symbols while building CompilerInstance against emscripten

[anutosh491]

When running clang-repl in the browser we end up having something like the following
"" -cc1 -triple wasm32-unknown-emscripten ...... -main-file-name "<<< inputs >>>" .... -fvisibility=hidden .... -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>"

Due to the default introduced through this commit (e3d71e1#diff-b5496baaf5c83e1ffa1a26d0815843b8d3224aba84366cbb6aeecf703808c803R2083)

That being said, when we generated the shared libraries to be loaded on top of the main module while running clang-repl in the browser, we have to surpass the above through --export-all

llvm-project/clang/lib/Interpreter/Wasm.cpp

Line 78 in b3e2b1a

"--export-all",

This is because obviously incr_module_XX.wasm might want to access symbols out of file from incr_module_XX-1.wasm to incr_mdoule_0.wasm

But this also exports some problematic things like __dso_handle that causes conflicts across modules.

A better way to deal with this is to pass -fvisibility=default to the CompilerInstance.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Anutosh Bhat (anutosh491)

Changes

When running clang-repl in the browser we end up having something like the following
"" -cc1 -triple wasm32-unknown-emscripten ...... -main-file-name "<<< inputs >>>" .... -fvisibility=hidden .... -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>"

Due to the default introduced through this commit (e3d71e1#diff-b5496baaf5c83e1ffa1a26d0815843b8d3224aba84366cbb6aeecf703808c803R2083)

That being said, when we generated the shared libraries to be loaded on top of the main module while running clang-repl in the browser, we have to surpass the above through --export-all

llvm-project/clang/lib/Interpreter/Wasm.cpp

Line 78 in b3e2b1a

"--export-all",

This is because obviously incr_module_XX.wasm might want to access symbols out of file from incr_module_XX-1.wasm to incr_mdoule_0.wasm

But this also exports some problematic things like __dso_handle that causes conflicts across modules.

A better way to deal with this is to pass -fvisibility=default to the CompilerInstance.

---

Full diff: https://github.com/llvm/llvm-project/pull/116779.diff

2 Files Affected:

• (modified) clang/lib/Interpreter/Interpreter.cpp (+1)
• (modified) clang/lib/Interpreter/Wasm.cpp (-1)

diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 73ad766e655180..999271aae7491d 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -197,6 +197,7 @@ IncrementalCompilerBuilder::CreateCpp() {
   Argv.push_back("-target");
   Argv.push_back("wasm32-unknown-emscripten");
   Argv.push_back("-shared");
+  Argv.push_back("-fvisibility=default")
 #endif
   Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
 
diff --git a/clang/lib/Interpreter/Wasm.cpp b/clang/lib/Interpreter/Wasm.cpp
index 79efbaa03982d0..6d4cc478dd6a85 100644
--- a/clang/lib/Interpreter/Wasm.cpp
+++ b/clang/lib/Interpreter/Wasm.cpp
@@ -75,7 +75,6 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
                                           "-shared",
                                           "--import-memory",
                                           "--no-entry",
-                                          "--export-all",
                                           "--experimental-pic",
                                           "--stack-first",
                                           "--allow-undefined",

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[anutosh491]

I just realized that cleanUp shouldn't use const. While building against emscripten, I go this error

 │ │ /home/runner/work/recipes/recipes/output/bld/rattler-build_llvm_1732028457/work/clang/lib/Interpreter/Wasm.cpp:111:38: error: out-of-line definition of 'cl
 │ │ eanUp' does not match any declaration in 'clang::WasmIncrementalExecutor'
 │ │   111 | llvm::Error WasmIncrementalExecutor::cleanUp() const {
 │ │       |                                      ^~~~~~~
 │ │ /home/runner/work/recipes/recipes/output/bld/rattler-build_llvm_1732028457/work/clang/lib/Interpreter/Wasm.h:31:15: note: member declaration does not match
 │ │  because it is not const qualified
 │ │    31 |   llvm::Error cleanUp() override;
 │ │       |               ^         ~~~~~~~~

Fixed this through the latest commit.

[anutosh491]

Closing as superseded by #117978