[llvm-branch-commits] [llvm] 0be9ca7 - [VFS] Fix inconsistencies between relative paths and fallthrough.
Jonas Devlieghere via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Jan 22 14:20:27 PST 2021
Author: Jonas Devlieghere
Date: 2021-01-22T14:15:48-08:00
New Revision: 0be9ca7c0f9a733f846bb6bc4e8e36d46b518162
URL: https://github.com/llvm/llvm-project/commit/0be9ca7c0f9a733f846bb6bc4e8e36d46b518162
DIFF: https://github.com/llvm/llvm-project/commit/0be9ca7c0f9a733f846bb6bc4e8e36d46b518162.diff
LOG: [VFS] Fix inconsistencies between relative paths and fallthrough.
This patch addresses inconsistencies in the way fallthrough is handled
in the RedirectingFileSystem. Rather than trying to change the working
directory of the external filesystem, the RedirectingFileSystem will
canonicalize every path before handing it down. This guarantees that
relative paths are resolved relative to the RedirectingFileSystem's
working directory.
This allows us to have a strictly virtual working directory, and still
fallthrough for absolute paths, but not for relative paths that would
get resolved incorrectly at the lower layer (for example, in case of the
RealFileSystem, because the strictly virtual path does not exist).
Differential revision: https://reviews.llvm.org/D95188
Added:
Modified:
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index 6547d2cdb629..c6ddbf60efdf 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -651,9 +651,12 @@ class RedirectingFileSystem : public vfs::FileSystem {
friend class VFSFromYamlDirIterImpl;
friend class RedirectingFileSystemParser;
- bool shouldUseExternalFS() const {
- return ExternalFSValidWD && IsFallthrough;
- }
+ bool shouldUseExternalFS() const { return IsFallthrough; }
+
+ /// Canonicalize path by removing ".", "..", "./", components. This is
+ /// a VFS request, do not bother about symlinks in the path components
+ /// but canonicalize in order to perform the correct entry search.
+ std::error_code makeCanonical(SmallVectorImpl<char> &Path) const;
// In a RedirectingFileSystem, keys can be specified in Posix or Windows
// style (or even a mixture of both), so this comparison helper allows
@@ -672,9 +675,6 @@ class RedirectingFileSystem : public vfs::FileSystem {
/// The current working directory of the file system.
std::string WorkingDirectory;
- /// Whether the current working directory is valid for the external FS.
- bool ExternalFSValidWD = false;
-
/// The file system to use for external references.
IntrusiveRefCntPtr<FileSystem> ExternalFS;
@@ -722,7 +722,7 @@ class RedirectingFileSystem : public vfs::FileSystem {
public:
/// Looks up \p Path in \c Roots.
- ErrorOr<Entry *> lookupPath(const Twine &Path) const;
+ ErrorOr<Entry *> lookupPath(StringRef Path) const;
/// Parses \p Buffer, which is expected to be in YAML format and
/// returns a virtual file system representing its contents.
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index c2ce63964f08..05332b00bd1f 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -1016,7 +1016,6 @@ RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
if (auto ExternalWorkingDirectory =
ExternalFS->getCurrentWorkingDirectory()) {
WorkingDirectory = *ExternalWorkingDirectory;
- ExternalFSValidWD = true;
}
}
@@ -1075,12 +1074,6 @@ RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
if (!exists(Path))
return errc::no_such_file_or_directory;
- // Always change the external FS but ignore its result.
- if (ExternalFS) {
- auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
- ExternalFSValidWD = !static_cast<bool>(EC);
- }
-
SmallString<128> AbsolutePath;
Path.toVector(AbsolutePath);
if (std::error_code EC = makeAbsolute(AbsolutePath))
@@ -1089,8 +1082,14 @@ RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
return {};
}
-std::error_code RedirectingFileSystem::isLocal(const Twine &Path,
+std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
bool &Result) {
+ SmallString<256> Path;
+ Path_.toVector(Path);
+
+ if (std::error_code EC = makeCanonical(Path))
+ return {};
+
return ExternalFS->isLocal(Path, Result);
}
@@ -1125,14 +1124,21 @@ std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path)
directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
std::error_code &EC) {
- ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Dir);
+ SmallString<256> Path;
+ Dir.toVector(Path);
+
+ EC = makeCanonical(Path);
+ if (EC)
+ return {};
+
+ ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
if (!E) {
EC = E.getError();
if (shouldUseExternalFS() && EC == errc::no_such_file_or_directory)
- return ExternalFS->dir_begin(Dir, EC);
+ return ExternalFS->dir_begin(Path, EC);
return {};
}
- ErrorOr<Status> S = status(Dir, *E);
+ ErrorOr<Status> S = status(Path, *E);
if (!S) {
EC = S.getError();
return {};
@@ -1145,7 +1151,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir,
auto *D = cast<RedirectingFileSystem::RedirectingDirectoryEntry>(*E);
return directory_iterator(std::make_shared<VFSFromYamlDirIterImpl>(
- Dir, D->contents_begin(), D->contents_end(),
+ Path, D->contents_begin(), D->contents_end(),
/*IterateExternalFS=*/shouldUseExternalFS(), *ExternalFS, EC));
}
@@ -1739,22 +1745,22 @@ std::unique_ptr<RedirectingFileSystem> RedirectingFileSystem::create(
return FS;
}
-ErrorOr<RedirectingFileSystem::Entry *>
-RedirectingFileSystem::lookupPath(const Twine &Path_) const {
- SmallString<256> Path;
- Path_.toVector(Path);
-
- // Handle relative paths
+std::error_code
+RedirectingFileSystem::makeCanonical(SmallVectorImpl<char> &Path) const {
if (std::error_code EC = makeAbsolute(Path))
return EC;
- // Canonicalize path by removing ".", "..", "./", components. This is
- // a VFS request, do not bother about symlinks in the path components
- // but canonicalize in order to perform the correct entry search.
- Path = canonicalize(Path);
- if (Path.empty())
+ llvm::SmallString<256> CanonicalPath =
+ canonicalize(StringRef(Path.data(), Path.size()));
+ if (CanonicalPath.empty())
return make_error_code(llvm::errc::invalid_argument);
+ Path.assign(CanonicalPath.begin(), CanonicalPath.end());
+ return {};
+}
+
+ErrorOr<RedirectingFileSystem::Entry *>
+RedirectingFileSystem::lookupPath(StringRef Path) const {
sys::path::const_iterator Start = sys::path::begin(Path);
sys::path::const_iterator End = sys::path::end(Path);
for (const auto &Root : Roots) {
@@ -1829,7 +1835,13 @@ ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path,
}
}
-ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path) {
+ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path_) {
+ SmallString<256> Path;
+ Path_.toVector(Path);
+
+ if (std::error_code EC = makeCanonical(Path))
+ return EC;
+
ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
if (!Result) {
if (shouldUseExternalFS() &&
@@ -1867,7 +1879,13 @@ class FileWithFixedStatus : public File {
} // namespace
ErrorOr<std::unique_ptr<File>>
-RedirectingFileSystem::openFileForRead(const Twine &Path) {
+RedirectingFileSystem::openFileForRead(const Twine &Path_) {
+ SmallString<256> Path;
+ Path_.toVector(Path);
+
+ if (std::error_code EC = makeCanonical(Path))
+ return EC;
+
ErrorOr<RedirectingFileSystem::Entry *> E = lookupPath(Path);
if (!E) {
if (shouldUseExternalFS() &&
@@ -1897,8 +1915,14 @@ RedirectingFileSystem::openFileForRead(const Twine &Path) {
}
std::error_code
-RedirectingFileSystem::getRealPath(const Twine &Path,
+RedirectingFileSystem::getRealPath(const Twine &Path_,
SmallVectorImpl<char> &Output) const {
+ SmallString<256> Path;
+ Path_.toVector(Path);
+
+ if (std::error_code EC = makeCanonical(Path))
+ return EC;
+
ErrorOr<RedirectingFileSystem::Entry *> Result = lookupPath(Path);
if (!Result) {
if (shouldUseExternalFS() &&
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 6aff06b9a72f..86b747466b5b 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2135,7 +2135,41 @@ TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) {
EXPECT_TRUE(Status->exists());
Status = FS->status("foo/a");
- ASSERT_TRUE(Status.getError());
+ ASSERT_FALSE(Status.getError());
+ EXPECT_TRUE(Status->exists());
+}
+
+TEST_F(VFSFromYAMLTest, VirtualWorkingDirectory) {
+ IntrusiveRefCntPtr<ErrorDummyFileSystem> Lower(new ErrorDummyFileSystem());
+ Lower->addDirectory("//root/");
+ Lower->addDirectory("//root/foo");
+ Lower->addRegularFile("//root/foo/a");
+ Lower->addRegularFile("//root/foo/b");
+ Lower->addRegularFile("//root/c");
+ IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
+ "{ 'use-external-names': false,\n"
+ " 'roots': [\n"
+ "{\n"
+ " 'type': 'directory',\n"
+ " 'name': '//root/bar',\n"
+ " 'contents': [ {\n"
+ " 'type': 'file',\n"
+ " 'name': 'a',\n"
+ " 'external-contents': '//root/foo/a'\n"
+ " }\n"
+ " ]\n"
+ "}\n"
+ "]\n"
+ "}",
+ Lower);
+ ASSERT_TRUE(FS.get() != nullptr);
+ std::error_code EC = FS->setCurrentWorkingDirectory("//root/bar");
+ ASSERT_FALSE(EC);
+ ASSERT_TRUE(FS.get() != nullptr);
+
+ llvm::ErrorOr<vfs::Status> Status = FS->status("a");
+ ASSERT_FALSE(Status.getError());
+ EXPECT_TRUE(Status->exists());
}
TEST_F(VFSFromYAMLTest, YAMLVFSWriterTest) {
More information about the llvm-branch-commits
mailing list