[Lldb-commits] [lldb] d144087 - [lldb/Support] Treat empty FileSpec as an invalid file.

[Jonas Devlieghere via lldb-commits]

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));
+}