https://github.com/hongtaihu created https://github.com/llvm/llvm-project/pull/209157
### Summary Empty `.i` files (and `-x cpp-output` with `.c`/`.cc`/`.m`/`.mm`) crash clangd with `"Compiler instance has no preprocessor!"` during shutdown, because `EndSourceFileAction()` unconditionally accesses the preprocessor for preprocessed inputs, but the preprocessor has been detached by `~ParsedAST()`. ### Root Cause `ParsedAST::~ParsedAST()` detaches the preprocessor from `CompilerInstance` (via `setPreprocessor(nullptr)`) before calling `EndSourceFile()`, to prevent a duplicate end-of-file notification. But `EndSourceFile()` calls `EndSourceFileAction()`, whose default implementation unconditionally calls `getPreprocessor()` for preprocessed inputs — hitting the assertion. The existing workaround in `~ParsedAST` was already marked `XXX: This is messy` — it manually detached the preprocessor to avoid repeated EOF. This patch replaces that with a clean flag-based approach. ### Fix Extract the preprocessor and diagnostic-client end-of-file notification into a new public method `EndSourceFileForDiagnostics()`, guarded by an `EndOfFileSignaled` flag to prevent double notification. - `EndSourceFile()` now calls `EndSourceFileForDiagnostics()` then proceeds with cleanup as before. External behavior unchanged. - clangd calls `EndSourceFileForDiagnostics()` at build time to flush clang-tidy diagnostics, then `EndSourceFile()` at destruction handles the rest. The flag prevents repeated EOF, so the `setPreprocessor(nullptr)` hack is no longer needed. - Since the preprocessor is never detached, `EndSourceFileAction()` can safely access it. ### Testing `ParsedASTTests.PreprocessedInputLifecycle` covers both the empty `.i` case and `-x c++-cpp-output`. Fixes #197662. >From 05f7bd70c2af55a9374e7515aca89a3ae8f3efcc Mon Sep 17 00:00:00 2001 From: hongtaihu <[email protected]> Date: Mon, 13 Jul 2026 20:13:13 +0800 Subject: [PATCH] [clang][FrontendAction] Extract EndSourceFileForDiagnostics to fix clangd assertion on preprocessed inputs (#197662) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When handling an empty .i file (or -x cpp-output with .c/.cc/.m/.mm), clangd crashes during shutdown with "Compiler instance has no preprocessor!" because ParsedAST::~ParsedAST() detaches the preprocessor before calling EndSourceFile(), but EndSourceFileAction() unconditionally accesses it for preprocessed inputs. The existing workaround in ~ParsedAST was already noted as messy — it manually detached the preprocessor via setPreprocessor(nullptr) to prevent duplicate EOF. This replaces that with a clean flag-based approach: - Extract PP.EndSourceFile() and DiagnosticClient.EndSourceFile() into a new public method EndSourceFileForDiagnostics(), guarded by an EndOfFileSignaled flag to prevent double notification. - EndSourceFile() internally calls EndSourceFileForDiagnostics() then proceeds with the rest of cleanup — external behavior unchanged. - clangd calls EndSourceFileForDiagnostics() at build time to flush clang-tidy diagnostics while retaining the AST. At destruction, EndSourceFile() sees the flag and skips repeated EOF, so the setPreprocessor(nullptr) hack is no longer needed. - Since PP is never detached, EndSourceFileAction() safely accesses it. Added ParsedASTTest.PreprocessedInputLifecycle covering both .i and -x c++-cpp-output. --- clang-tools-extra/clangd/ParsedAST.cpp | 19 +++++------------ .../clangd/unittests/ParsedASTTests.cpp | 11 ++++++++++ clang/include/clang/Frontend/FrontendAction.h | 9 ++++++++ clang/lib/Frontend/FrontendAction.cpp | 21 +++++++++++++------ 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index df56420cd7f24..5cfdc29d0a990 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -752,15 +752,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, CTFinder.matchAST(Clang->getASTContext()); } - // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF. - // However Action->EndSourceFile() would destroy the ASTContext! - // So just inform the preprocessor of EOF, while keeping everything alive. - PP.EndSourceFile(); + // Clang-tidy checks flush some diagnostics at EOF, but we need to retain the + // AST until ParsedAST is destroyed. + Action->EndSourceFileForDiagnostics(); // UnitDiagsConsumer is local, we can not store it in CompilerInstance that // has a longer lifetime. Clang->getDiagnostics().setClient(new IgnoreDiagnostics); - // CompilerInstance won't run this callback, do it directly. - ASTDiags.EndSourceFile(); std::vector<Diag> Diags = CompilerInvocationDiags; // FIXME: Also skip generation of diagnostics altogether to speed up ast @@ -787,14 +784,8 @@ ParsedAST::ParsedAST(ParsedAST &&Other) = default; ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default; ParsedAST::~ParsedAST() { - if (Action) { - // We already notified the PP of end-of-file earlier, so detach it first. - // We must keep it alive until after EndSourceFile(), Sema relies on this. - auto PP = Clang->getPreprocessorPtr(); // Keep PP alive for now. - Clang->setPreprocessor(nullptr); // Detach so we don't send EOF again. - Action->EndSourceFile(); // Destroy ASTContext and Sema. - // Now Sema is gone, it's safe for PP to go out of scope. - } + if (Action) + Action->EndSourceFile(); // Destroy ASTContext and Sema. } ASTContext &ParsedAST::getASTContext() { return Clang->getASTContext(); } diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index f9752d5d44f97..3b07f31230547 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -266,6 +266,17 @@ TEST(ParsedASTTest, NoCrashOnTokensWithTidyCheck) { EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}"); } +TEST(ParsedASTTest, PreprocessedInputLifecycle) { + TestTU TU; + TU.Filename = "TestTU.i"; + TU.Code = ""; + TU.build(); + + TU.Filename = "TestTU.cc"; + TU.ExtraArgs = {"-x", "c++-cpp-output"}; + TU.build(); +} + TEST(ParsedASTTest, CanBuildInvocationWithUnknownArgs) { MockFS FS; FS.Files = {{testPath("foo.cpp"), "void test() {}"}}; diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h index 08c5fbc78f8ae..1b7f13f55a8d6 100644 --- a/clang/include/clang/Frontend/FrontendAction.h +++ b/clang/include/clang/Frontend/FrontendAction.h @@ -38,6 +38,7 @@ class FrontendAction { FrontendInputFile CurrentInput; std::unique_ptr<ASTUnit> CurrentASTUnit; CompilerInstance *Instance; + bool EndOfFileSignaled = false; friend class ASTMergeAction; friend class WrapperFrontendAction; @@ -244,6 +245,14 @@ class FrontendAction { /// Set the source manager's main input file, and run the action. llvm::Error Execute(); + /// Notify the preprocessor and diagnostic client that the source file has + /// ended, without destroying per-file frontend state. + /// + /// This is useful for clients that need end-of-file diagnostics while + /// retaining the AST. The matching EndSourceFile() performs the remaining + /// cleanup and will not notify them a second time. + void EndSourceFileForDiagnostics(); + /// Perform any per-file post processing, deallocate per-file /// objects, and run statistics and output file cleanup code. virtual void EndSourceFile(); diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 55e3877384622..f6f0ff344517f 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -831,6 +831,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, assert(!Input.isEmpty() && "Unexpected empty filename!"); setCurrentInput(Input); setCompilerInstance(&CI); + EndOfFileSignaled = false; bool HasBegunSourceFile = false; bool ReplayASTFile = Input.getKind().getFormat() == InputKind::Precompiled && @@ -1362,17 +1363,25 @@ llvm::Error FrontendAction::Execute() { return llvm::Error::success(); } -void FrontendAction::EndSourceFile() { +void FrontendAction::EndSourceFileForDiagnostics() { CompilerInstance &CI = getCompilerInstance(); - // Inform the preprocessor we are done. + if (EndOfFileSignaled) + return; + + // Inform the preprocessor and diagnostic client we are done with this source + // file. Do this in order so that end-of-file preprocessor callbacks can + // report diagnostics. if (CI.hasPreprocessor()) CI.getPreprocessor().EndSourceFile(); - - // Inform the diagnostic client we are done with this source file. - // Do this after notifying the preprocessor, so that end-of-file preprocessor - // callbacks can report diagnostics. CI.getDiagnosticClient().EndSourceFile(); + EndOfFileSignaled = true; +} + +void FrontendAction::EndSourceFile() { + CompilerInstance &CI = getCompilerInstance(); + + EndSourceFileForDiagnostics(); // Finalize the action. EndSourceFileAction(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
