[clang] db8af0f - [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

[Balazs Benics via cfe-commits]

Author: Ella Ma
Date: 2021-05-25T09:19:14+02:00
New Revision: db8af0f21dc9aad4d336754c857c24470afe53e3

URL: https://github.com/llvm/llvm-project/commit/db8af0f21dc9aad4d336754c857c24470afe53e3
DIFF: https://github.com/llvm/llvm-project/commit/db8af0f21dc9aad4d336754c857c24470afe53e3.diff

LOG: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

During CTU, the *on-demand parsing* will read and parse the invocation
list to know how to compile the file being imported. However, it seems
that the invocation list will be parsed again if a previous parsing
has failed.
Then, parse again and fail again. This patch tries to overcome the
problem by storing the error code during the first parsing, and
re-create the stored error during the later parsings.

Reviewed By: steakhal

Patch By: OikawaKirie!

Differential Revision: https://reviews.llvm.org/D101763

Added: 
    clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp

Modified: 
    clang/include/clang/CrossTU/CrossTranslationUnit.h
    clang/lib/CrossTU/CrossTranslationUnit.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/CrossTU/CrossTranslationUnit.h b/clang/include/clang/CrossTU/CrossTranslationUnit.h
index b419b9be67cb9..d9f9c51fccd9c 100644
--- a/clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ b/clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -38,6 +38,7 @@ class TranslationUnitDecl;
 namespace cross_tu {
 
 enum class index_error_code {
+  success = 0,
   unspecified = 1,
   missing_index_file,
   invalid_index_format,
@@ -253,6 +254,7 @@ class CrossTranslationUnitContext {
     /// In case of on-demand parsing, the invocations for parsing the source
     /// files is stored.
     llvm::Optional<InvocationListTy> InvocationList;
+    index_error_code PreviousParsingResult = index_error_code::success;
   };
 
   /// Maintain number of AST loads and check for reaching the load limit.

diff  --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index 640538cae3bbc..adee55304c871 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -92,6 +92,10 @@ class IndexErrorCategory : public std::error_category {
 
   std::string message(int Condition) const override {
     switch (static_cast<index_error_code>(Condition)) {
+    case index_error_code::success:
+      // There should not be a success error. Jump to unreachable directly.
+      // Add this case to make the compiler stop complaining.
+      break;
     case index_error_code::unspecified:
       return "An unknown error has occurred.";
     case index_error_code::missing_index_file:
@@ -667,12 +671,15 @@ llvm::Error CrossTranslationUnitContext::ASTLoader::lazyInitInvocationList() {
   /// Lazily initialize the invocation list member used for on-demand parsing.
   if (InvocationList)
     return llvm::Error::success();
+  if (index_error_code::success != PreviousParsingResult)
+    return llvm::make_error<IndexError>(PreviousParsingResult);
 
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> FileContent =
       llvm::MemoryBuffer::getFile(InvocationListFilePath);
-  if (!FileContent)
-    return llvm::make_error<IndexError>(
-        index_error_code::invocation_list_file_not_found);
+  if (!FileContent) {
+    PreviousParsingResult = index_error_code::invocation_list_file_not_found;
+    return llvm::make_error<IndexError>(PreviousParsingResult);
+  }
   std::unique_ptr<llvm::MemoryBuffer> ContentBuffer = std::move(*FileContent);
   assert(ContentBuffer && "If no error was produced after loading, the pointer "
                           "should not be nullptr.");
@@ -680,8 +687,13 @@ llvm::Error CrossTranslationUnitContext::ASTLoader::lazyInitInvocationList() {
   llvm::Expected<InvocationListTy> ExpectedInvocationList =
       parseInvocationList(ContentBuffer->getBuffer(), PathStyle);
 
-  if (!ExpectedInvocationList)
-    return ExpectedInvocationList.takeError();
+  // Handle the error to store the code for next call to this function.
+  if (!ExpectedInvocationList) {
+    llvm::handleAllErrors(
+        ExpectedInvocationList.takeError(),
+        [&](const IndexError &E) { PreviousParsingResult = E.getCode(); });
+    return llvm::make_error<IndexError>(PreviousParsingResult);
+  }
 
   InvocationList = *ExpectedInvocationList;
 

diff  --git a/clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp b/clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
new file mode 100644
index 0000000000000..7121a798e3c27
--- /dev/null
+++ b/clang/test/Analysis/ctu-on-demand-parsing-multiple-invocation-list-parsing.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %host_cxx %s -fPIC -shared -o %t/mock_open.so
+
+// RUN: echo "void bar(); void foo() { bar(); bar(); }" > %t/trigger.c
+// RUN: echo "void bar() {}" > %t/importee.c
+// RUN: echo '[{"directory":"%t", "command":"cc -c %t/importee.c", "file": "%t/importee.c"}]' > %t/compile_commands.json
+// RUN: %clang_extdef_map -p %t "%t/importee.c" > %t/externalDefMap.txt
+
+// Add an empty invocation list to make the on-demand parsing fail and load it again.
+// RUN: echo '' > %t/invocations.yaml
+
+// RUN: cd %t && \
+// RUN: LD_PRELOAD=%t/mock_open.so \
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN:   %t/trigger.c | FileCheck %s
+
+// REQUIRES: shell, system-linux
+
+// CHECK: {{Opening file invocations.yaml: 1}}
+// CHECK-NOT: {{Opening file invocations.yaml: 2}}
+
+#define _GNU_SOURCE 1
+#include <dlfcn.h>
+#include <fcntl.h>
+
+#include <cassert>
+#include <cstdarg>
+#include <iostream>
+using namespace std;
+
+extern "C" int open(const char *name, int flag, ...) {
+  // Log how many times the invocation list is opened.
+  if ("invocations.yaml" == string(name)) {
+    static unsigned N = 0;
+    cout << "Opening file invocations.yaml: " << ++N << endl;
+  }
+
+  // The original open function will be called to open the files.
+  using open_t = int (*)(const char *, int, mode_t);
+  static open_t o_open = nullptr;
+  if (!o_open)
+    o_open = reinterpret_cast<open_t>(dlsym(RTLD_NEXT, "open"));
+  assert(o_open && "Cannot find function `open'.");
+
+  va_list vl;
+  va_start(vl, flag);
+  auto mode = va_arg(vl, mode_t);
+  va_end(vl);
+  return o_open(name, flag, mode);
+}