Skip to content

Commit 874a5da

Browse files
authored
[clang] Processing real directories added as virtual ones (#91645)
The `FileManager` might create a virtual directory that can be used later as a search path. This is the case when we use remapping, as demonstrated in the suggested LIT test. We might encounter a 'false cache miss' and add the same directory twice into `FileManager::SeenDirEntries` if the added record is a real directory that is present on a disk: - Once as a virtual directory - And once as a real one This isn't a problem if the added directories have the same name, as in this case, we will get a cache hit. However, it could lead to compilation errors if the directory names are different but point to the same folder. For example, one might use an absolute name and another a relative one. For instance, the **implicit-module-remap.cpp** LIT test will fail with the following message: ``` /.../implicit-module-remap.cpp.tmp/test.cpp:1:2: fatal error: module 'a' was built in directory '/.../implicit-module-remap.cpp.tmp' but now resides in directory '.' 1 | #include "a.h" | ^ 1 error generated. ``` The suggested fix checks if the added virtual directory is present on the disk and handles it as a real one if that is the case.
1 parent c609c04 commit 874a5da

File tree

3 files changed

+56
-15
lines changed

3 files changed

+56
-15
lines changed

clang/include/clang/Basic/FileManager.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,8 @@ class FileManager : public RefCountedBase<FileManager> {
299299
getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile,
300300
bool RequiresNullTerminator) const;
301301

302+
DirectoryEntry *&getRealDirEntry(const llvm::vfs::Status &Status);
303+
302304
public:
303305
/// Get the 'stat' information for the given \p Path.
304306
///

clang/lib/Basic/FileManager.cpp

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,22 @@ getDirectoryFromFile(FileManager &FileMgr, StringRef Filename,
8282
return FileMgr.getDirectoryRef(DirName, CacheFailure);
8383
}
8484

85+
DirectoryEntry *&FileManager::getRealDirEntry(const llvm::vfs::Status &Status) {
86+
assert(Status.isDirectory() && "The directory should exist!");
87+
// See if we have already opened a directory with the
88+
// same inode (this occurs on Unix-like systems when one dir is
89+
// symlinked to another, for example) or the same path (on
90+
// Windows).
91+
DirectoryEntry *&UDE = UniqueRealDirs[Status.getUniqueID()];
92+
93+
if (!UDE) {
94+
// We don't have this directory yet, add it. We use the string
95+
// key from the SeenDirEntries map as the string.
96+
UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
97+
}
98+
return UDE;
99+
}
100+
85101
/// Add all ancestors of the given path (pointing to either a file or
86102
/// a directory) as virtual directories.
87103
void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
@@ -99,10 +115,21 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
99115
if (NamedDirEnt.second)
100116
return;
101117

102-
// Add the virtual directory to the cache.
103-
auto *UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
104-
NamedDirEnt.second = *UDE;
105-
VirtualDirectoryEntries.push_back(UDE);
118+
// Check to see if the directory exists.
119+
llvm::vfs::Status Status;
120+
auto statError =
121+
getStatValue(DirName, Status, false, nullptr /*directory lookup*/);
122+
if (statError) {
123+
// There's no real directory at the given path.
124+
// Add the virtual directory to the cache.
125+
auto *UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
126+
NamedDirEnt.second = *UDE;
127+
VirtualDirectoryEntries.push_back(UDE);
128+
} else {
129+
// There is the real directory
130+
DirectoryEntry *&UDE = getRealDirEntry(Status);
131+
NamedDirEnt.second = *UDE;
132+
}
106133

107134
// Recursively add the other ancestors.
108135
addAncestorsAsVirtualDirs(DirName);
@@ -162,17 +189,8 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
162189
return llvm::errorCodeToError(statError);
163190
}
164191

165-
// It exists. See if we have already opened a directory with the
166-
// same inode (this occurs on Unix-like systems when one dir is
167-
// symlinked to another, for example) or the same path (on
168-
// Windows).
169-
DirectoryEntry *&UDE = UniqueRealDirs[Status.getUniqueID()];
170-
171-
if (!UDE) {
172-
// We don't have this directory yet, add it. We use the string
173-
// key from the SeenDirEntries map as the string.
174-
UDE = new (DirsAlloc.Allocate()) DirectoryEntry();
175-
}
192+
// It exists.
193+
DirectoryEntry *&UDE = getRealDirEntry(Status);
176194
NamedDirEnt.second = *UDE;
177195

178196
return DirectoryEntryRef(NamedDirEnt);
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: cd %t
4+
//
5+
// RUN: %clang_cc1 -fmodules -fmodule-map-file=module.modulemap -fmodules-cache-path=%t -remap-file "test.cpp;%t/test.cpp" %t/test.cpp
6+
7+
//--- a.h
8+
#define FOO
9+
10+
//--- module.modulemap
11+
module a {
12+
header "a.h"
13+
}
14+
15+
//--- test.cpp
16+
#include "a.h"
17+
18+
#ifndef FOO
19+
#error foo
20+
#endif
21+

0 commit comments

Comments
 (0)