llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Qiongsi Wu (qiongsiwu) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/135703.diff 3 Files Affected: - (modified) clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h (+8) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp (+26) - (modified) clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp (+28) ``````````diff diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index d12814e7c9253..648158c5287c6 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 the UnderlyingFS if + /// it is negatively stat cached. If the re-stat succeeds, print diagnostics + /// information to OS indicating that negative stat caching has been + /// invalidated. + void + diagnoseNegativeStatCachedPaths(llvm::raw_ostream &OS, + 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..fe72a1a91909e 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -108,6 +108,32 @@ DependencyScanningFilesystemSharedCache::getShardForUID( return CacheShards[Hash % NumShards]; } +void DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths( + llvm::raw_ostream &OS, llvm::vfs::FileSystem &UnderlyingFS) const { + // Iterate through all shards and look for cached stat errors. + for (unsigned i = 0; i < NumShards; i++) { + const CacheShard &Shard = CacheShards[i]; + for (const auto &Path : Shard.CacheByFilename.keys()) { + auto CachedPairIt = Shard.CacheByFilename.find(Path); + auto CachedPair = CachedPairIt->getValue(); + 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 negatively cached the non-existence + // of the file at Path, but the cache is later invalidated. Report + // diagnostics. + OS << Path << " did not exist when it was first searched " + << "but was created later. This may have led to a build failure " + "due to missing files.\n"; + } + } + } + } +} + 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..02b192f1ba587 100644 --- a/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp +++ b/clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp @@ -178,3 +178,31 @@ 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("/"); + InMemoryFS->addFile("/dir", 0, llvm::MemoryBuffer::getMemBuffer("")); + + 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::string Diags; + llvm::raw_string_ostream DiagsStream(Diags); + SharedCache.diagnoseNegativeStatCachedPaths(DiagsStream, *InMemoryFS.get()); + + ASSERT_STREQ( + "/path1 did not exist when it was first searched but was created later. " + "This may have led to a build failure due to missing files.\n", + Diags.c_str()); +} `````````` </details> https://github.com/llvm/llvm-project/pull/135703 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
