FileManager: mark virtual file entries as valid entries

The getVirtualFile method would create entries for e.g. libclang's
CXUnsavedFile but not mark them as valid. The effect is that a lookup
through getFile where the file name is not exactly matching the virtual
file (e.g. through mixing slashes and backslashes on Windows) would
result in a normal file "lookup", and re-using the file entry found
by using the UniqueID, and overwrite the file entry fields. Because the
lookup involves opening the file, and moving it into the file entry, the
file is now open. The SourceManager keys its buffers on the UniqueID
(which is still the same), so it will find an already loaded buffer.
Because only the loading a buffer from disk will close the file, the
FileEntry will hold on to an open file for as long as the FileManager
is around. As the FileManager will only get destroyed at a reparse,
you can't safe to the "leaked" and locked file on Windows.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@298905 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Basic/FileManager.cpp b/lib/Basic/FileManager.cpp
index 646c1910bf4e6a84533cb37a07cf7ccbf210b6c1..0c10b5f4d14cbd07fa27097a7358d0b49e1cd58a 100644 (file)
--- a/lib/Basic/FileManager.cpp
+++ b/lib/Basic/FileManager.cpp
@@ -386,6 +386,7 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
   UFE->ModTime = ModificationTime;
   UFE->Dir     = DirInfo;
   UFE->UID     = NextFileUID++;
+  UFE->IsValid = true;
   UFE->File.reset();
   return UFE;
 }

diff --git a/unittests/Basic/FileManagerTest.cpp b/unittests/Basic/FileManagerTest.cpp
index 4cf21cd8459abc2f255d2d5e5bcec79850ad8953..a19535e1047cc6fd96a4bec689a8b6e7bca4cb0f 100644 (file)
--- a/unittests/Basic/FileManagerTest.cpp
+++ b/unittests/Basic/FileManagerTest.cpp
@@ -12,6 +12,7 @@
 #include "clang/Basic/FileSystemStatCache.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -29,6 +30,12 @@ private:
   llvm::StringMap<FileData, llvm::BumpPtrAllocator> StatCalls;
 
   void InjectFileOrDirectory(const char *Path, ino_t INode, bool IsFile) {
+#ifndef LLVM_ON_WIN32
+    SmallString<128> NormalizedPath(Path);
+    llvm::sys::path::native(NormalizedPath);
+    Path = NormalizedPath.c_str();
+#endif
+
     FileData Data;
     Data.Name = Path;
     Data.Size = 0;
@@ -55,6 +62,12 @@ public:
   LookupResult getStat(StringRef Path, FileData &Data, bool isFile,
                        std::unique_ptr<vfs::File> *F,
                        vfs::FileSystem &FS) override {
+#ifndef LLVM_ON_WIN32
+    SmallString<128> NormalizedPath(Path);
+    llvm::sys::path::native(NormalizedPath);
+    Path = NormalizedPath.c_str();
+#endif
+
     if (StatCalls.count(Path) != 0) {
       Data = StatCalls[Path];
       return CacheExists;
@@ -140,6 +153,7 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingRealFile) {
 
   const FileEntry *file = manager.getFile("/tmp/test");
   ASSERT_TRUE(file != nullptr);
+  ASSERT_TRUE(file->isValid());
   EXPECT_EQ("/tmp/test", file->getName());
 
   const DirectoryEntry *dir = file->getDir();
@@ -164,6 +178,7 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
   const FileEntry *file = manager.getFile("virtual/dir/bar.h");
   ASSERT_TRUE(file != nullptr);
+  ASSERT_TRUE(file->isValid());
   EXPECT_EQ("virtual/dir/bar.h", file->getName());
 
   const DirectoryEntry *dir = file->getDir();
@@ -185,7 +200,9 @@ TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
   const FileEntry *fileFoo = manager.getFile("foo.cpp");
   const FileEntry *fileBar = manager.getFile("bar.cpp");
   ASSERT_TRUE(fileFoo != nullptr);
+  ASSERT_TRUE(fileFoo->isValid());
   ASSERT_TRUE(fileBar != nullptr);
+  ASSERT_TRUE(fileBar->isValid());
   EXPECT_NE(fileFoo, fileBar);
 }
 
@@ -231,8 +248,8 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
   statCache->InjectFile("abc/bar.cpp", 42);
   manager.addStatCache(std::move(statCache));
 
-  manager.getVirtualFile("abc/foo.cpp", 100, 0);
-  manager.getVirtualFile("abc/bar.cpp", 200, 0);
+  ASSERT_TRUE(manager.getVirtualFile("abc/foo.cpp", 100, 0)->isValid());
+  ASSERT_TRUE(manager.getVirtualFile("abc/bar.cpp", 200, 0)->isValid());
 
   EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
 }
@@ -246,6 +263,37 @@ TEST_F(FileManagerTest, addRemoveStatCache) {
   manager.removeStatCache(statCache);
 }
 
+// getFile() Should return the same entry as getVirtualFile if the file actually
+// is a virtual file, even if the name is not exactly the same (but is after
+// normalisation done by the file system, like on Windows). This can be checked
+// here by checkng the size.
+TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
+  // Inject fake files into the file system.
+  auto statCache = llvm::make_unique<FakeStatCache>();
+  statCache->InjectDirectory("c:\\tmp", 42);
+  statCache->InjectFile("c:\\tmp\\test", 43);
+
+  manager.addStatCache(std::move(statCache));
+
+  // Inject the virtual file:
+  const FileEntry *file1 = manager.getVirtualFile("c:\\tmp\\test", 123, 1);
+  ASSERT_TRUE(file1 != nullptr);
+  ASSERT_TRUE(file1->isValid());
+  EXPECT_EQ(43U, file1->getUniqueID().getFile());
+  EXPECT_EQ(123, file1->getSize());
+
+  // Lookup the virtual file with a different name:
+  const FileEntry *file2 = manager.getFile("c:/tmp/test", 100, 1);
+  ASSERT_TRUE(file2 != nullptr);
+  ASSERT_TRUE(file2->isValid());
+  // Check that it's the same UFE:
+  EXPECT_EQ(file1, file2);
+  EXPECT_EQ(43U, file2->getUniqueID().getFile());
+  // Check that the contents of the UFE are not overwritten by the entry in the
+  // filesystem:
+  EXPECT_EQ(123, file2->getSize());
+}
+
 #endif  // !LLVM_ON_WIN32
 
 } // anonymous namespace