From df3ac5aa4a59d4f654dd34d0a2e4faf55a376a96 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Mon, 12 Oct 2015 16:16:39 +0000 Subject: [PATCH] [VFS] Let the user decide if they want path normalization. This is a more principled version of what I did earlier. Path normalization is generally a good thing, but may break users in strange environments, e. g. using lots of symlinks. Let the user choose and default it to on. This also changes adding a duplicated file into returning an error if the file contents are different instead of an assertion failure. Differential Revision: http://reviews.llvm.org/D13658 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@250060 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/VirtualFileSystem.h | 13 +++++-- lib/Basic/VirtualFileSystem.cpp | 46 +++++++++++++++-------- unittests/Basic/VirtualFileSystemTest.cpp | 33 +++++++++++++++- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/include/clang/Basic/VirtualFileSystem.h b/include/clang/Basic/VirtualFileSystem.h index f93d69f604..1df4947dd7 100644 --- a/include/clang/Basic/VirtualFileSystem.h +++ b/include/clang/Basic/VirtualFileSystem.h @@ -273,17 +273,24 @@ class InMemoryDirectory; class InMemoryFileSystem : public FileSystem { std::unique_ptr Root; std::string WorkingDirectory; + bool UseNormalizedPaths = true; public: - InMemoryFileSystem(); + explicit InMemoryFileSystem(bool UseNormalizedPaths = true); ~InMemoryFileSystem() override; /// Add a buffer to the VFS with a path. The VFS owns the buffer. - void addFile(const Twine &Path, time_t ModificationTime, + /// \return true if the file was successfully added, false if the file already + /// exists in the file system with different contents. + bool addFile(const Twine &Path, time_t ModificationTime, std::unique_ptr Buffer); /// Add a buffer to the VFS with a path. The VFS does not own the buffer. - void addFileNoOwn(const Twine &Path, time_t ModificationTime, + /// \return true if the file was successfully added, false if the file already + /// exists in the file system with different contents. + bool addFileNoOwn(const Twine &Path, time_t ModificationTime, llvm::MemoryBuffer *Buffer); std::string toString() const; + /// Return true if this file system normalizes . and .. in paths. + bool useNormalizedPaths() const { return UseNormalizedPaths; } llvm::ErrorOr status(const Twine &Path) override; llvm::ErrorOr> diff --git a/lib/Basic/VirtualFileSystem.cpp b/lib/Basic/VirtualFileSystem.cpp index 1b75fbefd2..a20132b3de 100644 --- a/lib/Basic/VirtualFileSystem.cpp +++ b/lib/Basic/VirtualFileSystem.cpp @@ -10,6 +10,7 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/VirtualFileSystem.h" +#include "clang/Basic/FileManager.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" @@ -474,11 +475,12 @@ public: }; } -InMemoryFileSystem::InMemoryFileSystem() +InMemoryFileSystem::InMemoryFileSystem(bool UseNormalizedPaths) : Root(new detail::InMemoryDirectory( Status("", getNextVirtualUniqueID(), llvm::sys::TimeValue::MinTime(), 0, 0, 0, llvm::sys::fs::file_type::directory_file, - llvm::sys::fs::perms::all_all))) {} + llvm::sys::fs::perms::all_all))), + UseNormalizedPaths(UseNormalizedPaths) {} InMemoryFileSystem::~InMemoryFileSystem() {} @@ -486,7 +488,7 @@ std::string InMemoryFileSystem::toString() const { return Root->toString(/*Indent=*/0); } -void InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, +bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, std::unique_ptr Buffer) { SmallString<128> Path; P.toVector(Path); @@ -496,14 +498,14 @@ void InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, assert(!EC); (void)EC; - detail::InMemoryDirectory *Dir = Root.get(); - auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path); - if (*I == ".") - ++I; + if (useNormalizedPaths()) + FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true); - if (I == E) - return; + if (Path.empty()) + return false; + detail::InMemoryDirectory *Dir = Root.get(); + auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path); while (true) { StringRef Name = *I; detail::InMemoryNode *Node = Dir->getChild(Name); @@ -519,7 +521,7 @@ void InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, llvm::sys::fs::all_all); Dir->addChild(Name, llvm::make_unique( std::move(Stat), std::move(Buffer))); - return; + return true; } // Create a new directory. Use the path up to here. @@ -534,12 +536,24 @@ void InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, continue; } - if (auto *NewDir = dyn_cast(Node)) + if (auto *NewDir = dyn_cast(Node)) { Dir = NewDir; + } else { + assert(isa(Node) && + "Must be either file or directory!"); + + // Trying to insert a directory in place of a file. + if (I != E) + return false; + + // Return false only if the new file is different from the existing one. + return cast(Node)->getBuffer()->getBuffer() == + Buffer->getBuffer(); + } } } -void InMemoryFileSystem::addFileNoOwn(const Twine &P, time_t ModificationTime, +bool InMemoryFileSystem::addFileNoOwn(const Twine &P, time_t ModificationTime, llvm::MemoryBuffer *Buffer) { return addFile(P, ModificationTime, llvm::MemoryBuffer::getMemBuffer( @@ -557,13 +571,13 @@ lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory *Dir, assert(!EC); (void)EC; - auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path); - if (*I == ".") - ++I; + if (FS.useNormalizedPaths()) + FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true); - if (I == E) + if (Path.empty()) return Dir; + auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path); while (true) { detail::InMemoryNode *Node = Dir->getChild(*I); ++I; diff --git a/unittests/Basic/VirtualFileSystemTest.cpp b/unittests/Basic/VirtualFileSystemTest.cpp index bd75ad0112..96b8295acd 100644 --- a/unittests/Basic/VirtualFileSystemTest.cpp +++ b/unittests/Basic/VirtualFileSystemTest.cpp @@ -525,6 +525,11 @@ TEST(VirtualFileSystemTest, HiddenInIteration) { class InMemoryFileSystemTest : public ::testing::Test { protected: clang::vfs::InMemoryFileSystem FS; + clang::vfs::InMemoryFileSystem NormalizedFS; + + InMemoryFileSystemTest() + : FS(/*UseNormalizedPaths=*/false), + NormalizedFS(/*UseNormalizedPaths=*/true) {} }; TEST_F(InMemoryFileSystemTest, IsEmpty) { @@ -549,8 +554,13 @@ TEST_F(InMemoryFileSystemTest, WindowsPath) { TEST_F(InMemoryFileSystemTest, OverlayFile) { FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")); + NormalizedFS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")); auto Stat = FS.status("/"); ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString(); + Stat = FS.status("/."); + ASSERT_FALSE(Stat); + Stat = NormalizedFS.status("/."); + ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString(); Stat = FS.status("/a"); ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString(); ASSERT_EQ("/a", Stat->getName()); @@ -566,17 +576,36 @@ TEST_F(InMemoryFileSystemTest, OverlayFileNoOwn) { TEST_F(InMemoryFileSystemTest, OpenFileForRead) { FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")); - FS.addFile("./c", 0, MemoryBuffer::getMemBuffer("c")); + FS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c")); + FS.addFile("./d/../d", 0, MemoryBuffer::getMemBuffer("d")); + NormalizedFS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")); + NormalizedFS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c")); + NormalizedFS.addFile("./d/../d", 0, MemoryBuffer::getMemBuffer("d")); auto File = FS.openFileForRead("/a"); ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer()); File = FS.openFileForRead("/a"); // Open again. ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer()); + File = NormalizedFS.openFileForRead("/././a"); // Open again. + ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer()); File = FS.openFileForRead("/"); ASSERT_EQ(File.getError(), errc::invalid_argument) << FS.toString(); File = FS.openFileForRead("/b"); ASSERT_EQ(File.getError(), errc::no_such_file_or_directory) << FS.toString(); - File = FS.openFileForRead("c"); + File = FS.openFileForRead("./c"); + ASSERT_FALSE(File); + File = FS.openFileForRead("e/../d"); + ASSERT_FALSE(File); + File = NormalizedFS.openFileForRead("./c"); ASSERT_EQ("c", (*(*File)->getBuffer("ignored"))->getBuffer()); + File = NormalizedFS.openFileForRead("e/../d"); + ASSERT_EQ("d", (*(*File)->getBuffer("ignored"))->getBuffer()); +} + +TEST_F(InMemoryFileSystemTest, DuplicatedFile) { + ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"))); + ASSERT_FALSE(FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("a"))); + ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"))); + ASSERT_FALSE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("b"))); } TEST_F(InMemoryFileSystemTest, DirectoryIteration) { -- 2.40.0