diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index adf1de3a9ad83..69e92f60cb305 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -1001,12 +1001,12 @@ class RedirectingFileSystem /// 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 &Path) const; + std::error_code makeCanonicalForLookup(SmallVectorImpl &Path) const; /// Get the File status, or error, from the underlying external file system. /// This returns the status with the originally requested name, while looking - /// up the entry using the canonical path. - ErrorOr getExternalStatus(const Twine &CanonicalPath, + /// up the entry using a potentially different path. + ErrorOr getExternalStatus(const Twine &LookupPath, const Twine &OriginalPath) const; /// Make \a Path an absolute path. @@ -1094,7 +1094,7 @@ class RedirectingFileSystem llvm::SmallVectorImpl &Entries) const; /// Get the status for a path with the provided \c LookupResult. - ErrorOr status(const Twine &CanonicalPath, const Twine &OriginalPath, + ErrorOr status(const Twine &LookupPath, const Twine &OriginalPath, const LookupResult &Result); public: diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 90b1540075e74..79fb1f5165454 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1373,7 +1373,7 @@ std::error_code RedirectingFileSystem::isLocal(const Twine &Path_, SmallString<256> Path; Path_.toVector(Path); - if (std::error_code EC = makeCanonical(Path)) + if (makeAbsolute(Path)) return {}; return ExternalFS->isLocal(Path, Result); @@ -1440,7 +1440,7 @@ directory_iterator RedirectingFileSystem::dir_begin(const Twine &Dir, SmallString<256> Path; Dir.toVector(Path); - EC = makeCanonical(Path); + EC = makeAbsolute(Path); if (EC) return {}; @@ -2290,8 +2290,8 @@ void RedirectingFileSystem::LookupResult::getPath( llvm::sys::path::append(Result, E->getName()); } -std::error_code -RedirectingFileSystem::makeCanonical(SmallVectorImpl &Path) const { +std::error_code RedirectingFileSystem::makeCanonicalForLookup( + SmallVectorImpl &Path) const { if (std::error_code EC = makeAbsolute(Path)) return EC; @@ -2306,12 +2306,16 @@ RedirectingFileSystem::makeCanonical(SmallVectorImpl &Path) const { ErrorOr RedirectingFileSystem::lookupPath(StringRef Path) const { + llvm::SmallString<128> CanonicalPath(Path); + if (std::error_code EC = makeCanonicalForLookup(CanonicalPath)) + return EC; + // RedirectOnly means the VFS is always used. if (UsageTrackingActive && Redirection == RedirectKind::RedirectOnly) HasBeenUsed = true; - sys::path::const_iterator Start = sys::path::begin(Path); - sys::path::const_iterator End = sys::path::end(Path); + sys::path::const_iterator Start = sys::path::begin(CanonicalPath); + sys::path::const_iterator End = sys::path::end(CanonicalPath); llvm::SmallVector Entries; for (const auto &Root : Roots) { ErrorOr Result = @@ -2388,14 +2392,14 @@ static Status getRedirectedFileStatus(const Twine &OriginalPath, } ErrorOr RedirectingFileSystem::status( - const Twine &CanonicalPath, const Twine &OriginalPath, + const Twine &LookupPath, const Twine &OriginalPath, const RedirectingFileSystem::LookupResult &Result) { if (std::optional ExtRedirect = Result.getExternalRedirect()) { - SmallString<256> CanonicalRemappedPath((*ExtRedirect).str()); - if (std::error_code EC = makeCanonical(CanonicalRemappedPath)) + SmallString<256> RemappedPath((*ExtRedirect).str()); + if (std::error_code EC = makeAbsolute(RemappedPath)) return EC; - ErrorOr S = ExternalFS->status(CanonicalRemappedPath); + ErrorOr S = ExternalFS->status(RemappedPath); if (!S) return S; S = Status::copyWithNewName(*S, *ExtRedirect); @@ -2405,13 +2409,13 @@ ErrorOr RedirectingFileSystem::status( } auto *DE = cast(Result.E); - return Status::copyWithNewName(DE->getStatus(), CanonicalPath); + return Status::copyWithNewName(DE->getStatus(), LookupPath); } ErrorOr -RedirectingFileSystem::getExternalStatus(const Twine &CanonicalPath, +RedirectingFileSystem::getExternalStatus(const Twine &LookupPath, const Twine &OriginalPath) const { - auto Result = ExternalFS->status(CanonicalPath); + auto Result = ExternalFS->status(LookupPath); // The path has been mapped by some nested VFS, don't override it with the // original path. @@ -2421,65 +2425,63 @@ RedirectingFileSystem::getExternalStatus(const Twine &CanonicalPath, } ErrorOr RedirectingFileSystem::status(const Twine &OriginalPath) { - SmallString<256> CanonicalPath; - OriginalPath.toVector(CanonicalPath); + SmallString<256> Path; + OriginalPath.toVector(Path); - if (std::error_code EC = makeCanonical(CanonicalPath)) + if (std::error_code EC = makeAbsolute(Path)) return EC; if (Redirection == RedirectKind::Fallback) { // Attempt to find the original file first, only falling back to the // mapped file if that fails. - ErrorOr S = getExternalStatus(CanonicalPath, OriginalPath); + ErrorOr S = getExternalStatus(Path, OriginalPath); if (S) return S; } - ErrorOr Result = - lookupPath(CanonicalPath); + ErrorOr Result = lookupPath(Path); if (!Result) { // Was not able to map file, fallthrough to using the original path if // that was the specified redirection type. if (Redirection == RedirectKind::Fallthrough && isFileNotFound(Result.getError())) - return getExternalStatus(CanonicalPath, OriginalPath); + return getExternalStatus(Path, OriginalPath); return Result.getError(); } - ErrorOr S = status(CanonicalPath, OriginalPath, *Result); + ErrorOr S = status(Path, OriginalPath, *Result); if (!S && Redirection == RedirectKind::Fallthrough && isFileNotFound(S.getError(), Result->E)) { // Mapped the file but it wasn't found in the underlying filesystem, // fallthrough to using the original path if that was the specified // redirection type. - return getExternalStatus(CanonicalPath, OriginalPath); + return getExternalStatus(Path, OriginalPath); } return S; } bool RedirectingFileSystem::exists(const Twine &OriginalPath) { - SmallString<256> CanonicalPath; - OriginalPath.toVector(CanonicalPath); + SmallString<256> Path; + OriginalPath.toVector(Path); - if (makeCanonical(CanonicalPath)) + if (makeAbsolute(Path)) return false; if (Redirection == RedirectKind::Fallback) { // Attempt to find the original file first, only falling back to the // mapped file if that fails. - if (ExternalFS->exists(CanonicalPath)) + if (ExternalFS->exists(Path)) return true; } - ErrorOr Result = - lookupPath(CanonicalPath); + ErrorOr Result = lookupPath(Path); if (!Result) { // Was not able to map file, fallthrough to using the original path if // that was the specified redirection type. if (Redirection == RedirectKind::Fallthrough && isFileNotFound(Result.getError())) - return ExternalFS->exists(CanonicalPath); + return ExternalFS->exists(Path); return false; } @@ -2489,18 +2491,18 @@ bool RedirectingFileSystem::exists(const Twine &OriginalPath) { return true; } - SmallString<256> CanonicalRemappedPath((*ExtRedirect).str()); - if (makeCanonical(CanonicalRemappedPath)) + SmallString<256> RemappedPath((*ExtRedirect).str()); + if (makeAbsolute(RemappedPath)) return false; - if (ExternalFS->exists(CanonicalRemappedPath)) + if (ExternalFS->exists(RemappedPath)) return true; if (Redirection == RedirectKind::Fallthrough) { // Mapped the file but it wasn't found in the underlying filesystem, // fallthrough to using the original path if that was the specified // redirection type. - return ExternalFS->exists(CanonicalPath); + return ExternalFS->exists(Path); } return false; @@ -2549,30 +2551,27 @@ File::getWithPath(ErrorOr> Result, const Twine &P) { ErrorOr> RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { - SmallString<256> CanonicalPath; - OriginalPath.toVector(CanonicalPath); + SmallString<256> Path; + OriginalPath.toVector(Path); - if (std::error_code EC = makeCanonical(CanonicalPath)) + if (std::error_code EC = makeAbsolute(Path)) return EC; if (Redirection == RedirectKind::Fallback) { // Attempt to find the original file first, only falling back to the // mapped file if that fails. - auto F = File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), - OriginalPath); + auto F = File::getWithPath(ExternalFS->openFileForRead(Path), OriginalPath); if (F) return F; } - ErrorOr Result = - lookupPath(CanonicalPath); + ErrorOr Result = lookupPath(Path); if (!Result) { // Was not able to map file, fallthrough to using the original path if // that was the specified redirection type. if (Redirection == RedirectKind::Fallthrough && isFileNotFound(Result.getError())) - return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), - OriginalPath); + return File::getWithPath(ExternalFS->openFileForRead(Path), OriginalPath); return Result.getError(); } @@ -2580,22 +2579,21 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { return make_error_code(llvm::errc::invalid_argument); StringRef ExtRedirect = *Result->getExternalRedirect(); - SmallString<256> CanonicalRemappedPath(ExtRedirect.str()); - if (std::error_code EC = makeCanonical(CanonicalRemappedPath)) + SmallString<256> RemappedPath(ExtRedirect.str()); + if (std::error_code EC = makeAbsolute(RemappedPath)) return EC; auto *RE = cast(Result->E); - auto ExternalFile = File::getWithPath( - ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect); + auto ExternalFile = + File::getWithPath(ExternalFS->openFileForRead(RemappedPath), ExtRedirect); if (!ExternalFile) { if (Redirection == RedirectKind::Fallthrough && isFileNotFound(ExternalFile.getError(), Result->E)) { // Mapped the file but it wasn't found in the underlying filesystem, // fallthrough to using the original path if that was the specified // redirection type. - return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), - OriginalPath); + return File::getWithPath(ExternalFS->openFileForRead(Path), OriginalPath); } return ExternalFile; } @@ -2615,28 +2613,27 @@ RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { std::error_code RedirectingFileSystem::getRealPath(const Twine &OriginalPath, SmallVectorImpl &Output) const { - SmallString<256> CanonicalPath; - OriginalPath.toVector(CanonicalPath); + SmallString<256> Path; + OriginalPath.toVector(Path); - if (std::error_code EC = makeCanonical(CanonicalPath)) + if (std::error_code EC = makeAbsolute(Path)) return EC; if (Redirection == RedirectKind::Fallback) { // Attempt to find the original file first, only falling back to the // mapped file if that fails. - std::error_code EC = ExternalFS->getRealPath(CanonicalPath, Output); + std::error_code EC = ExternalFS->getRealPath(Path, Output); if (!EC) return EC; } - ErrorOr Result = - lookupPath(CanonicalPath); + ErrorOr Result = lookupPath(Path); if (!Result) { // Was not able to map file, fallthrough to using the original path if // that was the specified redirection type. if (Redirection == RedirectKind::Fallthrough && isFileNotFound(Result.getError())) - return ExternalFS->getRealPath(CanonicalPath, Output); + return ExternalFS->getRealPath(Path, Output); return Result.getError(); } @@ -2649,7 +2646,7 @@ RedirectingFileSystem::getRealPath(const Twine &OriginalPath, // Mapped the file but it wasn't found in the underlying filesystem, // fallthrough to using the original path if that was the specified // redirection type. - return ExternalFS->getRealPath(CanonicalPath, Output); + return ExternalFS->getRealPath(Path, Output); } return P; } diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp index a85ab3fe0a799..3e7a7b3007c2f 100644 --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -7,9 +7,11 @@ //===----------------------------------------------------------------------===// #include "llvm/Support/VirtualFileSystem.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/Config/llvm-config.h" #include "llvm/Support/Errc.h" +#include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/SourceMgr.h" @@ -3509,3 +3511,71 @@ TEST(RedirectingFileSystemTest, Used) { EXPECT_TRUE(Redirecting1->hasBeenUsed()); EXPECT_FALSE(Redirecting2->hasBeenUsed()); } + +// Check that paths looked up in the external filesystem are unmodified, except +// potentially to add the working directory. We cannot canonicalize away .. +// in the presence of symlinks in the external filesystem. +TEST(RedirectingFileSystemTest, ExternalPaths) { + struct InterceptorFS : llvm::vfs::ProxyFileSystem { + std::vector SeenPaths; + + InterceptorFS(IntrusiveRefCntPtr UnderlyingFS) + : ProxyFileSystem(UnderlyingFS) {} + + llvm::ErrorOr status(const Twine &Path) override { + SeenPaths.push_back(Path.str()); + return ProxyFileSystem::status(Path); + } + + llvm::ErrorOr> + openFileForRead(const Twine &Path) override { + SeenPaths.push_back(Path.str()); + return ProxyFileSystem::openFileForRead(Path); + } + + std::error_code isLocal(const Twine &Path, bool &Result) override { + SeenPaths.push_back(Path.str()); + return ProxyFileSystem::isLocal(Path, Result); + } + + vfs::directory_iterator dir_begin(const Twine &Dir, + std::error_code &EC) override { + SeenPaths.push_back(Dir.str()); + return ProxyFileSystem::dir_begin(Dir, EC); + } + + bool exists(const Twine &Path) override { + SeenPaths.push_back(Path.str()); + return ProxyFileSystem::exists(Path); + } + }; + + std::error_code EC; + auto BaseFS = makeIntrusiveRefCnt(); + BaseFS->setCurrentWorkingDirectory("/cwd"); + auto CheckFS = makeIntrusiveRefCnt(BaseFS); + auto FS = vfs::RedirectingFileSystem::create({}, /*UseExternalNames=*/false, + *CheckFS); + + FS->status("/a/../b"); + FS->openFileForRead("c"); + FS->exists("./d"); + bool IsLocal = false; + FS->isLocal("/e/./../f", IsLocal); + FS->dir_begin(".././g", EC); + + std::vector Expected{"/a/../b", "/cwd/c", "/cwd/./d", + "/e/./../f", "/cwd/.././g"}; + + EXPECT_EQ(CheckFS->SeenPaths, Expected); + + CheckFS->SeenPaths.clear(); + FS->setRedirection(vfs::RedirectingFileSystem::RedirectKind::Fallback); + FS->status("/a/../b"); + FS->openFileForRead("c"); + FS->exists("./d"); + FS->isLocal("/e/./../f", IsLocal); + FS->dir_begin(".././g", EC); + + EXPECT_EQ(CheckFS->SeenPaths, Expected); +}