FileSystem::EnumerateDirectory should keep iterating if one entry has an invalid Status

[jasonmolenda]

Summary

EnumerateDirectory gets the vfs::Status of each file entry, to check if we are asked to report on this file type, and if we can't get the Status, it stops the directory search. Instead, if it can't get the Status of a directory entry, it should skip that entry and continue searching.

This happens with a broken symlink - the link points to another file, but that file doesn't exist, and the Status returned is of the destination file. When we can't get the destination file Status, an error code is returned.

Add a test case to FileSystemTests unittest.

[JDevlieghere]

LGTM modulo capitalization.

lldb/unittests/Host/FileSystemTest.cpp

54

Comments should start with a capital.

lldb/source/Host/common/FileSystem.cpp

void FileSystem::EnumerateDirectory(Twine path, bool find_directories,
                                     void *callback_baton) {
   std::error_code EC;
   vfs::recursive_directory_iterator Iter(*m_fs, path, EC);
   vfs::recursive_directory_iterator End;
   for (; Iter != End && !EC; Iter.increment(EC)) {
     const auto &Item = *Iter;
     ErrorOr<vfs::Status> Status = m_fs->status(Item.path());
     if (!Status)
-      break;
+      continue;
     if (!find_files && Status->isRegularFile())
       continue;
     if (!find_directories && Status->isDirectory())
       continue;
     if (!find_other && Status->isOther())
       continue;
 
     auto Result = callback(callback_baton, Status->getType(), Item.path());

lldb/unittests/Host/FileSystemTest.cpp

 public:
   DummyFileSystem() : FSID(getNextFSID()) {}
 
   ErrorOr<vfs::Status> status(const Twine &Path) override {
     std::map<std::string, vfs::Status>::iterator I =
         FilesAndDirs.find(Path.str());
     if (I == FilesAndDirs.end())
       return make_error_code(llvm::errc::no_such_file_or_directory);
+    // Simulate a broken symlink, where it points to a file/dir that
+    // does not exist.
+    if (I->second.isSymlink() &&
+        I->second.getPermissions() == sys::fs::perms::no_perms)
+      return std::error_code(ENOENT, std::generic_category());
     return I->second;
   }
   ErrorOr<std::unique_ptr<vfs::File>>
   openFileForRead(const Twine &Path) override {
     auto S = status(Path);
     if (S)
       return std::unique_ptr<vfs::File>(new DummyFile{*S});
     return S.getError();
public:
   }
 
   void addSymlink(StringRef Path) {
     vfs::Status S(Path, UniqueID(FSID, FileID++),
                   std::chrono::system_clock::now(), 0, 0, 0,
                   sys::fs::file_type::symlink_file, sys::fs::all_all);
     addEntry(Path, S);
   }
+
+  void addBrokenSymlink(StringRef Path) {
+    vfs::Status S(Path, UniqueID(FSID, FileID++),
+                  std::chrono::system_clock::now(), 0, 0, 0,
+                  sys::fs::file_type::symlink_file, sys::fs::no_perms);
+    addEntry(Path, S);
+  }
 };
 } // namespace
 
 TEST(FileSystemTest, FileAndDirectoryComponents) {
   using namespace std::chrono;
   FileSystem fs;
 
 #ifdef _WIN32
EXPECT_LT(system_clock::time_point() + hours(24 * 365 * 20),
             fs.GetModificationTime(fs2));
 }
 
 static IntrusiveRefCntPtr<DummyFileSystem> GetSimpleDummyFS() {
   IntrusiveRefCntPtr<DummyFileSystem> D(new DummyFileSystem());
   D->addRegularFile("/foo");
   D->addDirectory("/bar");
   D->addSymlink("/baz");
+  D->addBrokenSymlink("/lux");
   D->addRegularFile("/qux", ~sys::fs::perms::all_read);
   D->setCurrentWorkingDirectory("/");
   return D;
 }
 
 TEST(FileSystemTest, Exists) {
   FileSystem fs(GetSimpleDummyFS());