https://github.com/anutosh491 created https://github.com/llvm/llvm-project/pull/197133
Fixes #178139 This work is built on top of discussions done in https://github.com/llvm/llvm-project/pull/132670 Making a fresh PR cause the branch is a a year old and a bit outdated. ### Problem Statement : This is a 3 part problem statement. Imagine our compiler builder args look like this ``` CB.SetCompilerArgs({ "-v", "-xc++", "-std=c++23", "-Xclang", "-iwithsysroot/include/compat", "-fwasm-exceptions", "-mllvm", "-wasm-enable-sjlj" }); ``` **Problem 1** : clang-repl won't respect mllvm args just yet But the interpreter should be responsible for this. Hence i) We move relevant code into a function called `parseLLVMArgs` in `CompilerInstance.cpp` ii) And we need to add `CI->parseLLVMArgs();` while interpreter creation in `Interpreter.cpp` **Result** : We are now at a stage where clang-repl would repsect the mllvm args passed from the top **Problem 2** : clang-repl would respect the mllvm but only for the first `ParseAndExecute` call. So the block parsing the `Runtimes` is formed correctly. Any cell after that would fail with ``` LLVM ERROR: -exception-model=wasm only allowed with at least one of -wasm-enable-eh or -wasm-enable-sjlj Aborted() ``` And this is because of this line here. https://github.com/llvm/llvm-project/blob/3bab75245a2bbbec36206f7e0c569c17a1a487e6/lld/wasm/Driver.cpp#L1315 We call `lldMain` in `Wasm.cpp` which leads us to `linkerMain` in `lld/wasm/Driver.cpp`. Now I thought this is a case where wasm-ld is not respecting "incremental" compilation and resetting options set by the clang globally. But as Sam's comment conveys [here](https://github.com/llvm/llvm-project/issues/178139#issuecomment-3806525095) I think wasm-ld/lld's design **is not wrong**. My understanding goes like this 1. lld's reset is correct within lld's model. lld was never designed to coexist with a live incremental compiler in the same process. Its cleanup code exists for test isolation and is doing exactly what it was designed to do. Asking lld to skip the reset would break test isolation 2. lld cannot know which options it "owns" vs. which were set by the caller. LLVM's `cl::` system is a global flat namespace. There is no ownership or scoping concept. When lld calls `ResetAllOptionOccurrences()`, it resets *everything* — it literally cannot distinguish. 3. The options being reset are conceptually the CALLER's responsibility. `-wasm-enable-eh` and `-wasm-enable-sjlj` are codegen flags set by the frontend (via `-fwasm-exceptions` / `-mllvm`). The fact that they happen to live in the same global cl registry that lld resets is an artifact of LLVM's global option design, not a meaningful coupling between lld and clang. The Right Fix: **Restore in WasmIncrementalExecutor::addModule()** **Result** : clang-repl would respect mllvm across all cells. <img width="703" height="454" alt="image" src="https://github.com/user-attachments/assets/862d5459-6e78-41e3-bc03-1bbd8c081edd"; /> **Problem 3** : Once we've solved the above 2 problems. I added a test in InterpreterTest.cpp but that woudl fail with ``` [----------] 1 test from InterpreterTest [ RUN ] InterpreterTest.EmscriptenExceptionHandling [DBG CreateCpp] wasm-enable-eh registered: NO [DBG CreateCpp] wasm-enable-sjlj registered: NO clang version 23.0.0git ([email protected]:anutosh491/llvm-project.git d1b0c854354cb9bc0033b126fa1f71b2a6c721b2) Target: wasm32-unknown-emscripten Thread model: posix InstalledDir: "" -cc1 -triple wasm32-unknown-emscripten -emit-obj -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name "<<< inputs >>>" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-feature +exception-handling -target-feature +multivalue -target-feature +reference-types -exception-model=wasm -mllvm -wasm-enable-eh -target-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/ -v -fcoverage-compilation-dir=/ -resource-dir lib/clang/23 -internal-isystem lib/clang/23/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++23 -fdeprecated-macro -ferror-limit 19 -fmessage-length=80 -fvisibility=default -fgnuc-version=4.2.1 -fno-implicit-modules -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -exception-model=wasm -emit-llvm-only -fincremental-extensions -mllvm -wasm-enable-sjlj -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>" [DBG parseLLVMArgs] wasm-enable-eh registered: NO [DBG parseLLVMArgs] wasm-enable-sjlj registered: NO [DBG parseLLVMArgs] args to parse: -wasm-enable-eh -wasm-enable-sjlj clang (LLVM option parsing): Unknown command line argument '-wasm-enable-eh'. Try: 'clang (LLVM option parsing) --help' clang (LLVM option parsing): Did you mean '--rng-seed'? clang (LLVM option parsing): Unknown command line argument '-wasm-enable-sjlj'. Try: 'clang (LLVM option parsing) --help' clang (LLVM option parsing): Did you mean '--sort-timers'? ``` **Root cause** : llvm_shutdown() Destroys the Option Registry `InterpreterTestBase::TearDownTestSuite()` (in `InterpreterTestFixture.h`) calls `llvm::llvm_shutdown()` after each test suite completes. This function destroys all `ManagedStatic<>` objects, including `ManagedStatic<CommandLineParser>` — the singleton that holds the entire `cl::opt` registry. When the next test suite starts and accesses the `CommandLineParser` again, a fresh empty instance is created. The `cl::opt` global objects (like`WasmEnableEH`) still exist in memory — their constructors ran at program startup — but they have `FullyInitialized = true` and will **not** call `addArgument()` again. So they remain permanently unregistered in the new parser. Fix : Guard llvm_shutdown() on Emscripten. Have added a comment in `TearDownTestSuite()` for the same. Now we have ``` [ RUN ] InterpreterTest.EmscriptenExceptionHandling clang version 23.0.0git ([email protected]:anutosh491/llvm-project.git d1b0c854354cb9bc0033b126fa1f71b2a6c721b2) Target: wasm32-unknown-emscripten Thread model: posix InstalledDir: "" -cc1 -triple wasm32-unknown-emscripten -emit-obj -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name "<<< inputs >>>" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-feature +exception-handling -target-feature +multivalue -target-feature +reference-types -exception-model=wasm -mllvm -wasm-enable-eh -target-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/ -v -fcoverage-compilation-dir=/ -resource-dir lib/clang/23 -internal-isystem lib/clang/23/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++23 -fdeprecated-macro -ferror-limit 19 -fmessage-length=80 -fvisibility=default -fgnuc-version=4.2.1 -fno-implicit-modules -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -exception-model=wasm -emit-llvm-only -fincremental-extensions -mllvm -wasm-enable-sjlj -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>" clang -cc1 version 23.0.0git based upon LLVM 23.0.0git default target wasm32-unknown-emscripten ignoring nonexistent directory "lib/clang/23/include" ignoring nonexistent directory "/include/wasm32-emscripten" ignoring nonexistent directory "/include" #include "..." search starts here: End of search list. [ OK ] InterpreterTest.EmscriptenExceptionHandling (37 ms) [----------] 10 tests from InterpreterTest (674 ms total) ``` >From 707120c326feb3ae99be02863e21cf7efeef58b9 Mon Sep 17 00:00:00 2001 From: anutosh491 <[email protected]> Date: Tue, 12 May 2026 14:22:33 +0530 Subject: [PATCH] Handle -mllvm args for clang-repl before calling executeAction --- clang/include/clang/Frontend/CompilerInstance.h | 5 +++++ .../clang/Interpreter/IncrementalExecutor.h | 3 +++ clang/lib/Frontend/CompilerInstance.cpp | 14 ++++++++++++++ .../FrontendTool/ExecuteCompilerInvocation.cpp | 16 +--------------- clang/lib/Interpreter/IncrementalExecutor.cpp | 2 +- clang/lib/Interpreter/Interpreter.cpp | 7 +++++++ clang/lib/Interpreter/Wasm.cpp | 17 ++++++++++++++++- clang/lib/Interpreter/Wasm.h | 5 ++++- clang/unittests/Interpreter/CMakeLists.txt | 1 + clang/unittests/Interpreter/InterpreterTest.cpp | 16 ++++++++++++++++ .../Interpreter/InterpreterTestFixture.h | 12 +++++++++++- 11 files changed, 79 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index bce9ff421ec87..6502c22e9dd83 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -259,6 +259,11 @@ class CompilerInstance : public ModuleLoader { /// Load the list of plugins requested in the \c FrontendOptions. void LoadRequestedPlugins(); + /// Parse and apply LLVM command line arguments from FrontendOptions. + /// This processes the LLVMArgs option that comes from -mllvm flags. + /// This should be called after plugins are loaded and before ExecuteAction. + void parseLLVMArgs(); + /// @} /// @name Compiler Invocation and Options /// @{ diff --git a/clang/include/clang/Interpreter/IncrementalExecutor.h b/clang/include/clang/Interpreter/IncrementalExecutor.h index 0e879efa0b55f..a293978b2917b 100644 --- a/clang/include/clang/Interpreter/IncrementalExecutor.h +++ b/clang/include/clang/Interpreter/IncrementalExecutor.h @@ -52,6 +52,9 @@ class IncrementalExecutorBuilder { std::optional<llvm::CodeModel::Model> CM = std::nullopt; /// An optional external IncrementalExecutor std::unique_ptr<IncrementalExecutor> IE; + /// mllvm args from the frontend; on wasm these are re-applied after each + /// lldMain call because lld resets all cl options for test-isolation purposes. + std::vector<std::string> LLVMArgs; /// An optional external orc jit builder std::unique_ptr<llvm::orc::LLJITBuilder> JITBuilder; /// A default callback that can be used in the IncrementalCompilerBuilder to diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 91eda7392784f..52357fd86a3b4 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1108,6 +1108,20 @@ void CompilerInstance::LoadRequestedPlugins() { } } +void CompilerInstance::parseLLVMArgs() { + if (!getFrontendOpts().LLVMArgs.empty()) { + unsigned NumArgs = getFrontendOpts().LLVMArgs.size(); + auto Args = std::make_unique<const char *[]>(NumArgs + 2); + Args[0] = "clang (LLVM option parsing)"; + for (unsigned i = 0; i != NumArgs; ++i) + Args[i + 1] = getFrontendOpts().LLVMArgs[i].c_str(); + Args[NumArgs + 1] = nullptr; + llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get(), /*Overview=*/"", + /*Errs=*/nullptr, + /*VFS=*/&getVirtualFileSystem()); + } +} + /// Determine the appropriate source input kind based on language /// options. static Language getLanguageFromOptions(const LangOptions &LangOpts) { diff --git a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp index e4622496758ac..7ecf356f15641 100644 --- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp +++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp @@ -239,21 +239,7 @@ bool ExecuteCompilerInvocation(CompilerInstance *Clang) { Clang->LoadRequestedPlugins(); - // Honor -mllvm. - // - // FIXME: Remove this, one day. - // This should happen AFTER plugins have been loaded! - if (!Clang->getFrontendOpts().LLVMArgs.empty()) { - unsigned NumArgs = Clang->getFrontendOpts().LLVMArgs.size(); - auto Args = std::make_unique<const char*[]>(NumArgs + 2); - Args[0] = "clang (LLVM option parsing)"; - for (unsigned i = 0; i != NumArgs; ++i) - Args[i + 1] = Clang->getFrontendOpts().LLVMArgs[i].c_str(); - Args[NumArgs + 1] = nullptr; - llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get(), /*Overview=*/"", - /*Errs=*/nullptr, - /*VFS=*/&Clang->getVirtualFileSystem()); - } + Clang->parseLLVMArgs(); #if CLANG_ENABLE_STATIC_ANALYZER // These should happen AFTER plugins have been loaded! diff --git a/clang/lib/Interpreter/IncrementalExecutor.cpp b/clang/lib/Interpreter/IncrementalExecutor.cpp index 65cb29a2f441a..51a601433e552 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.cpp +++ b/clang/lib/Interpreter/IncrementalExecutor.cpp @@ -393,7 +393,7 @@ IncrementalExecutorBuilder::create(llvm::orc::ThreadSafeContext &TSC, llvm::Error Err = llvm::Error::success(); std::unique_ptr<IncrementalExecutor> Executor; #ifdef __EMSCRIPTEN__ - Executor = std::make_unique<WasmIncrementalExecutor>(Err); + Executor = std::make_unique<WasmIncrementalExecutor>(Err, LLVMArgs); #else Executor = std::make_unique<OrcIncrementalExecutor>(TSC, *JITBuilder, Err); #endif diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp index 2684a00ce5f07..5d02dceb472a2 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -257,6 +257,9 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance, auto LLVMCtx = std::make_unique<llvm::LLVMContext>(); TSCtx = std::make_unique<llvm::orc::ThreadSafeContext>(std::move(LLVMCtx)); + // Honor -mllvm options + CI->parseLLVMArgs(); + Act = TSCtx->withContextDo([&](llvm::LLVMContext *Ctx) { return std::make_unique<IncrementalAction>(*CI, *Ctx, ErrOut, *this, std::move(Consumer)); @@ -499,6 +502,10 @@ llvm::Error Interpreter::CreateExecutor() { if (!IncrExecutorBuilder) IncrExecutorBuilder = std::make_unique<IncrementalExecutorBuilder>(); + // Propagate mllvm args so the wasm executor can restore them after each + // lldMain invocation (which resets all cl options for test isolation). + IncrExecutorBuilder->LLVMArgs = CI->getFrontendOpts().LLVMArgs; + auto ExecutorOrErr = IncrExecutorBuilder->create(*TSCtx, CI->getTarget()); if (ExecutorOrErr) IncrExecutor = std::move(*ExecutorOrErr); diff --git a/clang/lib/Interpreter/Wasm.cpp b/clang/lib/Interpreter/Wasm.cpp index 96600cf9fa6d0..9ebd23da5f564 100644 --- a/clang/lib/Interpreter/Wasm.cpp +++ b/clang/lib/Interpreter/Wasm.cpp @@ -12,6 +12,7 @@ #include "Wasm.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include <llvm/IR/LegacyPassManager.h> @@ -59,7 +60,9 @@ bool link(llvm::ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS, namespace clang { -WasmIncrementalExecutor::WasmIncrementalExecutor(llvm::Error &Err) { +WasmIncrementalExecutor::WasmIncrementalExecutor(llvm::Error &Err, + std::vector<std::string> LLVMArgs) + : StoredLLVMArgs(std::move(LLVMArgs)) { llvm::ErrorAsOutParameter EAO(&Err); if (Err) @@ -132,6 +135,18 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) { lld::Result Result = lld::lldMain(LinkerArgs, llvm::outs(), llvm::errs(), WasmDriverArgs); + // lld::wasm::linkerMain calls cl::ResetAllOptionOccurrences() which wipes + // all global LLVM cl options, including mllvm flags set by the frontend + // (e.g. -wasm-enable-eh, -wasm-enable-sjlj). Re-apply them so the next + // Parse() call's WebAssemblyTargetMachine creation finds the correct state. + if (!StoredLLVMArgs.empty()) { + std::vector<const char *> ArgPtrs; + ArgPtrs.push_back("clang-repl (restoring LLVM options)"); + for (const std::string &Arg : StoredLLVMArgs) + ArgPtrs.push_back(Arg.c_str()); + llvm::cl::ParseCommandLineOptions(ArgPtrs.size(), ArgPtrs.data()); + } + if (Result.retCode) return llvm::make_error<llvm::StringError>( "Failed to link incremental module", llvm::inconvertibleErrorCode()); diff --git a/clang/lib/Interpreter/Wasm.h b/clang/lib/Interpreter/Wasm.h index bf8777a41eac2..6340393dc6b9b 100644 --- a/clang/lib/Interpreter/Wasm.h +++ b/clang/lib/Interpreter/Wasm.h @@ -19,12 +19,14 @@ #include "clang/Interpreter/IncrementalExecutor.h" #include "llvm/ADT/SmallString.h" +#include <string> +#include <vector> namespace clang { class WasmIncrementalExecutor : public IncrementalExecutor { public: - WasmIncrementalExecutor(llvm::Error &Err); + WasmIncrementalExecutor(llvm::Error &Err, std::vector<std::string> LLVMArgs); ~WasmIncrementalExecutor() override; llvm::Error addModule(PartialTranslationUnit &PTU) override; @@ -38,6 +40,7 @@ class WasmIncrementalExecutor : public IncrementalExecutor { private: llvm::SmallString<256> TempDir; + std::vector<std::string> StoredLLVMArgs; }; } // namespace clang diff --git a/clang/unittests/Interpreter/CMakeLists.txt b/clang/unittests/Interpreter/CMakeLists.txt index 66b396b53cb55..9fdd69ea49069 100644 --- a/clang/unittests/Interpreter/CMakeLists.txt +++ b/clang/unittests/Interpreter/CMakeLists.txt @@ -76,6 +76,7 @@ get_target_property(LINKED_LIBS ClangReplInterpreterTests LINK_LIBRARIES) list(REMOVE_ITEM LINKED_LIBS LLVMSupport) set_target_properties(ClangReplInterpreterTests PROPERTIES LINK_LIBRARIES "${LINKED_LIBS}") target_link_options(ClangReplInterpreterTests + PUBLIC "SHELL: -fwasm-exceptions" PUBLIC "SHELL: -s MAIN_MODULE=1" PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" PUBLIC "SHELL: -s STACK_SIZE=32mb" diff --git a/clang/unittests/Interpreter/InterpreterTest.cpp b/clang/unittests/Interpreter/InterpreterTest.cpp index 9ff9092524d21..32b6e00e05a67 100644 --- a/clang/unittests/Interpreter/InterpreterTest.cpp +++ b/clang/unittests/Interpreter/InterpreterTest.cpp @@ -443,4 +443,20 @@ TEST_F(InterpreterTest, TranslationUnit_CanonicalDecl) { sema.getASTContext().getTranslationUnitDecl()->getCanonicalDecl()); } +TEST_F(InterpreterTest, EmscriptenExceptionHandling) { +#ifndef __EMSCRIPTEN__ + GTEST_SKIP() << "This test only applies to Emscripten builds."; +#endif + + using Args = std::vector<const char *>; + Args ExtraArgs = {"-std=c++23", "-v", + "-fwasm-exceptions", + "-mllvm", "-wasm-enable-sjlj"}; + + std::unique_ptr<Interpreter> Interp = createInterpreter(ExtraArgs); + + llvm::cantFail( + Interp->ParseAndExecute("try { throw 1; } catch (...) { 0; }")); +} + } // end anonymous namespace diff --git a/clang/unittests/Interpreter/InterpreterTestFixture.h b/clang/unittests/Interpreter/InterpreterTestFixture.h index b088fa4a5f896..c4a30dd06220c 100644 --- a/clang/unittests/Interpreter/InterpreterTestFixture.h +++ b/clang/unittests/Interpreter/InterpreterTestFixture.h @@ -55,7 +55,17 @@ class InterpreterTestBase : public ::testing::Test { llvm::InitializeNativeTargetAsmPrinter(); } - static void TearDownTestSuite() { llvm::llvm_shutdown(); } + static void TearDownTestSuite() { + // llvm_shutdown() cleans up JIT (LLJIT/ORC) state between test suites. + // On Emscripten, WasmIncrementalExecutor is used instead of LLJIT, so + // there is no JIT state to tear down. Calling llvm_shutdown() here would + // destroy the ManagedStatic<CommandLineParser> global, permanently + // deregistering all cl::opt options (wasm-enable-eh, etc.) since their + // constructors already ran and will not fire again after the reset. +#ifndef __EMSCRIPTEN__ + llvm::llvm_shutdown(); +#endif + } }; } // namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
