[clang-tools-extra] 9cc08cb - [clangd] Add integration test for crash handling
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 27 02:52:41 PDT 2021
Author: Sam McCall
Date: 2021-10-27T11:52:31+02:00
New Revision: 9cc08cb02fdc4f8a2b717519c3bf33b4ed3070e4
URL: https://github.com/llvm/llvm-project/commit/9cc08cb02fdc4f8a2b717519c3bf33b4ed3070e4
DIFF: https://github.com/llvm/llvm-project/commit/9cc08cb02fdc4f8a2b717519c3bf33b4ed3070e4.diff
LOG: [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
Added:
clang-tools-extra/clangd/test/crash-parse.test
clang-tools-extra/clangd/test/crash-preamble.test
Modified:
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp
index 809119cda6b1a..cdc29198d434f 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 b46765f6813ad..67b92c564550d 100644
--- a/clang-tools-extra/clangd/Compiler.h
+++ b/clang-tools-extra/clangd/Compiler.h
@@ -87,6 +87,10 @@ std::unique_ptr<CompilerInstance> prepareCompilerInstance(
std::unique_ptr<llvm::MemoryBuffer> MainFile,
IntrusiveRefCntPtr<llvm::vfs::FileSystem>, 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 8d696b00084fe..afaf6ed780b0c 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -589,6 +589,8 @@ class ASTWorker {
void startTask(llvm::StringRef Name, llvm::unique_function<void()> Task,
llvm::Optional<UpdateType> Update,
TUScheduler::ASTActionInvalidation);
+ /// Runs a task synchronously.
+ void runTask(llvm::StringRef Name, llvm::function_ref<void()> 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<CompilerInvocation> 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<void()> 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<void()> Task,
llvm::Optional<UpdateType> 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 0000000000000..7d4ed69167c24
--- /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 0000000000000..7d767208b9963
--- /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 4f8895b5d605c..c6cf5c3d6e9a3 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<bool> 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<bool> CrashPragmas{
+ "crash-pragmas",
+ cat(Misc),
+ desc("Respect `#pragma clang __debug crash` and friends."),
+ init(false),
+ Hidden,
+};
+
opt<Path> 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<TestScheme> 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 "
More information about the cfe-commits
mailing list