aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/Basic/FileManager.h5
-rw-r--r--clang/lib/Basic/FileManager.cpp35
-rw-r--r--clang/test/PCH/leakfiles29
-rw-r--r--clang/unittests/Basic/FileManagerTest.cpp27
4 files changed, 42 insertions, 54 deletions
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 4c940a0..88c1467 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -69,15 +69,14 @@ class FileEntry {
bool IsNamedPipe;
bool InPCH;
bool IsValid; // Is this \c FileEntry initialized and valid?
- bool DeferredOpen; // Created by getFile(OpenFile=0); may open later.
/// The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr<llvm::vfs::File> File;
public:
FileEntry()
- : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false),
- DeferredOpen(false) {}
+ : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
+ {}
FileEntry(const FileEntry &) = delete;
FileEntry &operator=(const FileEntry &) = delete;
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 01acfd5..e378626 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -188,21 +188,15 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
*SeenFileEntries.insert(std::make_pair(Filename, nullptr)).first;
// See if there is already an entry in the map.
- if (NamedFileEnt.second) {
- if (NamedFileEnt.second == NON_EXISTENT_FILE)
- return nullptr;
- // Entry exists: return it *unless* it wasn't opened and open is requested.
- if (!(NamedFileEnt.second->DeferredOpen && openFile))
- return NamedFileEnt.second;
- // We previously stat()ed the file, but didn't open it: do that below.
- // FIXME: the below does other redundant work too (stats the dir and file).
- } else {
- // By default, initialize it to invalid.
- NamedFileEnt.second = NON_EXISTENT_FILE;
- }
+ if (NamedFileEnt.second)
+ return NamedFileEnt.second == NON_EXISTENT_FILE ? nullptr
+ : NamedFileEnt.second;
++NumFileCacheMisses;
+ // By default, initialize it to invalid.
+ NamedFileEnt.second = NON_EXISTENT_FILE;
+
// Get the null-terminated file name as stored as the key of the
// SeenFileEntries map.
StringRef InterndFileName = NamedFileEnt.first();
@@ -240,7 +234,6 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
// It exists. See if we have already opened a file with the same inode.
// This occurs when one dir is symlinked to another, for example.
FileEntry &UFE = UniqueRealFiles[Data.UniqueID];
- UFE.DeferredOpen = !openFile;
NamedFileEnt.second = &UFE;
@@ -257,15 +250,6 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
InterndFileName = NamedFileEnt.first().data();
}
- // If we opened the file for the first time, record the resulting info.
- // Do this even if the cache entry was valid, maybe we didn't previously open.
- if (F && !UFE.File) {
- if (auto PathName = F->getName())
- fillRealPathName(&UFE, *PathName);
- UFE.File = std::move(F);
- assert(!UFE.DeferredOpen && "we just opened it!");
- }
-
if (UFE.isValid()) { // Already have an entry with this inode, return it.
// FIXME: this hack ensures that if we look up a file by a virtual path in
@@ -296,9 +280,13 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
UFE.UniqueID = Data.UniqueID;
UFE.IsNamedPipe = Data.IsNamedPipe;
UFE.InPCH = Data.InPCH;
+ UFE.File = std::move(F);
UFE.IsValid = true;
- // Note File and DeferredOpen were initialized above.
+ if (UFE.File) {
+ if (auto PathName = UFE.File->getName())
+ fillRealPathName(&UFE, *PathName);
+ }
return &UFE;
}
@@ -370,7 +358,6 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
UFE->UID = NextFileUID++;
UFE->IsValid = true;
UFE->File.reset();
- UFE->DeferredOpen = false;
return UFE;
}
diff --git a/clang/test/PCH/leakfiles b/clang/test/PCH/leakfiles
new file mode 100644
index 0000000..90b2790
--- /dev/null
+++ b/clang/test/PCH/leakfiles
@@ -0,0 +1,29 @@
+// Test that compiling using a PCH doesn't leak file descriptors.
+// https://bugs.chromium.org/p/chromium/issues/detail?id=924225
+//
+// This test requires bash loops and ulimit.
+// REQUIRES: shell
+// UNSUPPORTED: win32
+//
+// Set up source files. lib/lib.h includes lots of lib*.h files in that dir.
+// client.c includes lib/lib.h, and also the individual files directly.
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: mkdir lib
+// RUN: for i in {1..300}; do touch lib/lib$i.h; done
+// RUN: for i in {1..300}; do echo "#include \"lib$i.h\"" >> lib/lib.h; done
+// RUN: echo "#include \"lib/lib.h\"" > client.c
+// RUN: for i in {1..300}; do echo "#include \"lib/lib$i.h\"" >> client.c; done
+//
+// We want to verify that we don't hold all the files open at the same time.
+// This is important e.g. on mac, which has a low default FD limit.
+// RUN: ulimit -n 100
+//
+// Test without PCH.
+// RUN: %clang_cc1 -fsyntax-only -Ilib/ client.c
+//
+// Test with PCH.
+// RUN: %clang_cc1 -emit-pch -o pch -Ilib/ client.c
+// RUN: %clang_cc1 -include-pch pch -Ilib/ client.c -fsyntax-only
diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp
index ddc8a13..9f05197 100644
--- a/clang/unittests/Basic/FileManagerTest.cpp
+++ b/clang/unittests/Basic/FileManagerTest.cpp
@@ -221,33 +221,6 @@ TEST_F(FileManagerTest, getFileReturnsNULLForNonexistentFile) {
EXPECT_EQ(nullptr, file);
}
-// When calling getFile(OpenFile=false); getFile(OpenFile=true) the file is
-// opened for the second call.
-TEST_F(FileManagerTest, getFileDefersOpen) {
- llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS(
- new llvm::vfs::InMemoryFileSystem());
- FS->addFile("/tmp/test", 0, llvm::MemoryBuffer::getMemBufferCopy("test"));
- FS->addFile("/tmp/testv", 0, llvm::MemoryBuffer::getMemBufferCopy("testv"));
- FileManager manager(options, FS);
-
- const FileEntry *file = manager.getFile("/tmp/test", /*OpenFile=*/false);
- ASSERT_TRUE(file != nullptr);
- ASSERT_TRUE(file->isValid());
- // "real path name" reveals whether the file was actually opened.
- EXPECT_FALSE(file->isOpenForTests());
-
- file = manager.getFile("/tmp/test", /*OpenFile=*/true);
- ASSERT_TRUE(file != nullptr);
- ASSERT_TRUE(file->isValid());
- EXPECT_TRUE(file->isOpenForTests());
-
- // However we should never try to open a file previously opened as virtual.
- ASSERT_TRUE(manager.getVirtualFile("/tmp/testv", 5, 0));
- ASSERT_TRUE(manager.getFile("/tmp/testv", /*OpenFile=*/false));
- file = manager.getFile("/tmp/testv", /*OpenFile=*/true);
- EXPECT_FALSE(file->isOpenForTests());
-}
-
// The following tests apply to Unix-like system only.
#ifndef _WIN32
generated by cgit v1.1 at 2025-08-04 06:07:52 +0000