[WasmFS] Return errors from getEntries and getNumEntries (#17807)

Make the return type of `getEntries` a variant that can be either a vector of
entries as before or a negative error code. Update the return type of
`getNumEntries` to be a ssize_t so it can return negative error codes as well.
This caps the number of directory entries that can be reported at around two
billion, but that should be sufficient.

I have manually tested via an injected error that the error handling works
correctly for the OPFS backend, although I was not able to construct a test case
for this.

Author: tlively

src/library_wasmfs_opfs.js

@@ -149,21 +149,25 @@ mergeInto(LibraryManager.library, {
   },
 
   _wasmfs_opfs_get_entries__deps: [],
-  _wasmfs_opfs_get_entries: async function(ctx, dirID, entries) {
+  _wasmfs_opfs_get_entries: async function(ctx, dirID, entriesPtr, errPtr) {
     let dirHandle = wasmfsOPFSDirectoryHandles.get(dirID);
 
     // TODO: Use 'for await' once Acorn supports that.
-    // TODO: Error handling.
-    let iter = dirHandle.entries();
-    for (let entry; entry = await iter.next(), !entry.done;) {
-      let [name, child] = entry.value;
-      withStackSave(() => {
-        let namePtr = allocateUTF8OnStack(name);
-        let type = child.kind == "file" ?
-            {{{ cDefine('File::DataFileKind') }}} :
-            {{{ cDefine('File::DirectoryKind') }}};
-        __wasmfs_opfs_record_entry(entries, namePtr, type)
-      });
+    try {
+      let iter = dirHandle.entries();
+      for (let entry; entry = await iter.next(), !entry.done;) {
+        let [name, child] = entry.value;
+        withStackSave(() => {
+          let namePtr = allocateUTF8OnStack(name);
+          let type = child.kind == "file" ?
+              {{{ cDefine('File::DataFileKind') }}} :
+          {{{ cDefine('File::DirectoryKind') }}};
+          __wasmfs_opfs_record_entry(entriesPtr, namePtr, type)
+        });
+      }
+    } catch {
+      let err = -{{{ cDefine('EIO') }}};
+      {{{ makeSetValue('errPtr', 0, 'err', 'i32') }}};
     }
     _emscripten_proxy_finish(ctx);
   },

system/lib/wasmfs/backends/memory_backend.cpp

@@ -54,13 +54,13 @@ int MemoryDirectory::removeChild(const std::string& name) {
   return 0;
 }
 
-std::vector<Directory::Entry> MemoryDirectory::getEntries() {
+Directory::MaybeEntries MemoryDirectory::getEntries() {
   std::vector<Directory::Entry> result;
   result.reserve(entries.size());
   for (auto& [name, child] : entries) {
     result.push_back({name, child->kind, child->getIno()});
   }
-  return result;
+  return {result};
 }
 
 int MemoryDirectory::insertMove(const std::string& name,

system/lib/wasmfs/backends/node_backend.cpp

@@ -258,21 +258,22 @@ class NodeDirectory : public Directory {
     abort();
   }
 
-  size_t getNumEntries() override {
+  ssize_t getNumEntries() override {
     // TODO: optimize this?
-    return getEntries().size();
+    auto entries = getEntries();
+    if (int err = entries.getError()) {
+      return err;
+    }
+    return entries->size();
   }
 
-  std::vector<Directory::Entry> getEntries() override {
+  Directory::MaybeEntries getEntries() override {
     std::vector<Directory::Entry> entries;
     int err = _wasmfs_node_readdir(state.path.c_str(), &entries);
-    // TODO: Make this fallible. We actually depend on suppressing the error
-    //       here to pass test_unlink_wasmfs_node because the File stored in the
-    //       file table is not the same File that had its parent pointer reset
-    //       during the unlink. Fixing this may require caching Files at some
-    //       layer to ensure they are the same.
-    (void)err;
-    return entries;
+    if (err) {
+      return {-err};
+    }
+    return {entries};
   }
 };
 

system/lib/wasmfs/backends/opfs_backend.cpp

@@ -52,7 +52,8 @@ void _wasmfs_opfs_remove_child(em_proxying_ctx* ctx,
 
 void _wasmfs_opfs_get_entries(em_proxying_ctx* ctx,
                               int dirID,
-                              std::vector<Directory::Entry>* entries);
+                              std::vector<Directory::Entry>* entries,
+                              int* err);
 
 void _wasmfs_opfs_open_access(em_proxying_ctx* ctx,
                               int file_id,
@@ -420,13 +421,25 @@ class OPFSDirectory : public Directory {
     return err;
   }
 
-  size_t getNumEntries() override { return getEntries().size(); }
+  ssize_t getNumEntries() override {
+    auto entries = getEntries();
+    if (int err = entries.getError()) {
+      return err;
+    }
+    return entries->size();
+  }
 
-  std::vector<Directory::Entry> getEntries() override {
+  Directory::MaybeEntries getEntries() override {
     std::vector<Directory::Entry> entries;
-    proxy(
-      [&](auto ctx) { _wasmfs_opfs_get_entries(ctx.ctx, dirID, &entries); });
-    return entries;
+    int err = 0;
+    proxy([&](auto ctx) {
+      _wasmfs_opfs_get_entries(ctx.ctx, dirID, &entries, &err);
+    });
+    if (err) {
+      assert(err < 0);
+      return {err};
+    }
+    return {entries};
   }
 };
 

system/lib/wasmfs/file.cpp

@@ -225,24 +225,31 @@ std::string Directory::Handle::getName(std::shared_ptr<File> file) {
   return "";
 }
 
-size_t Directory::Handle::getNumEntries() {
+ssize_t Directory::Handle::getNumEntries() {
   size_t mounts = 0;
   auto& dcache = getDir()->dcache;
   for (auto it = dcache.begin(); it != dcache.end(); ++it) {
     if (it->second.kind == DCacheKind::Mount) {
       ++mounts;
     }
   }
-  return getDir()->getNumEntries() + mounts;
+  auto numReal = getDir()->getNumEntries();
+  if (numReal < 0) {
+    return numReal;
+  }
+  return numReal + mounts;
 }
 
-std::vector<Directory::Entry> Directory::Handle::getEntries() {
+Directory::MaybeEntries Directory::Handle::getEntries() {
   auto entries = getDir()->getEntries();
+  if (entries.getError()) {
+    return entries;
+  }
   auto& dcache = getDir()->dcache;
   for (auto it = dcache.begin(); it != dcache.end(); ++it) {
     auto& [name, entry] = *it;
     if (entry.kind == DCacheKind::Mount) {
-      entries.push_back({name, entry.file->kind, entry.file->getIno()});
+      entries->push_back({name, entry.file->kind, entry.file->getIno()});
     }
   }
   return entries;

system/lib/wasmfs/file.h

@@ -16,6 +16,7 @@
 #include <mutex>
 #include <optional>
 #include <sys/stat.h>
+#include <variant>
 #include <vector>
 #include <wasi/api.h>
 
@@ -171,6 +172,20 @@ class Directory : public File {
     ino_t ino;
   };
 
+  struct MaybeEntries : std::variant<std::vector<Entry>, int> {
+    int getError() {
+      if (int* err = std::get_if<int>(this)) {
+        assert(*err < 0);
+        return *err;
+      }
+      return 0;
+    }
+
+    std::vector<Entry>* operator->() {
+      return std::get_if<std::vector<Entry>>(this);
+    }
+  };
+
 private:
   // The directory cache, or `dcache`, stores `File` objects for the children of
   // each directory so that subsequent lookups do not need to query the backend.
@@ -213,11 +228,12 @@ class Directory : public File {
   // if the child cannot be removed.
   virtual int removeChild(const std::string& name) = 0;
 
-  // The number of entries in this directory.
-  virtual size_t getNumEntries() = 0;
+  // The number of entries in this directory. Returns the number of entries or a
+  // negative error code.
+  virtual ssize_t getNumEntries() = 0;
 
-  // The list of entries in this directory.
-  virtual std::vector<Directory::Entry> getEntries() = 0;
+  // The list of entries in this directory or a negative error code.
+  virtual MaybeEntries getEntries() = 0;
 
   // Only backends that maintain file identity themselves (see below) need to
   // implement this.
@@ -384,8 +400,8 @@ class Directory::Handle : public File::Handle {
 
   std::string getName(std::shared_ptr<File> file);
 
-  size_t getNumEntries();
-  std::vector<Directory::Entry> getEntries();
+  [[nodiscard]] ssize_t getNumEntries();
+  [[nodiscard]] MaybeEntries getEntries();
 };
 
 inline File::Handle File::locked() { return Handle(shared_from_this()); }

system/lib/wasmfs/memory_backend.h

@@ -88,8 +88,8 @@ class MemoryDirectory : public Directory {
 
   int insertMove(const std::string& name, std::shared_ptr<File> file) override;
 
-  size_t getNumEntries() override { return entries.size(); }
-  std::vector<Directory::Entry> getEntries() override;
+  ssize_t getNumEntries() override { return entries.size(); }
+  Directory::MaybeEntries getEntries() override;
 
   std::string getName(std::shared_ptr<File> file) override;
 

system/lib/wasmfs/syscalls.cpp

@@ -879,7 +879,10 @@ int __syscall_getdents64(int fd, intptr_t dirp, size_t count) {
     {".", File::DirectoryKind, dir->getIno()},
     {"..", File::DirectoryKind, parent->getIno()}};
   auto dirEntries = lockedDir.getEntries();
-  entries.insert(entries.end(), dirEntries.begin(), dirEntries.end());
+  if (int err = dirEntries.getError()) {
+    return err;
+  }
+  entries.insert(entries.end(), dirEntries->begin(), dirEntries->end());
 
   off_t bytesRead = 0;
   for (; index < entries.size() && bytesRead + sizeof(dirent) <= count;