OikawaKirie updated this revision to Diff 343346.
OikawaKirie added a comment.
In D101763#2741536 <https://reviews.llvm.org/D101763#2741536>, @steakhal wrote:
> Overall, it looks promising. But I don't quite get this test.
> There is no invocation yaml in the temp directory. So, you are probably not
> testing the right thing.
> You wanted to test if the invocation yaml exists, and could be opened **but
> the parsing fails**.
> You should demonstrate that when a parsing error happens, the error code has
> recoded and it won't try to reparse the invocation yaml again and again.
The main idea of the test case is that the mocked `open` function will dump a
log every time when the invocation list file is opened to be read. If a second
open operation is detected in the log, it means the list is parsed again.
As the behavior of missing the invocation list file or failing to parse it are
the same (both returns the error), the previous version checks whether function
`open` will be called again after a file-not-found error.
In this update, I use an empty invocation list to really trigger an
`invocation_list_empty` error and check whether the invocation list file will
be read again via function `open`.
Compared with the previous version, opening the invocation list will not be
manually failed, and a real parsing error is triggered instead.
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.cpp
Index: clang/test/Analysis/multiple-invocation-list-parsing.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/multiple-invocation-list-parsing.cpp
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// Compile the mocked `open` function.
+// RUN: %host_cxx %s -fPIC -shared -o %t/mock_open.so
+
+// RUN: echo "void bar(); void foo() { bar(); bar(); }" > %t/importer.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
+// RUN: echo "" > %t/invocations.yaml
+
+// Run the test code.
+// 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=%t \
+// RUN: -analyzer-config ctu-invocation-list=invocations.yaml \
+// RUN: %t/importer.c | FileCheck %s
+
+// REQUIRES: shell, system-linux
+
+// Check the log for the second open of the invocation list.
+// CHECK: {{Mocking file invocations.yaml: 1}}
+// CHECK-NOT: {{Mocking 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 << "Mocking 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);
+}
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===================================================================
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -667,12 +667,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 +683,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
