Skip to content

Commit 5c9c278

Browse files
committed
[clang][Dependency Scanning] Adding an API to Diagnose Invalid Negative Stat Cache Entries (llvm#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 (cherry picked from commit 9ef9167) Conflicts: clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp
1 parent b372ced commit 5c9c278

File tree

3 files changed

+59
-0
lines changed

3 files changed

+59
-0
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@ class DependencyScanningFilesystemSharedCache {
233233
CacheShard &getShardForFilename(StringRef Filename) const;
234234
CacheShard &getShardForUID(llvm::sys::fs::UniqueID UID) const;
235235

236+
/// Visits all cached entries and re-stat an entry using FS if
237+
/// it is negatively stat cached. If re-stat succeeds on a path,
238+
/// the path is added to InvalidPaths, indicating that the cache
239+
/// may have erroneously negatively cached it. The caller can then
240+
/// use InvalidPaths to issue diagnostics.
241+
std::vector<StringRef>
242+
getInvalidNegativeStatCachedPaths(llvm::vfs::FileSystem &UnderlyingFS) const;
243+
236244
private:
237245
std::unique_ptr<CacheShard[]> CacheShards;
238246
unsigned NumShards;

clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,33 @@ DependencyScanningFilesystemSharedCache::getShardForUID(
113113
return CacheShards[Hash % NumShards];
114114
}
115115

116+
std::vector<StringRef>
117+
DependencyScanningFilesystemSharedCache::getInvalidNegativeStatCachedPaths(
118+
llvm::vfs::FileSystem &UnderlyingFS) const {
119+
// Iterate through all shards and look for cached stat errors.
120+
std::vector<StringRef> InvalidPaths;
121+
for (unsigned i = 0; i < NumShards; i++) {
122+
const CacheShard &Shard = CacheShards[i];
123+
std::lock_guard<std::mutex> LockGuard(Shard.CacheLock);
124+
for (const auto &[Path, CachedPair] : Shard.CacheByFilename) {
125+
const CachedFileSystemEntry *Entry = CachedPair.first;
126+
127+
if (Entry->getError()) {
128+
// Only examine cached errors.
129+
llvm::ErrorOr<llvm::vfs::Status> Stat = UnderlyingFS.status(Path);
130+
if (Stat) {
131+
// This is the case where we have cached the non-existence
132+
// of the file at Path first, and a a file at the path is created
133+
// later. The cache entry is not invalidated (as we have no good
134+
// way to do it now), which may lead to missing file build errors.
135+
InvalidPaths.push_back(Path);
136+
}
137+
}
138+
}
139+
}
140+
return InvalidPaths;
141+
}
142+
116143
const CachedFileSystemEntry *
117144
DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename(
118145
StringRef Filename) const {

clang/unittests/Tooling/DependencyScanning/DependencyScanningFilesystemTest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,27 @@ TEST(DependencyScanningFilesystem, CacheStatOnExists) {
146146
EXPECT_EQ(InstrumentingFS->NumStatusCalls, 2u);
147147
EXPECT_EQ(InstrumentingFS->NumExistsCalls, 0u);
148148
}
149+
150+
TEST(DependencyScanningFilesystem, DiagnoseStaleStatFailures) {
151+
auto InMemoryFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
152+
InMemoryFS->setCurrentWorkingDirectory("/");
153+
154+
DependencyScanningFilesystemSharedCache SharedCache;
155+
DependencyScanningWorkerFilesystem DepFS(SharedCache, InMemoryFS);
156+
157+
bool Path1Exists = DepFS.exists("/path1");
158+
EXPECT_EQ(Path1Exists, false);
159+
160+
// Adding a file that has been stat-ed,
161+
InMemoryFS->addFile("/path1", 0, llvm::MemoryBuffer::getMemBuffer(""));
162+
Path1Exists = DepFS.exists("/path1");
163+
// Due to caching in SharedCache, path1 should not exist in
164+
// DepFS's eyes.
165+
EXPECT_EQ(Path1Exists, false);
166+
167+
std::vector<llvm::StringRef> InvalidPaths =
168+
SharedCache.getInvalidNegativeStatCachedPaths(*InMemoryFS.get());
169+
170+
EXPECT_EQ(InvalidPaths.size(), 1u);
171+
ASSERT_STREQ("/path1", InvalidPaths[0].str().c_str());
172+
}

0 commit comments

Comments
 (0)