[clangd] Add integration test for crash handling

This replaces the test removed in 51be706
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

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.

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
 

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;

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"

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"

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 "