[DependencyScanningFilesystem] Make sure the local/shared cache filen…

…ame lookups use only absolute paths (llvm#66122)

Previously a relative path would be used as a key for cache lookup and
if the same relative path was used from another compiler invocation with
a different working directory then the first cache entry was erroneously
returned.

(cherry picked from commit 36b37c7)

clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

@@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache {
 public:
  /// Returns entry associated with the filename or nullptr if none is found.
  const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
  auto It = Cache.find(Filename);
  return It == Cache.end() ? nullptr : It->getValue();
  }
@@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache {
  const CachedFileSystemEntry &
  insertEntryForFilename(StringRef Filename,
  const CachedFileSystemEntry &Entry) {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
  const auto *InsertedEntry = Cache.insert({Filename, &Entry}).first->second;
  assert(InsertedEntry == &Entry && "entry already present");
  return *InsertedEntry;
@@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
 public:
  DependencyScanningWorkerFilesystem(
  DependencyScanningFilesystemSharedCache &SharedCache,
- IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
- : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {}
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
 
  llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
  llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
  openFileForRead(const Twine &Path) override;
 
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
+
  /// Returns entry for the given filename.
  ///
  /// Attempts to use the local and shared caches first, then falls back to
@@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
  /// For a filename that's not yet associated with any entry in the caches,
  /// uses the underlying filesystem to either look up the entry based in the
  /// shared cache indexed by unique ID, or creates new entry from scratch.
+ /// \p FilenameForLookup will always be an absolute path, and different than
+ /// \p OriginalFilename if \p OriginalFilename is relative.
  llvm::ErrorOr<const CachedFileSystemEntry &>
- computeAndStoreResult(StringRef Filename);
+ computeAndStoreResult(StringRef OriginalFilename,
+ StringRef FilenameForLookup);
 
  /// Scan for preprocessor directives for the given entry if necessary and
  /// returns a wrapper object with reference semantics.
@@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem {
  /// The local cache is used by the worker thread to cache file system queries
  /// locally instead of querying the global cache every time.
  DependencyScanningFilesystemLocalCache LocalCache;
+
+ /// The working directory to use for making relative paths absolute before
+ /// using them for cache lookups.
+ llvm::ErrorOr<std::string> WorkingDirForCacheLookup;
+
+ void updateWorkingDirForCacheLookup();
 };
 
 } // end namespace dependencies

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

@@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache::
 DependencyScanningFilesystemSharedCache::CacheShard &
 DependencyScanningFilesystemSharedCache::getShardForFilename(
  StringRef Filename) const {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
  return CacheShards[llvm::hash_value(Filename) % NumShards];
 }
 
@@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
 const CachedFileSystemEntry *
 DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename(
  StringRef Filename) const {
+ assert(llvm::sys::path::is_absolute_gnu(Filename));
  std::lock_guard<std::mutex> LockGuard(CacheLock);
  auto It = EntriesByFilename.find(Filename);
  return It == EntriesByFilename.end() ? nullptr : It->getValue();
@@ -189,6 +191,14 @@ static bool shouldCacheStatFailures(StringRef Filename) {
  return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 
+DependencyScanningWorkerFilesystem::DependencyScanningWorkerFilesystem(
+ DependencyScanningFilesystemSharedCache &SharedCache,
+ IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+ : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache),
+ WorkingDirForCacheLookup(llvm::errc::invalid_argument) {
+ updateWorkingDirForCacheLookup();
+}
+
 bool DependencyScanningWorkerFilesystem::shouldScanForDirectives(
  StringRef Filename) {
  return shouldScanForDirectivesBasedOnExtension(Filename);
@@ -215,44 +225,62 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough(
 }
 
 llvm::ErrorOr<const CachedFileSystemEntry &>
-DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) {
- llvm::ErrorOr<llvm::vfs::Status> Stat = getUnderlyingFS().status(Filename);
+DependencyScanningWorkerFilesystem::computeAndStoreResult(
+ StringRef OriginalFilename, StringRef FilenameForLookup) {
+ llvm::ErrorOr<llvm::vfs::Status> Stat =
+ getUnderlyingFS().status(OriginalFilename);
  if (!Stat) {
- if (!shouldCacheStatFailures(Filename))
+ if (!shouldCacheStatFailures(OriginalFilename))
  return Stat.getError();
  const auto &Entry =
- getOrEmplaceSharedEntryForFilename(Filename, Stat.getError());
- return insertLocalEntryForFilename(Filename, Entry);
+ getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError());
+ return insertLocalEntryForFilename(FilenameForLookup, Entry);
  }
 
  if (const auto *Entry = findSharedEntryByUID(*Stat))
- return insertLocalEntryForFilename(Filename, *Entry);
+ return insertLocalEntryForFilename(FilenameForLookup, *Entry);
 
  auto TEntry =
- Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename);
+ Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename);
 
  const CachedFileSystemEntry *SharedEntry = [&]() {
  if (TEntry) {
  const auto &UIDEntry = getOrEmplaceSharedEntryForUID(std::move(*TEntry));
- return &getOrInsertSharedEntryForFilename(Filename, UIDEntry);
+ return &getOrInsertSharedEntryForFilename(FilenameForLookup, UIDEntry);
  }
- return &getOrEmplaceSharedEntryForFilename(Filename, TEntry.getError());
+ return &getOrEmplaceSharedEntryForFilename(FilenameForLookup,
+ TEntry.getError());
  }();
 
- return insertLocalEntryForFilename(Filename, *SharedEntry);
+ return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry);
 }
 
 llvm::ErrorOr<EntryRef>
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
- StringRef Filename, bool DisableDirectivesScanning) {
- if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename))
- return scanForDirectivesIfNecessary(*Entry, Filename,
+ StringRef OriginalFilename, bool DisableDirectivesScanning) {
+ StringRef FilenameForLookup;
+ SmallString<256> PathBuf;
+ if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) {
+ FilenameForLookup = OriginalFilename;
+ } else if (!WorkingDirForCacheLookup) {
+ return WorkingDirForCacheLookup.getError();
+ } else {
+ StringRef RelFilename = OriginalFilename;
+ RelFilename.consume_front("./");
+ PathBuf = *WorkingDirForCacheLookup;
+ llvm::sys::path::append(PathBuf, RelFilename);
+ FilenameForLookup = PathBuf.str();
+ }
+ assert(llvm::sys::path::is_absolute_gnu(FilenameForLookup));
+ if (const auto *Entry =
+ findEntryByFilenameWithWriteThrough(FilenameForLookup))
+ return scanForDirectivesIfNecessary(*Entry, OriginalFilename,
  DisableDirectivesScanning)
  .unwrapError();
