https://github.com/qiongsiwu updated
https://github.com/llvm/llvm-project/pull/135703
>From d4b1210c16b4fccc6faa9445bee457a1e330a025 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Mon, 14 Apr 2025 16:49:07 -0700
Subject: [PATCH 1/2] Initial commit.
---
.../DependencyScanningFilesystem.h | 8 ++++++
.../DependencyScanningFilesystem.cpp | 26 +++++++++++++++++
.../DependencyScanningFilesystemTest.cpp | 28 +++++++++++++++++++
3 files changed, 62 insertions(+)
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());
+}
>From b1058390940f40c9f62e9bcf745b4381a40db5e5 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi...@apple.com>
Date: Tue, 15 Apr 2025 10:37:52 -0700
Subject: [PATCH 2/2] Address code review.
---
.../DependencyScanningFilesystem.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index fe72a1a91909e..e61d8afeb51bb 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -113,18 +113,17 @@ void
DependencyScanningFilesystemSharedCache::diagnoseNegativeStatCachedPaths(
// 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();
+ 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 negatively cached the non-existence
- // of the file at Path, but the cache is later invalidated. Report
- // diagnostics.
+ // This is the case where we have cached the non-existence
+ // of the file at Path, but the file is created but the cache is
+ // not 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";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
