[clang] 9ef9167 - [clang][Dependency Scanning] Adding an API to Diagnose Invalid Negative Stat Cache Entries (#135703)

[via cfe-commits]

Author: Qiongsi Wu
Date: 2025-04-18T10:52:39-07:00
New Revision: 9ef91677a4a2d4b017d2b96b8c89c2a9b648d6c3

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

LOG: [clang][Dependency Scanning] Adding an API to Diagnose Invalid Negative Stat Cache Entries (#135703)

We have had numerous situations where the negatively stat cached paths
are invalidated during the build, and such invalidations lead to build
errors.

This PR adds an API to diagnose such cases.
`DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths`
allows users of the cache to ask the cache to examine all negatively
stat cached paths, and re-stat the paths using the passed-in file
system. If any re-stat succeeds, the API emits diagnostics.

rdar://149147920

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
    clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index d12814e7c9253..74b40f7452edb 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -220,6 +220,14 @@ class DependencyScanningFilesystemSharedCache {
   CacheShard &getShardForFilename(StringRef Filename) const;
   CacheShard &getShardForUID(llvm::sys::fs::UniqueID UID) const;
 
+  /// Visits all cached entries and re-stat an entry using FS if
+  /// it is negatively stat cached. If re-stat succeeds on a path,
+  /// the path is added to InvalidPaths, indicating that the cache
+  /// may have erroneously negatively cached it. The caller can then
+  /// use InvalidPaths to issue diagnostics.
+  std::vector<StringRef>
+  getInvalidNegativeStatCachedPaths(llvm::vfs::FileSystem &UnderlyingFS) const;
+
 private:
   std::unique_ptr<CacheShard[]> CacheShards;
   unsigned NumShards;

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 4d738e4bea41a..191b6ef439860 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -108,6 +108,33 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
   return CacheShards[Hash % NumShards];
 }
 
+std::vector<StringRef>
+DependencyScanningFilesystemSharedCache::getInvalidNegativeStatCachedPaths(
+    llvm::vfs::FileSystem &UnderlyingFS) const {
+  // Iterate through all shards and look for cached stat errors.
+  std::vector<StringRef> InvalidPaths;
+  for (unsigned i = 0; i < NumShards; i++) {
+    const CacheShard &Shard = CacheShards[i];
+    std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
+    for (const auto &[Path, CachedPair] : Shard.CacheByFilename) {
+      const CachedFileSystemEntry *Entry = CachedPair.first;
+
+      if (Entry->getError()) {
+        // Only examine cached errors.
+        llvm::ErrorOr<llvm::vfs::Status> Stat = UnderlyingFS.status(Path);
+        if (Stat) {
+          // This is the case where we have cached the non-existence
+          // of the file at Path first, and a a file at the path is created
+          // later. The cache entry is not invalidated (as we have no good
+          // way to do it now), which may lead to missing file build errors.
+          InvalidPaths.push_back(Path);
+        }
+      }
+    }
+  }
+  return InvalidPaths;
+}
+
 const CachedFileSystemEntry *
 DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename(
     StringRef Filename) const {

diff  --git a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
index 111351fb90cee..5ea8d02353dc3 100644
--- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
+++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
@@ -178,3 +178,27 @@ TEST(DependencyScanningFilesystem, CacheStatFailures) {
   DepFS.exists("/cache/a.pcm");
   EXPECT_EQ(InstrumentingFS->NumStatusCalls, 5u);
 }
+
+TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
+  auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  InMemoryFS->setCurrentWorkingDirectory("/");
+
+  DependencyScanningFilesystemSharedCache SharedCache;
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
+
+  bool Path1Exists = DepFS.exists("/path1");
+  EXPECT_EQ(Path1Exists, false);
+
+  // Adding a file that has been stat-ed,
+  InMemoryFS->addFile("/path1", 0, llvm::MemoryBuffer::getMemBuffer(""));
+  Path1Exists = DepFS.exists("/path1");
+  // Due to caching in SharedCache, path1 should not exist in
+  // DepFS's eyes.
+  EXPECT_EQ(Path1Exists, false);
+
+  std::vector<llvm::StringRef> InvalidPaths =
+      SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS.get());
+
+  EXPECT_EQ(InvalidPaths.size(), 1u);
+  ASSERT_STREQ("/path1", InvalidPaths[0].str().c_str());
+}