- auto MaybeEntry = computeAndStoreResult(Filename);
+ auto MaybeEntry = computeAndStoreResult(OriginalFilename, FilenameForLookup);
  if (!MaybeEntry)
  return MaybeEntry.getError();
- return scanForDirectivesIfNecessary(*MaybeEntry, Filename,
+ return scanForDirectivesIfNecessary(*MaybeEntry, OriginalFilename,
  DisableDirectivesScanning)
  .unwrapError();
 }
@@ -330,3 +358,24 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path) {
  return Result.getError();
  return DepScanFile::create(Result.get());
 }
+
+std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory(
+ const Twine &Path) {
+ std::error_code EC = ProxyFileSystem::setCurrentWorkingDirectory(Path);
+ updateWorkingDirForCacheLookup();
+ return EC;
+}
+
+void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup() {
+ llvm::ErrorOr<std::string> CWD =
+ getUnderlyingFS().getCurrentWorkingDirectory();
+ if (!CWD) {
+ WorkingDirForCacheLookup = CWD.getError();
+ } else if (!llvm::sys::path::is_absolute_gnu(*CWD)) {
+ WorkingDirForCacheLookup = llvm::errc::invalid_argument;
+ } else {
+ WorkingDirForCacheLookup = *CWD;
+ }
+ assert(!WorkingDirForCacheLookup ||
+ llvm::sys::path::is_absolute_gnu(*WorkingDirForCacheLookup));
+}

clang/test/ClangScanDeps/relative-filenames.c

@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format make -j 1 > %t/result.txt
+// RUN: FileCheck %s -input-file=%t/result.txt
+
+// CHECK: {{/|\\}}dir1{{/|\\}}t1.c
+// CHECK: {{/|\\}}dir1{{/|\\}}head.h
+// CHECK: {{/|\\}}dir2{{/|\\}}t2.c
+// CHECK: {{/|\\}}dir2{{/|\\}}head.h
+
+//--- cdb.json.template
+[
+ {
+ "directory": "DIR/dir1",
+ "command": "clang -fsyntax-only t1.c",
+ "file": "t1.c"
+ },
+ {
+ "directory": "DIR/dir2",
+ "command": "clang -fsyntax-only t2.c",
+ "file": "t2.c"
+ }
+]
+
+//--- dir1/t1.c
+#include "head.h"
+
+//--- dir1/head.h
+#ifndef BBB
+#define BBB
+#endif
+
+//--- dir2/t2.c
+#include "head.h"
+
+//--- dir2/head.h