diff --git a/clang/test/VFS/relative-path-errors.c b/clang/test/VFS/relative-path-errors.c new file mode 100644 --- /dev/null +++ b/clang/test/VFS/relative-path-errors.c @@ -0,0 +1,11 @@ +// RUN: mkdir -p %t +// RUN: cd %t +// RUN: cp %s main.c +// RUN: not %clang_cc1 main.c 2>&1 | FileCheck %s +// RUN: echo '{"roots": [],"version": 0}' > %t.yaml +// RUN: not %clang_cc1 -ivfsoverlay %t.yaml main.c 2>&1 | FileCheck %s +// RUN: echo '{"version": 0,"roots":[{"type":"directory","name":"%/t","contents":[{"type":"file","name":"vfsname", "external-contents":"main.c"}]}]}' > %t.yaml +// RUN: not %clang_cc1 -ivfsoverlay %t.yaml vfsname 2>&1 | FileCheck %s + +// CHECK: {{^}}main.c:[[# @LINE + 1]]:1: error: +foobarbaz diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -121,6 +121,14 @@ /// Closes the file. virtual std::error_code close() = 0; + + // Get the same file with a different path. + static ErrorOr> + getWithPath(ErrorOr> Result, const Twine &P); + +protected: + // Set the file's underlying path. + virtual void setPath(const Twine &Path) {} }; /// A member of a directory, yielded by a directory_iterator. @@ -757,6 +765,12 @@ /// with the given error code on a path associated with the provided Entry. bool shouldFallBackToExternalFS(std::error_code EC, Entry *E = nullptr) 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, + const Twine &OriginalPath) const; + // In a RedirectingFileSystem, keys can be specified in Posix or Windows // style (or even a mixture of both), so this comparison helper allows // slashes (representing a root) to match backslashes (and vice versa). Note @@ -819,7 +833,8 @@ Entry *From) const; /// Get the status for a path with the provided \c LookupResult. - ErrorOr status(const Twine &Path, const LookupResult &Result); + ErrorOr status(const Twine &CanonicalPath, const Twine &OriginalPath, + const LookupResult &Result); public: /// Looks up \p Path in \c Roots and returns a LookupResult giving the diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -194,6 +194,7 @@ bool RequiresNullTerminator, bool IsVolatile) override; std::error_code close() override; + void setPath(const Twine &Path) override; }; } // namespace @@ -229,6 +230,12 @@ return EC; } +void RealFile::setPath(const Twine &Path) { + RealName = Path.str(); + if (auto Status = status()) + S = Status.get().copyWithNewName(Status.get(), Path); +} + namespace { /// A file system according to your operating system. @@ -639,6 +646,8 @@ } std::error_code close() override { return {}; } + + void setPath(const Twine &Path) override { RequestedName = Path.str(); } }; } // namespace @@ -1233,7 +1242,7 @@ } // Use status to make sure the path exists and refers to a directory. - ErrorOr S = status(Path, *Result); + ErrorOr S = status(Path, Dir, *Result); if (!S) { if (shouldFallBackToExternalFS(S.getError(), Result->E)) return ExternalFS->dir_begin(Dir, EC); @@ -1959,47 +1968,68 @@ return make_error_code(llvm::errc::no_such_file_or_directory); } -static Status getRedirectedFileStatus(const Twine &Path, bool UseExternalNames, +static Status getRedirectedFileStatus(const Twine &OriginalPath, + bool UseExternalNames, Status ExternalStatus) { Status S = ExternalStatus; if (!UseExternalNames) - S = Status::copyWithNewName(S, Path); + S = Status::copyWithNewName(S, OriginalPath); S.IsVFSMapped = true; return S; } ErrorOr RedirectingFileSystem::status( - const Twine &Path, const RedirectingFileSystem::LookupResult &Result) { + const Twine &CanonicalPath, const Twine &OriginalPath, + const RedirectingFileSystem::LookupResult &Result) { if (Optional ExtRedirect = Result.getExternalRedirect()) { - ErrorOr S = ExternalFS->status(*ExtRedirect); + SmallString<256> CanonicalRemappedPath((*ExtRedirect).str()); + if (std::error_code EC = makeCanonical(CanonicalRemappedPath)) + return EC; + + ErrorOr S = ExternalFS->status(CanonicalRemappedPath); if (!S) return S; + S = Status::copyWithNewName(*S, *ExtRedirect); auto *RE = cast(Result.E); - return getRedirectedFileStatus(Path, RE->useExternalName(UseExternalNames), - *S); + return getRedirectedFileStatus(OriginalPath, + RE->useExternalName(UseExternalNames), *S); } auto *DE = cast(Result.E); - return Status::copyWithNewName(DE->getStatus(), Path); + return Status::copyWithNewName(DE->getStatus(), CanonicalPath); } -ErrorOr RedirectingFileSystem::status(const Twine &Path_) { - SmallString<256> Path; - Path_.toVector(Path); +ErrorOr +RedirectingFileSystem::getExternalStatus(const Twine &CanonicalPath, + const Twine &OriginalPath) const { + if (auto Result = ExternalFS->status(CanonicalPath)) { + return Result.get().copyWithNewName(Result.get(), OriginalPath); + } else { + return Result.getError(); + } +} + +ErrorOr RedirectingFileSystem::status(const Twine &OriginalPath) { + SmallString<256> CanonicalPath; + OriginalPath.toVector(CanonicalPath); - if (std::error_code EC = makeCanonical(Path)) + if (std::error_code EC = makeCanonical(CanonicalPath)) return EC; - ErrorOr Result = lookupPath(Path); + ErrorOr Result = + lookupPath(CanonicalPath); if (!Result) { - if (shouldFallBackToExternalFS(Result.getError())) - return ExternalFS->status(Path); + if (shouldFallBackToExternalFS(Result.getError())) { + return getExternalStatus(CanonicalPath, OriginalPath); + } return Result.getError(); } - ErrorOr S = status(Path, *Result); - if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) - S = ExternalFS->status(Path); + ErrorOr S = status(CanonicalPath, OriginalPath, *Result); + if (!S && shouldFallBackToExternalFS(S.getError(), Result->E)) { + return getExternalStatus(CanonicalPath, OriginalPath); + } + return S; } @@ -2024,22 +2054,39 @@ } std::error_code close() override { return InnerFile->close(); } + + void setPath(const Twine &Path) override { S = S.copyWithNewName(S, Path); } }; } // namespace ErrorOr> -RedirectingFileSystem::openFileForRead(const Twine &Path_) { - SmallString<256> Path; - Path_.toVector(Path); +File::getWithPath(ErrorOr> Result, const Twine &P) { + if (!Result) + return Result; - if (std::error_code EC = makeCanonical(Path)) + auto F = std::move(*Result); + auto Name = F->getName(); + if (Name && Name.get() != P.str()) + F->setPath(P); + return F; +} + +ErrorOr> +RedirectingFileSystem::openFileForRead(const Twine &OriginalPath) { + SmallString<256> CanonicalPath; + OriginalPath.toVector(CanonicalPath); + + if (std::error_code EC = makeCanonical(CanonicalPath)) return EC; - ErrorOr Result = lookupPath(Path); + ErrorOr Result = + lookupPath(CanonicalPath); if (!Result) { if (shouldFallBackToExternalFS(Result.getError())) - return ExternalFS->openFileForRead(Path); + return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), + OriginalPath); + return Result.getError(); } @@ -2047,12 +2094,18 @@ return make_error_code(llvm::errc::invalid_argument); StringRef ExtRedirect = *Result->getExternalRedirect(); + SmallString<256> CanonicalRemappedPath(ExtRedirect.str()); + if (std::error_code EC = makeCanonical(CanonicalRemappedPath)) + return EC; + auto *RE = cast(Result->E); - auto ExternalFile = ExternalFS->openFileForRead(ExtRedirect); + auto ExternalFile = File::getWithPath( + ExternalFS->openFileForRead(CanonicalRemappedPath), ExtRedirect); if (!ExternalFile) { if (shouldFallBackToExternalFS(ExternalFile.getError(), Result->E)) - return ExternalFS->openFileForRead(Path); + return File::getWithPath(ExternalFS->openFileForRead(CanonicalPath), + OriginalPath); return ExternalFile; } @@ -2062,7 +2115,7 @@ // FIXME: Update the status with the name and VFSMapped. Status S = getRedirectedFileStatus( - Path, RE->useExternalName(UseExternalNames), *ExternalStatus); + OriginalPath, RE->useExternalName(UseExternalNames), *ExternalStatus); return std::unique_ptr( std::make_unique(std::move(*ExternalFile), S)); } diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -1627,6 +1627,153 @@ EXPECT_EQ(0, NumDiagnostics); } +TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) { + IntrusiveRefCntPtr BaseFS( + new vfs::InMemoryFileSystem); + BaseFS->addFile("//root/foo/a", 0, + MemoryBuffer::getMemBuffer("contents of a")); + ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo")); + auto RemappedFS = vfs::RedirectingFileSystem::create( + {}, /*UseExternalNames=*/false, *BaseFS); + + auto OpenedF = RemappedFS->openFileForRead("a"); + ASSERT_FALSE(OpenedF.getError()); + llvm::ErrorOr Name = (*OpenedF)->getName(); + ASSERT_FALSE(Name.getError()); + EXPECT_EQ("a", Name.get()); + + auto OpenedS = (*OpenedF)->status(); + ASSERT_FALSE(OpenedS.getError()); + EXPECT_EQ("a", OpenedS->getName()); + EXPECT_FALSE(OpenedS->IsVFSMapped); + + auto DirectS = RemappedFS->status("a"); + ASSERT_FALSE(DirectS.getError()); + EXPECT_EQ("a", DirectS->getName()); + EXPECT_FALSE(DirectS->IsVFSMapped); + + EXPECT_EQ(0, NumDiagnostics); +} + +TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) { + IntrusiveRefCntPtr BaseFS( + new vfs::InMemoryFileSystem); + BaseFS->addFile("//root/foo/realname", 0, + MemoryBuffer::getMemBuffer("contents of a")); + auto FS = + getFromYAMLString("{ 'use-external-names': true,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/foo',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'vfsname',\n" + " 'external-contents': 'realname'\n" + " }\n" + " ]\n" + "}]}", + BaseFS); + ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo")); + + auto OpenedF = FS->openFileForRead("vfsname"); + ASSERT_FALSE(OpenedF.getError()); + llvm::ErrorOr Name = (*OpenedF)->getName(); + ASSERT_FALSE(Name.getError()); + EXPECT_EQ("realname", Name.get()); + + auto OpenedS = (*OpenedF)->status(); + ASSERT_FALSE(OpenedS.getError()); + EXPECT_EQ("realname", OpenedS->getName()); + EXPECT_TRUE(OpenedS->IsVFSMapped); + + auto DirectS = FS->status("vfsname"); + ASSERT_FALSE(DirectS.getError()); + EXPECT_EQ("realname", DirectS->getName()); + EXPECT_TRUE(DirectS->IsVFSMapped); + + EXPECT_EQ(0, NumDiagnostics); +} + +TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) { + IntrusiveRefCntPtr BaseFS( + new vfs::InMemoryFileSystem); + BaseFS->addFile("//root/foo/realname", 0, + MemoryBuffer::getMemBuffer("contents of a")); + auto FS = + getFromYAMLString("{ 'use-external-names': false,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/foo',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'vfsname',\n" + " 'external-contents': 'realname'\n" + " }\n" + " ]\n" + "}]}", + BaseFS); + ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo")); + + auto OpenedF = FS->openFileForRead("vfsname"); + ASSERT_FALSE(OpenedF.getError()); + llvm::ErrorOr Name = (*OpenedF)->getName(); + ASSERT_FALSE(Name.getError()); + EXPECT_EQ("vfsname", Name.get()); + + auto OpenedS = (*OpenedF)->status(); + ASSERT_FALSE(OpenedS.getError()); + EXPECT_EQ("vfsname", OpenedS->getName()); + EXPECT_TRUE(OpenedS->IsVFSMapped); + + auto DirectS = FS->status("vfsname"); + ASSERT_FALSE(DirectS.getError()); + EXPECT_EQ("vfsname", DirectS->getName()); + EXPECT_TRUE(DirectS->IsVFSMapped); + + EXPECT_EQ(0, NumDiagnostics); +} + +TEST_F(VFSFromYAMLTest, RelativePathHitWithoutCWD) { + IntrusiveRefCntPtr BaseFS( + new vfs::InMemoryFileSystem); + BaseFS->addFile("//root/foo/realname", 0, + MemoryBuffer::getMemBuffer("contents of a")); + auto FS = + getFromYAMLString("{ 'use-external-names': false,\n" + " 'roots': [\n" + "{\n" + " 'type': 'directory',\n" + " 'name': '//root/foo',\n" + " 'contents': [ {\n" + " 'type': 'file',\n" + " 'name': 'vfsname',\n" + " 'external-contents': 'realname'\n" + " }\n" + " ]\n" + "}]}", + BaseFS); + + auto OpenedF = FS->openFileForRead("//root/foo/vfsname"); + ASSERT_FALSE(OpenedF.getError()); + llvm::ErrorOr Name = (*OpenedF)->getName(); + ASSERT_FALSE(Name.getError()); + EXPECT_EQ("vfsname", Name.get()); + + auto OpenedS = (*OpenedF)->status(); + ASSERT_FALSE(OpenedS.getError()); + EXPECT_EQ("vfsname", OpenedS->getName()); + EXPECT_TRUE(OpenedS->IsVFSMapped); + + auto DirectS = FS->status("vfsname"); + ASSERT_FALSE(DirectS.getError()); + EXPECT_EQ("vfsname", DirectS->getName()); + EXPECT_TRUE(DirectS->IsVFSMapped); + + EXPECT_EQ(0, NumDiagnostics); +} + TEST_F(VFSFromYAMLTest, CaseInsensitive) { IntrusiveRefCntPtr Lower(new DummyFileSystem()); Lower->addRegularFile("//root/foo/bar/a");