[PATCH] D55650: [clangd] Fix an assertion failure in background index.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.

When indexing a file which contains an uncompilable error, we will
trigger an assertion failure -- the IndexFileIn data is not set, but we
access them in the backgound index.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55650

Files:
  clangd/index/Background.cpp
  clangd/index/IndexAction.cpp
  unittests/clangd/BackgroundIndexTests.cpp


Index: unittests/clangd/BackgroundIndexTests.cpp
===================================================================
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -66,6 +66,25 @@
   BackgroundIndexTest() { preventThreadStarvationInTests(); }
 };
 
+TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.cc")] = "error file";
+  llvm::StringMap<std::string> Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+                      [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+}
+
 TEST_F(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
Index: clangd/index/IndexAction.cpp
===================================================================
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -134,22 +134,30 @@
   void EndSourceFileAction() override {
     WrapperFrontendAction::EndSourceFileAction();
 
+    SymbolSlab Symbols;
+    RefSlab Refs;
+    IncludeGraph IGraph;
     const auto &CI = getCompilerInstance();
-    if (CI.hasDiagnostics() &&
-        CI.getDiagnostics().hasUncompilableErrorOccurred()) {
-      errs() << "Skipping TU due to uncompilable errors\n";
-      return;
+    bool HasUncompilableErrors =
+        CI.hasDiagnostics() &&
+        CI.getDiagnostics().hasUncompilableErrorOccurred();
+    if (!CI.hasDiagnostics() || !HasUncompilableErrors) {
+      Symbols = Collector->takeSymbols();
+      Refs = Collector->takeRefs();
+      IGraph = std::move(IG);
     }
-    SymbolsCallback(Collector->takeSymbols());
-    if (RefsCallback != nullptr)
-      RefsCallback(Collector->takeRefs());
-    if (IncludeGraphCallback != nullptr) {
-#ifndef NDEBUG
+    assert(SymbolsCallback && "SymbolsCallback must be set.");
+    SymbolsCallback(std::move(Symbols));
+    if (RefsCallback)
+      RefsCallback(std::move(Refs));
+    if (IncludeGraphCallback) {
       // This checks if all nodes are initialized.
-      for (const auto &Node : IG)
-        assert(Node.getKeyData() == Node.getValue().URI.data());
-#endif
-      IncludeGraphCallback(std::move(IG));
+      assert(std::all_of(IGraph.begin(), IGraph.end(),
+                         [](const StringMapEntry<IncludeGraphNode> &Node) {
+                           return Node.getKeyData() ==
+                                  Node.getValue().URI.data();
+                         }));
+      IncludeGraphCallback(std::move(IGraph));
     }
   }
 
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -382,6 +382,9 @@
     return createStringError(inconvertibleErrorCode(), "Execute() failed");
   Action->EndSourceFile();
 
+  assert(Index.Symbols && Index.Refs && Index.Sources
+     && "Symbols, Refs and Sources must be set.");
+
   log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
       Inputs.CompileCommand.Filename, Index.Symbols->size(),
       Index.Refs->numRefs(), Index.Sources->size());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55650.178036.patch
Type: text/x-patch
Size: 3494 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181213/e0bf79dd/attachment-0001.bin>