Author: Jonas Devlieghere Date: 2020-04-03T09:29:22-07:00 New Revision: d144087c963d8189bb4aeaa7800dcb9f93ac84db
URL: https://github.com/llvm/llvm-project/commit/d144087c963d8189bb4aeaa7800dcb9f93ac84db DIFF: https://github.com/llvm/llvm-project/commit/d144087c963d8189bb4aeaa7800dcb9f93ac84db.diff LOG: [lldb/Support] Treat empty FileSpec as an invalid file. LLDB relies on empty FileSpecs being invalid files, for example, they don't exists. Currently this assumption does not always hold during reproducer replay, because we pass the result of GetPath to the VFS. This is an empty string, which the VFS converts to an absolute directory by prepending the current working directory, before looking it up in the YAML mapping. This means that an empty FileSpec will exist when the current working directory does. This breaks at least one test (TestAddDsymCommand.py) when ran from replay. This patch special cases empty FileSpecs and returns a sensible result before calling GetPath and forwarding the call. Differential revision: https://reviews.llvm.org/D77351 Added: Modified: lldb/source/Host/common/FileSystem.cpp lldb/unittests/Host/FileSystemTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp index dcfa594597a1..7e33eeb2a812 100644 --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -87,6 +87,11 @@ Optional<FileSystem> &FileSystem::InstanceImpl() { vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec, std::error_code &ec) { + if (!file_spec) { + ec = std::error_code(static_cast<int>(errc::no_such_file_or_directory), + std::system_category()); + return {}; + } return DirBegin(file_spec.GetPath(), ec); } @@ -97,6 +102,9 @@ vfs::directory_iterator FileSystem::DirBegin(const Twine &dir, llvm::ErrorOr<vfs::Status> FileSystem::GetStatus(const FileSpec &file_spec) const { + if (!file_spec) + return std::error_code(static_cast<int>(errc::no_such_file_or_directory), + std::system_category()); return GetStatus(file_spec.GetPath()); } @@ -106,6 +114,8 @@ llvm::ErrorOr<vfs::Status> FileSystem::GetStatus(const Twine &path) const { sys::TimePoint<> FileSystem::GetModificationTime(const FileSpec &file_spec) const { + if (!file_spec) + return sys::TimePoint<>(); return GetModificationTime(file_spec.GetPath()); } @@ -117,6 +127,8 @@ sys::TimePoint<> FileSystem::GetModificationTime(const Twine &path) const { } uint64_t FileSystem::GetByteSize(const FileSpec &file_spec) const { + if (!file_spec) + return 0; return GetByteSize(file_spec.GetPath()); } @@ -133,6 +145,8 @@ uint32_t FileSystem::GetPermissions(const FileSpec &file_spec) const { uint32_t FileSystem::GetPermissions(const FileSpec &file_spec, std::error_code &ec) const { + if (!file_spec) + return sys::fs::perms::perms_not_known; return GetPermissions(file_spec.GetPath(), ec); } @@ -154,7 +168,7 @@ uint32_t FileSystem::GetPermissions(const Twine &path, bool FileSystem::Exists(const Twine &path) const { return m_fs->exists(path); } bool FileSystem::Exists(const FileSpec &file_spec) const { - return Exists(file_spec.GetPath()); + return file_spec && Exists(file_spec.GetPath()); } bool FileSystem::Readable(const Twine &path) const { @@ -162,7 +176,7 @@ bool FileSystem::Readable(const Twine &path) const { } bool FileSystem::Readable(const FileSpec &file_spec) const { - return Readable(file_spec.GetPath()); + return file_spec && Readable(file_spec.GetPath()); } bool FileSystem::IsDirectory(const Twine &path) const { @@ -173,7 +187,7 @@ bool FileSystem::IsDirectory(const Twine &path) const { } bool FileSystem::IsDirectory(const FileSpec &file_spec) const { - return IsDirectory(file_spec.GetPath()); + return file_spec && IsDirectory(file_spec.GetPath()); } bool FileSystem::IsLocal(const Twine &path) const { @@ -183,7 +197,7 @@ bool FileSystem::IsLocal(const Twine &path) const { } bool FileSystem::IsLocal(const FileSpec &file_spec) const { - return IsLocal(file_spec.GetPath()); + return file_spec && IsLocal(file_spec.GetPath()); } void FileSystem::EnumerateDirectory(Twine path, bool find_directories, @@ -261,6 +275,9 @@ void FileSystem::Resolve(SmallVectorImpl<char> &path) { } void FileSystem::Resolve(FileSpec &file_spec) { + if (!file_spec) + return; + // Extract path from the FileSpec. SmallString<128> path; file_spec.GetPath(path); diff --git a/lldb/unittests/Host/FileSystemTest.cpp b/lldb/unittests/Host/FileSystemTest.cpp index d8db2c12b0ee..f31c5c42a1e1 100644 --- a/lldb/unittests/Host/FileSystemTest.cpp +++ b/lldb/unittests/Host/FileSystemTest.cpp @@ -303,3 +303,29 @@ TEST(FileSystemTest, OpenErrno) { EXPECT_EQ(code.value(), ENOENT); } +TEST(FileSystemTest, EmptyTest) { + FileSpec spec; + FileSystem fs; + + { + std::error_code ec; + fs.DirBegin(spec, ec); + EXPECT_EQ(ec.category(), std::system_category()); + EXPECT_EQ(ec.value(), ENOENT); + } + + { + llvm::ErrorOr<vfs::Status> status = fs.GetStatus(spec); + ASSERT_FALSE(status); + EXPECT_EQ(status.getError().category(), std::system_category()); + EXPECT_EQ(status.getError().value(), ENOENT); + } + + EXPECT_EQ(sys::TimePoint<>(), fs.GetModificationTime(spec)); + EXPECT_EQ(static_cast<uint64_t>(0), fs.GetByteSize(spec)); + EXPECT_EQ(llvm::sys::fs::perms::perms_not_known, fs.GetPermissions(spec)); + EXPECT_FALSE(fs.Exists(spec)); + EXPECT_FALSE(fs.Readable(spec)); + EXPECT_FALSE(fs.IsDirectory(spec)); + EXPECT_FALSE(fs.IsLocal(spec)); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits