Author: Anutosh Bhat Date: 2026-10-08T04:24:58Z New Revision: 415b70a8a11d369176ea2db5151424d9354ba41a
URL: https://github.com/llvm/llvm-project/commit/415b70a8a11d369176ea2db5151424d9354ba41a DIFF: https://github.com/llvm/llvm-project/commit/415b70a8a11d369176ea2db5151424d9354ba41a.diff LOG: [clang-repl] Handle mllvm args for clang-repl before calling executeAction (#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) ``` Added: Modified: clang/include/clang/Frontend/CompilerInstance.h clang/include/clang/Interpreter/IncrementalExecutor.h clang/lib/Frontend/CompilerInstance.cpp clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp clang/lib/Interpreter/IncrementalExecutor.cpp clang/lib/Interpreter/Interpreter.cpp clang/lib/Interpreter/Wasm.cpp clang/lib/Interpreter/Wasm.h clang/unittests/Interpreter/CMakeLists.txt clang/unittests/Interpreter/InterpreterTest.cpp clang/unittests/Interpreter/InterpreterTestFixture.h Removed: ################################################################################ diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 977180a8dc3aa..b87dd037480d6 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -264,6 +264,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 913da9230a947..f1a68c0288d00 100644 --- a/clang/include/clang/Interpreter/IncrementalExecutor.h +++ b/clang/include/clang/Interpreter/IncrementalExecutor.h @@ -52,6 +52,10 @@ 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 7048bec2cdceb..b09c9a9f5a5be 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1135,6 +1135,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 e872f0823f23e..24e699b42ca7f 100644 --- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp +++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp @@ -253,21 +253,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 5d01ba4774269..101120626f766 100644 --- a/clang/lib/Interpreter/IncrementalExecutor.cpp +++ b/clang/lib/Interpreter/IncrementalExecutor.cpp @@ -429,7 +429,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 40fbc83db9107..85cefa1270049 100644 --- a/clang/lib/Interpreter/Interpreter.cpp +++ b/clang/lib/Interpreter/Interpreter.cpp @@ -352,6 +352,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)); @@ -608,6 +611,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 573c8910d3a41..017b049fb470c 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> @@ -61,7 +62,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) @@ -142,6 +145,22 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) { if (!Result.canRunAgain) lld::exitLld(Result.retCode); + // 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. + // + // FIXME: Remove this once library command-line options no longer rely on + // process-global cl::opt state. See: + // https://discourse.llvm.org/t/rfc-declare-library-command-line-options-in-tablegen-one-struct-per-library/91877 + 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 6d999104054c3..d18d7819001b1 100644 --- a/clang/unittests/Interpreter/CMakeLists.txt +++ b/clang/unittests/Interpreter/CMakeLists.txt @@ -77,6 +77,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 a0ed7e671ddbe..17cc6fc9b6ca8 100644 --- a/clang/unittests/Interpreter/InterpreterTest.cpp +++ b/clang/unittests/Interpreter/InterpreterTest.cpp @@ -174,6 +174,9 @@ TEST_F(InterpreterTest, TranslationUnitRedeclChainAcrossManyPTUs) { } TEST_F(InterpreterTest, UndoLeavesDeclsInTranslationUnitChain) { +#ifdef __EMSCRIPTEN__ + GTEST_SKIP() << "Undo is not supported for Emscripten builds"; +#endif std::unique_ptr<Interpreter> Interp = createInterpreter(); cantFail(Interp->Parse("struct Kept {};")); @@ -520,6 +523,11 @@ TEST_F(InterpreterTest, ValueSetRawBitsCopiesByteCount) { // Earlier the move ctor called Release() on the just-moved-into storage, // double-releasing on the next read. TEST_F(InterpreterTest, ValueMoveSemantics) { + // FIXME: Emscripten cannot resolve MoveT's destructor symbol + // `_ZN5MoveTD2Ev` from the incrementally loaded Wasm side module. +#ifdef __EMSCRIPTEN__ + GTEST_SKIP() << "Unresolved destructor symbol: _ZN5MoveTD2Ev"; +#endif std::vector<const char *> Args = {"-fno-sized-deallocation"}; std::unique_ptr<Interpreter> Interp = createInterpreter(Args); @@ -588,4 +596,19 @@ 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
