OikawaKirie updated this revision to Diff 343626.
OikawaKirie added a comment.

In D101763#2743984 <https://reviews.llvm.org/D101763#2743984>, @steakhal wrote:

> By inspecting the output that is piped to the `count`, I have a suspicion:
>
>   openat(AT_FDCWD, "invocations.yaml", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No 
> such file or directory)

According to the output of the failed test, it seems that CTU is not working 
during the test case execution (`Expected 1 lines, got 0.`). Otherwise, it 
should be matched by grep.
There seems to be something wrong with the test case, although it works well in 
my environment.

> My guess is that `ctu-invocation-list` is not using the `ctu-dir`. It should 
> be a different bug though.
> To workaround this issue, simply use an absolute path for the invocation yaml.

It seems that you are correct, I will fix this problem later.
I can just `cd` to the `ctu-dir` before running CSA as a workaround. Just as 
the author of on-demand parsing did (refer to 
`clang/test/Analysis/ctu-on-demand-parsing.c`).

> `// CHECK: openat(AT_FDCWD, "{{[^"]+}}invocations.yaml", O_RDONLY|O_CLOEXEC) 
> = {{[0-9]+}}`

I am not quite sure whether we need to verify the flags (`AT_FDCWD`, `O_RDONLY` 
and `O_CLOEXEC`) as well. Therefore, I removed them in this update.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101763/new/

https://reviews.llvm.org/D101763

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/multiple-invocation-list-parsing.c

Index: clang/test/Analysis/multiple-invocation-list-parsing.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/multiple-invocation-list-parsing.c
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// 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
+// RUN: echo "" > %t/invocations.yaml
+
+// RUN: cd %t && \
+// RUN: strace -e trace=openat \
+// 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:   %s 2>&1 | grep 'invocations\.yaml' | FileCheck %s
+
+// REQUIRES: shell, system-linux
+
+void bar();
+
+void foo() {
+  // It finds the invocation yaml, then it tries to parse it which expected to fail.
+  bar();
+  // CHECK: openat({{[^,]+}}, "{{[^"]*/?}}invocations.yaml", {{[^)]+}}) = {{[0-9]+}}
+
+  // We should remember that the parsing failed, we shouldn't try to reopen and parse the yaml again.
+  bar();
+  // CHECK-NOT: openat({{[^,]+}}, "{{[^"]*/?}}invocations.yaml", {{[^)]+}}) = {{[0-9]+}}
+}
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===================================================================
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -92,6 +92,10 @@
 
   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 @@
   /// Lazily initialize the invocation list member used for on-demand parsing.
   if (InvocationList)
     return llvm::Error::success();
+  else 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::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;
 
Index: clang/include/clang/CrossTU/CrossTranslationUnit.h
===================================================================
--- clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -38,6 +38,7 @@
 namespace cross_tu {
 
 enum class index_error_code {
+  success = 0,
   unspecified = 1,
   missing_index_file,
   invalid_index_format,
@@ -253,6 +254,7 @@
     /// 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to