Author: erikjv
Date: Tue Mar 28 04:18:05 2017
New Revision: 298905
URL: http://llvm.org/viewvc/llvm-project?rev=298905&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=298905&r1=298904&r2=298905&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Mar 28 04:18:05 2017
@@ -386,6 +386,7 @@ FileManager::getVirtualFile(StringRef Fi
UFE->ModTime = ModificationTime;
UFE->Dir = DirInfo;
UFE->UID = NextFileUID++;
+ UFE->IsValid = true;
UFE->File.reset();
return UFE;
}
Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=298905&r1=298904&r2=298905&view=diff
==============================================================================
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Tue Mar 28 04:18:05 2017
@@ -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, getFileReturnsVa
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, getFileReturnsVa
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, getFileReturnsDi
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, getFileReturnsSa
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, addRemoveStatCac
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
