From: Sam McCall Date: Tue, 26 Oct 2021 13:27:07 +0000 (+0200) Subject: [clangd] Add integration test for crash handling X-Git-Tag: upstream/15.0.7~27523 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9cc08cb02fdc4f8a2b717519c3bf33b4ed3070e4;p=platform%2Fupstream%2Fllvm.git [clangd] Add integration test for crash handling This replaces the test removed in 51be7061d025139ba66869d5d99c7157a3ae9edd It is more principled and tests more critical cases: a crash while parsing. We need two pieces of plumbing: - a way to re-enable the crashing #pragmas via a flag, to test parse crashes - a bit of reshuffling around ASTWorker execution so that we set up the crash handler in both sync/async modes. Sync mode is useful for debugging, so I tested both. Differential Revision: https://reviews.llvm.org/D112565 --- diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index 809119c..cdc2919 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -42,6 +42,9 @@ void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, IgnoreDiagnostics::log(DiagLevel, Info); } +static bool AllowCrashPragmasForTest = false; +void allowCrashPragmasForTest() { AllowCrashPragmasForTest = true; } + void disableUnsupportedOptions(CompilerInvocation &CI) { // Disable "clang -verify" diagnostics, they are rarely useful in clangd, and // our compiler invocation set-up doesn't seem to work with it (leading @@ -66,7 +69,8 @@ void disableUnsupportedOptions(CompilerInvocation &CI) { CI.getPreprocessorOpts().PCHWithHdrStop = false; CI.getPreprocessorOpts().PCHWithHdrStopCreate = false; // Don't crash on `#pragma clang __debug parser_crash` - CI.getPreprocessorOpts().DisablePragmaDebugCrash = true; + if (!AllowCrashPragmasForTest) + CI.getPreprocessorOpts().DisablePragmaDebugCrash = true; // Always default to raw container format as clangd doesn't registry any other // and clang dies when faced with unknown formats. diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h index b46765f..67b92c5 100644 --- a/clang-tools-extra/clangd/Compiler.h +++ b/clang-tools-extra/clangd/Compiler.h @@ -87,6 +87,10 @@ std::unique_ptr prepareCompilerInstance( std::unique_ptr MainFile, IntrusiveRefCntPtr, DiagnosticConsumer &); +/// Respect `#pragma clang __debug crash` etc, which are usually disabled. +/// This may only be called before threads are spawned. +void allowCrashPragmasForTest(); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 8d696b0..afaf6ed 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -589,6 +589,8 @@ private: void startTask(llvm::StringRef Name, llvm::unique_function Task, llvm::Optional Update, TUScheduler::ASTActionInvalidation); + /// Runs a task synchronously. + void runTask(llvm::StringRef Name, llvm::function_ref Task); /// Determines the next action to perform. /// All actions that should never run are discarded. @@ -1025,7 +1027,7 @@ void ASTWorker::updatePreamble(std::unique_ptr CI, generateDiagnostics(std::move(CI), std::move(PI), std::move(CIDiags)); }; if (RunSync) { - Task(); + runTask(TaskName, Task); return; } { @@ -1195,15 +1197,23 @@ void ASTWorker::stop() { RequestsCV.notify_all(); } +void ASTWorker::runTask(llvm::StringRef Name, llvm::function_ref Task) { + ThreadCrashReporter ScopedReporter([this, Name]() { + llvm::errs() << "Signalled during AST worker action: " << Name << "\n"; + crashDumpParseInputs(llvm::errs(), FileInputs); + }); + trace::Span Tracer(Name); + WithContext WithProvidedContext(ContextProvider(FileName)); + Task(); +} + void ASTWorker::startTask(llvm::StringRef Name, llvm::unique_function Task, llvm::Optional Update, TUScheduler::ASTActionInvalidation Invalidation) { if (RunSync) { assert(!Done && "running a task after stop()"); - trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName)); - WithContext WithProvidedContext(ContextProvider(FileName)); - Task(); + runTask(Name, Task); return; } @@ -1323,18 +1333,11 @@ void ASTWorker::run() { Lock.lock(); } WithContext Guard(std::move(CurrentRequest->Ctx)); - trace::Span Tracer(CurrentRequest->Name); Status.update([&](TUStatus &Status) { Status.ASTActivity.K = ASTAction::RunningAction; Status.ASTActivity.Name = CurrentRequest->Name; }); - WithContext WithProvidedContext(ContextProvider(FileName)); - ThreadCrashReporter ScopedReporter([this]() { - const char *Name = CurrentRequest ? CurrentRequest->Name.c_str() : ""; - llvm::errs() << "Signalled during AST worker action: " << Name << "\n"; - crashDumpParseInputs(llvm::errs(), FileInputs); - }); - CurrentRequest->Action(); + runTask(CurrentRequest->Name, CurrentRequest->Action); } bool IsEmpty = false; diff --git a/clang-tools-extra/clangd/test/crash-parse.test b/clang-tools-extra/clangd/test/crash-parse.test new file mode 100644 index 0000000..7d4ed69 --- /dev/null +++ b/clang-tools-extra/clangd/test/crash-parse.test @@ -0,0 +1,19 @@ +# RUN: not --crash clangd -lit-test < %s 2> %t.err +# RUN: FileCheck %s < %t.err --check-prefixes=CHECK,CHECK-SYNC +# RUN: not --crash clangd -lit-test -sync=0 < %s 2> %t.async.err +# RUN: FileCheck %s < %t.async.err +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///foo.cc", + "languageId":"cpp", + "text":"int x;\n#pragma clang __debug llvm_fatal_error" +}}} +--- +{"jsonrpc":"2.0","id":1,"method":"sync","params":{}} +# CHECK: Signalled during AST worker action: Build AST +# CHECK-NEXT: Filename: foo.cc +# CHECK-SYNC: Signalled during AST worker action: Update +# CHECK-SYNC: Filename: foo.cc +# CHECK-SYNC: Signalled while processing message: +# CHECK-SYNC: "languageId":"cpp" diff --git a/clang-tools-extra/clangd/test/crash-preamble.test b/clang-tools-extra/clangd/test/crash-preamble.test new file mode 100644 index 0000000..7d76720 --- /dev/null +++ b/clang-tools-extra/clangd/test/crash-preamble.test @@ -0,0 +1,19 @@ +# RUN: not --crash clangd -lit-test < %s 2> %t.err +# RUN: FileCheck %s < %t.err --check-prefixes=CHECK,CHECK-SYNC +# RUN: not --crash clangd -lit-test -sync=0 < %s 2> %t.async.err +# RUN: FileCheck %s < %t.async.err +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{ + "uri":"test:///foo.cc", + "languageId":"cpp", + "text":"#pragma clang __debug llvm_fatal_error" +}}} +--- +{"jsonrpc":"2.0","id":1,"method":"sync","params":{}} +# CHECK: Signalled while building preamble +# CHECK-NEXT: Filename: foo.cc +# CHECK-SYNC: Signalled during AST worker action: Update +# CHECK-SYNC: Filename: foo.cc +# CHECK-SYNC: Signalled while processing message: +# CHECK-SYNC: "languageId":"cpp" diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 4f8895b..c6cf5c3 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -8,6 +8,7 @@ #include "ClangdLSPServer.h" #include "CodeComplete.h" +#include "Compiler.h" #include "Config.h" #include "ConfigProvider.h" #include "Feature.h" @@ -344,12 +345,20 @@ opt Test{ "lit-test", cat(Misc), desc("Abbreviation for -input-style=delimited -pretty -sync " - "-enable-test-scheme -enable-config=0 -log=verbose. " + "-enable-test-scheme -enable-config=0 -log=verbose -crash-pragmas. " "Intended to simplify lit tests"), init(false), Hidden, }; +opt CrashPragmas{ + "crash-pragmas", + cat(Misc), + desc("Respect `#pragma clang __debug crash` and friends."), + init(false), + Hidden, +}; + opt CheckFile{ "check", cat(Misc), @@ -707,7 +716,10 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var llvm::cl::ParseCommandLineOptions(argc, argv, Overview, /*Errs=*/nullptr, FlagsEnvVar); if (Test) { - Sync = true; + if (!Sync.getNumOccurrences()) + Sync = true; + if (!CrashPragmas.getNumOccurrences()) + CrashPragmas = true; InputStyle = JSONStreamStyle::Delimited; LogLevel = Logger::Verbose; PrettyPrint = true; @@ -725,6 +737,8 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var static URISchemeRegistry::Add X( "test", "Test scheme for clangd lit tests."); } + if (CrashPragmas) + allowCrashPragmasForTest(); if (!Sync && WorkerThreadsCount == 0) { llvm::errs() << "A number of worker threads cannot be 0. Did you mean to "