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

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;
 }

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