[analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths

[OikawaKirie]

Summary

This patch fixes the problem mentioned in D101763#2743984.
When the ctu-invocation-list is not an absolute path and the current working directory is not the ctu-dir, the invocation list will fail to be loaded.
In this patch, the ctu-dir is appended to the ctu-invocation-list if the ctu-invocation-list is not an absolute path.
And the original test cases for on-demand parsing are reused to test this feature.

[steakhal]

On second thought the current behavior is reasonable.
We call clang from a command line, so I think it's fair to expect that relative paths are resolved using the CWD.
AFAIK if one does not supply the ctu-invocation-list, then it would be substituted by ctu-dir/invocations.yaml anyway.

[OikawaKirie]

In D102062#2744267, @steakhal wrote:

On second thought the current behavior is reasonable.

The behavior of all CTU affairs are related to the ctu-dir rather than CWD, such as loading the external function map file and looking for the AST dumps and source files to be imported.
If a relative path is provided for these files, the ctu-dir will be added to the path when trying to open the file (see the comments in the code).
Therefore, IMO the behavior of ctu-invocation-list and its contents should also be relative to the ctu-dir unless an absolute path is provided.

We call clang from a command line, so I think it's fair to expect that relative paths are resolved using the CWD.

I do not know how other people use CSA, but I prefer using CSA via clang-check.
It works with the relative path to the "directory" option in the compilation database of each source file, which could make the CWD not unique.

AFAIK if one does not supply the ctu-invocation-list, then it would be substituted by ctu-dir/invocations.yaml anyway.

But we cannot currently distinguish the difference between setting ctu-invocation-list to invocations.yaml and not setting it. As the default value of ctu-invocation-list is invocations.yaml rather than an empty string.
Therefore, adding the ctu-dir to a relative ctu-invocation-list path by default seems to be a better choice.

clang/lib/CrossTU/CrossTranslationUnit.cpp

470

Loading the external function map.

516

Loading the ASTUnit from external AST dumps or source files.

[martong]

First of all, thank you for the patch!
We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following.

We'd like to keep the current behavior because this way the behavior is similar that we got used to with any other command line tools.

[OikawaKirie]

In D102062#2768569, @martong wrote:

First of all, thank you for the patch!
We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the following.

We'd like to keep the current behavior because this way the behavior is similar that we got used to with any other command line tools.

Ok, if you all agree with using ctu-invocation-list with a path related to CWD rather than ctu-dir, I will drop this revision here.
Although, I still believe that it would be better to make the behavior similar to the ctu index file, which is related to the ctu-dir (see the comments in the code).

Thanks for your suggestions on this revision.

clang/lib/CrossTU/CrossTranslationUnit.cpp

llvm::Error CrossTranslationUnitContext::ASTUnitStorage::ensureCTUIndexLoaded(
   if (!NameFileMap.empty())
     return llvm::Error::success();
 
   // Get the absolute path to the index file.
   SmallString<256> IndexFile = CrossTUDir;
   if (llvm::sys::path::is_absolute(IndexName))
     IndexFile = IndexName;
   else
     llvm::sys::path::append(IndexFile, IndexName);
 
   if (auto IndexMapping = parseCrossTUIndex(IndexFile)) {
     // Initialize member map.
     NameFileMap = *IndexMapping;
     return llvm::Error::success();
   } else {
     // Error while parsing CrossTU index file.
     return IndexMapping.takeError();

 
 CrossTranslationUnitContext::LoadResultTy
 CrossTranslationUnitContext::ASTLoader::load(StringRef Identifier) {
   llvm::SmallString<256> Path;
   if (llvm::sys::path::is_absolute(Identifier, PathStyle)) {
     Path = Identifier;
   } else {
     Path = CTUDir;
     llvm::sys::path::append(Path, PathStyle, Identifier);
   }
 
   // The path is stored in the InvocationList member in posix style. To
   // successfully lookup an entry based on filepath, it must be converted.
   llvm::sys::path::native(Path, PathStyle);
 
   // Normalize by removing relative path components.
   llvm::sys::path::remove_dots(Path, /*remove_dot_dot*/ true, PathStyle);
parseInvocationList(StringRef FileContent, llvm::sys::path::Style PathStyle) {
   return InvocationList;
 }
 
 llvm::Error CrossTranslationUnitContext::ASTLoader::lazyInitInvocationList() {
   /// Lazily initialize the invocation list member used for on-demand parsing.
   if (InvocationList)
     return llvm::Error::success();
 
+  SmallString<256> InvocationListFileAbsolutePath = CTUDir;
+  if (llvm::sys::path::is_absolute(InvocationListFilePath))
+    InvocationListFileAbsolutePath = InvocationListFilePath;
+  else
+    llvm::sys::path::append(InvocationListFileAbsolutePath,
+                            InvocationListFilePath);
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> FileContent =
-      llvm::MemoryBuffer::getFile(InvocationListFilePath);
+      llvm::MemoryBuffer::getFile(InvocationListFileAbsolutePath);
   if (!FileContent)
     return llvm::make_error<IndexError>(
         index_error_code::invocation_list_file_not_found);
   std::unique_ptr<llvm::MemoryBuffer> ContentBuffer = std::move(*FileContent);
   assert(ContentBuffer && "If no error was produced after loading, the pointer "
                           "should not be nullptr.");
 
   llvm::Expected<InvocationListTy> ExpectedInvocationList =

clang/test/Analysis/ctu-on-demand-parsing.c

 // RUN: rm -rf %t
 // RUN: mkdir -p %t
 // RUN: cp "%s" "%t/ctu-on-demand-parsing.c"
 // RUN: cp "%S/Inputs/ctu-other.c" "%t/ctu-other.c"
 //
 // Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
 // compile_commands.json is only needed for extdef_mapping, not for the analysis itself.
 // RUN: echo '[{"directory":"%t","command":"gcc -std=c89 -Wno-visibility ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\/\\\\/g' > %t/compile_commands.json
 //
-// RUN: echo '"%t/ctu-other.c": ["gcc", "-std=c89", "-Wno-visibility", "ctu-other.c"]' | sed -e 's/\\/\\\\/g' > %t/invocations.yaml
+// RUN: echo '"%t/ctu-other.c": ["gcc", "-std=c89", "-Wno-visibility", "%t/ctu-other.c"]' | sed -e 's/\\/\\\\/g' > %t/invocations.yaml
 //
-// RUN: cd "%t" && %clang_extdef_map "%t/ctu-other.c" > externalDefMap.txt
+// RUN: %clang_extdef_map -p %t "%t/ctu-other.c" > %t/externalDefMap.txt
 //
-// RUN: cd "%t" && %clang_cc1 -fsyntax-only -std=c89 -analyze \
+// RUN: %clang_cc1 -fsyntax-only -std=c89 -analyze \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
-// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-dir=%t \
 // RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
-// RUN:   -verify ctu-on-demand-parsing.c
+// RUN:   -verify %t/ctu-on-demand-parsing.c
 //
 // FIXME: Path handling should work on all platforms.
 // REQUIRES: system-linux
 
 void clang_analyzer_eval(int);
 
 // Test typedef and global variable in function.
 typedef struct {

clang/test/Analysis/ctu-on-demand-parsing.cpp

 // RUN: rm -rf %t
 // RUN: mkdir -p %t/Inputs
 // RUN: cp %s %t/ctu-on-demand-parsing.cpp
 // RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
 // RUN: cp %S/Inputs/ctu-chain.cpp %t/Inputs/ctu-chain.cpp
 // RUN: cp %S/Inputs/ctu-other.cpp %t/Inputs/ctu-other.cpp
 //
 // Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
 // compile_commands.json is only needed for the extdef_mapping, not for the analysis itself.
 // RUN: echo '[{"directory":"%t/Inputs","command":"clang++ ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++ ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\/\\\\/g' > %t/compile_commands.json
 //
 // RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], "%t/Inputs/ctu-other.cpp": ["g++", "%t/Inputs/ctu-other.cpp"]}' | sed -e 's/\\/\\\\/g' > %t/invocations.yaml
 //
-// RUN: cd "%t" && %clang_extdef_map Inputs/ctu-chain.cpp Inputs/ctu-other.cpp > externalDefMap.txt
+// RUN: %clang_extdef_map -p %t %t/Inputs/ctu-chain.cpp %t/Inputs/ctu-other.cpp > %t/externalDefMap.txt
 //
-// RUN: cd "%t" && %clang_analyze_cc1 \
+// RUN: %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
-// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-dir=%t \
 // RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
-// RUN:   -verify ctu-on-demand-parsing.cpp
-// RUN: cd "%t" && %clang_analyze_cc1 \
+// RUN:   -verify %t/ctu-on-demand-parsing.cpp
+// RUN: %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
-// RUN:   -analyzer-config ctu-dir=. \
+// RUN:   -analyzer-config ctu-dir=%t \
 // RUN:   -analyzer-config ctu-invocation-list=invocations.yaml \
-// RUN:   -analyzer-config display-ctu-progress=true ctu-on-demand-parsing.cpp 2>&1 | FileCheck %t/ctu-on-demand-parsing.cpp
+// RUN:   -analyzer-config display-ctu-progress=true %t/ctu-on-demand-parsing.cpp 2>&1 | FileCheck %t/ctu-on-demand-parsing.cpp
 //
 // CHECK: CTU loaded AST file: {{.*}}ctu-other.cpp
 // CHECK: CTU loaded AST file: {{.*}}ctu-chain.cpp
 //
 // FIXME: Path handling should work on all platforms.
 // REQUIRES: system-linux
 
 #include "ctu-hdr.h"