wasi: supports rewinding a directory and fixes seek error (#1084)

This allows rewinding a directory by specifying the cookie 0, after
already scrolling a directory. This allows zig and clang functionality
to work which expect this.

This also fixes the case where a directory was allowed to be seeked.
This is prohibited by the current version of wasi-testsuite.

Signed-off-by: Adrian Cole <[email protected]>
Co-authored-by: Takeshi Yoneda <[email protected]>

.github/workflows/integration.yaml

@@ -174,7 +174,6 @@ jobs:
             unicode/utf16 \
             unicode/utf8
 
-
   wasi-testsuite:
     name: wasi-testsuite
     runs-on: ${{ matrix.os }}

imports/wasi_snapshot_preview1/fs.go

@@ -694,9 +694,17 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		return errno
 	}
 
-	// expect a cookie only if we are continuing a read.
 	if cookie == 0 && dir.CountRead > 0 {
-		return ErrnoInval // cookie is minimally one.
+		// This means that there was a previous call to the dir, but cookie is reset.
+		// This happens when the program calls rewinddir, for example:
+		// https://github.com/WebAssembly/wasi-libc/blob/659ff414560721b1660a19685110e484a081c3d4/libc-bottom-half/cloudlibc/src/libc/dirent/rewinddir.c#L10-L12
+		//
+		// Since we cannot unwind fs.ReadDirFile results, we re-open while keeping the same file descriptor.
+		f, err := fsc.ReOpenDir(fd)
+		if err != nil {
+			return ToErrno(err)
+		}
+		rd, dir = f.File.(fs.ReadDirFile), f.ReadDir
 	}
 
 	// First, determine the maximum directory entries that can be encoded as
@@ -721,10 +729,19 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
 
 	// Check if we have maxDirEntries, and read more from the FS as needed.
 	if entryCount := len(entries); entryCount < maxDirEntries {
-		if l, err := rd.ReadDir(maxDirEntries - entryCount); err != io.EOF {
-			if err != nil {
-				return ErrnoIo
+		l, err := rd.ReadDir(maxDirEntries - entryCount)
+		if err == io.EOF { // EOF is not an error
+		} else if err != nil {
+			if errno = ToErrno(err); errno == ErrnoNoent {
+				// Only on Linux platforms, ReadDir returns ErrNotExist for the "removed while opened"
+				// directories. In order to provide consistent behavior, ignore it.
+				// See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
+				//
+				// TODO: Once we have our own File type, we should punt this into the behind ReadDir method above.
+			} else {
+				return errno
 			}
+		} else {
 			dir.CountRead += uint64(len(l))
 			entries = append(entries, l...)
 			// Replace the cache with up to maxDirEntries, starting at cookie.
@@ -783,7 +800,6 @@ func lastDirEntries(dir *sys.ReadDir, cookie int64) (entries []fs.DirEntry, errn
 	switch {
 	case cookiePos < 0: // cookie is asking for results outside our window.
 		errno = ErrnoNosys // we can't implement directory seeking backwards.
-	case cookiePos == 0: // cookie is asking for the next page.
 	case cookiePos > entryCount:
 		errno = ErrnoInval // invalid as we read that far, yet.
 	case cookiePos > 0: // truncate so to avoid large lists.
@@ -989,7 +1005,7 @@ func fdSeekFn(_ context.Context, mod api.Module, params []uint64) Errno {
 	if f, ok := fsc.LookupFile(fd); !ok {
 		return ErrnoBadf
 		// fs.FS doesn't declare io.Seeker, but implementations such as os.File implement it.
-	} else if seeker, ok = f.File.(io.Seeker); !ok {
+	} else if seeker, ok = f.File.(io.Seeker); !ok || f.IsDir() {
 		return ErrnoBadf
 	}
 

imports/wasi_snapshot_preview1/fs_test.go

@@ -1908,6 +1908,60 @@ func Test_fdReaddir(t *testing.T) {
 	}
 }
 
+func Test_fdReaddir_Rewind(t *testing.T) {
+	mod, r, _ := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.FS))
+	defer r.Close(testCtx)
+
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+
+	fd, err := fsc.OpenFile(fsc.RootFS(), "dir", os.O_RDONLY, 0)
+	require.NoError(t, err)
+
+	mem := mod.Memory()
+	const resultBufUsed, buf, bufSize = 0, 8, 100
+	read := func(cookie, bufSize uint64) (bufUsed uint32) {
+		requireErrno(t, ErrnoSuccess, mod, FdReaddirName,
+			uint64(fd), buf, bufSize, cookie, uint64(resultBufUsed))
+
+		bufUsed, ok := mem.ReadUint32Le(resultBufUsed)
+		require.True(t, ok)
+		return bufUsed
+	}
+
+	cookie := uint64(0)
+	// Initial read.
+	initialBufUsed := read(cookie, bufSize)
+	// Ensure that all is read.
+	require.Equal(t, len(dirent1)+len(dirent2)+len(dirent3), int(initialBufUsed))
+	resultBuf, ok := mem.Read(buf, initialBufUsed)
+	require.True(t, ok)
+	require.Equal(t, append(append(dirent1, dirent2...), dirent3...), resultBuf)
+
+	// Mask the result.
+	for i := range resultBuf {
+		resultBuf[i] = '?'
+	}
+
+	// Advance the cookie beyond the existing entries.
+	cookie += 3
+	// Nothing to read from, so bufUsed must be zero.
+	require.Equal(t, 0, int(read(cookie, bufSize)))
+
+	// Ensure buffer is intact.
+	for i := range resultBuf {
+		require.Equal(t, byte('?'), resultBuf[i])
+	}
+
+	// Here, we rewind the directory by setting cookie=0 on the same file descriptor.
+	cookie = 0
+	usedAfterRewind := read(cookie, bufSize)
+	// Ensure that all is read.
+	require.Equal(t, len(dirent1)+len(dirent2)+len(dirent3), int(usedAfterRewind))
+	resultBuf, ok = mem.Read(buf, usedAfterRewind)
+	require.True(t, ok)
+	require.Equal(t, append(append(dirent1, dirent2...), dirent3...), resultBuf)
+}
+
 func Test_fdReaddir_Errors(t *testing.T) {
 	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fstest.FS))
 	defer r.Close(testCtx)
@@ -2069,8 +2123,8 @@ func Test_fdSeek(t *testing.T) {
 				'?',
 			},
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=4,offset=4,whence=0,result.newoffset=1)
-<== errno=ESUCCESS
+==> wasi_snapshot_preview1.fd_seek(fd=4,offset=4,whence=0,4557430888798830399)
+<== (newoffset=4,errno=ESUCCESS)
 `,
 		},
 		{
@@ -2084,8 +2138,8 @@ func Test_fdSeek(t *testing.T) {
 				'?',
 			},
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=4,offset=1,whence=1,result.newoffset=1)
-<== errno=ESUCCESS
+==> wasi_snapshot_preview1.fd_seek(fd=4,offset=1,whence=1,4557430888798830399)
+<== (newoffset=2,errno=ESUCCESS)
 `,
 		},
 		{
@@ -2099,8 +2153,8 @@ func Test_fdSeek(t *testing.T) {
 				'?',
 			},
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=4,offset=-1,whence=2,result.newoffset=1)
-<== errno=ESUCCESS
+==> wasi_snapshot_preview1.fd_seek(fd=4,offset=-1,whence=2,4557430888798830399)
+<== (newoffset=5,errno=ESUCCESS)
 `,
 		},
 	}
@@ -2138,9 +2192,13 @@ func Test_fdSeek(t *testing.T) {
 }
 
 func Test_fdSeek_Errors(t *testing.T) {
-	mod, fd, log, r := requireOpenFile(t, t.TempDir(), "test_path", []byte("wazero"), true)
+	mod, fd, log, r := requireOpenFile(t, t.TempDir(), "test_path", []byte("wazero"), false)
 	defer r.Close(testCtx)
 
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+	require.NoError(t, fsc.RootFS().Mkdir("dir", 0o0700))
+	dirFD := requireOpenFD(t, mod, "dir")
+
 	memorySize := mod.Memory().Size()
 
 	tests := []struct {
@@ -2156,8 +2214,8 @@ func Test_fdSeek_Errors(t *testing.T) {
 			fd:            42, // arbitrary invalid fd
 			expectedErrno: ErrnoBadf,
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=42,offset=0,whence=0,result.newoffset=0)
-<== errno=EBADF
+==> wasi_snapshot_preview1.fd_seek(fd=42,offset=0,whence=0,0)
+<== (newoffset=,errno=EBADF)
 `,
 		},
 		{
@@ -2166,8 +2224,17 @@ func Test_fdSeek_Errors(t *testing.T) {
 			whence:        3, // invalid whence, the largest whence io.SeekEnd(2) + 1
 			expectedErrno: ErrnoInval,
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=4,offset=0,whence=3,result.newoffset=0)
-<== errno=EINVAL
+==> wasi_snapshot_preview1.fd_seek(fd=4,offset=0,whence=3,0)
+<== (newoffset=,errno=EINVAL)
+`,
+		},
+		{
+			name:          "dir not file",
+			fd:            dirFD,
+			expectedErrno: ErrnoBadf,
+			expectedLog: `
+==> wasi_snapshot_preview1.fd_seek(fd=5,offset=0,whence=0,0)
+<== (newoffset=,errno=EBADF)
 `,
 		},
 		{
@@ -2176,8 +2243,8 @@ func Test_fdSeek_Errors(t *testing.T) {
 			resultNewoffset: memorySize,
 			expectedErrno:   ErrnoFault,
 			expectedLog: `
-==> wasi_snapshot_preview1.fd_seek(fd=4,offset=0,whence=0,result.newoffset=65536)
-<== errno=EFAULT
+==> wasi_snapshot_preview1.fd_seek(fd=4,offset=0,whence=0,)
+<== (newoffset=,errno=EFAULT)
 `,
 		},
 	}

imports/wasi_snapshot_preview1/fs_unit_test.go

@@ -158,6 +158,15 @@ func Test_lastDirEntries(t *testing.T) {
 			cookie:        1,
 			expectedErrno: ErrnoNosys, // not implemented
 		},
+		{
+			name: "read from the beginning (cookie=0)",
+			f: &sys.ReadDir{
+				CountRead: 3,
+				Entries:   testDirEntries,
+			},
+			cookie:          0,
+			expectedEntries: testDirEntries,
+		},
 	}
 
 	for _, tt := range tests {

imports/wasi_snapshot_preview1/testdata/cargo-wasi/wasi.rs

@@ -14,7 +14,10 @@ fn main() {
     match args[1].as_str() {
         "ls" => {
             main_ls(&args[2]);
-        },
+            if args.len() == 4 && args[3].as_str() == "repeat" {
+                main_ls(&args[2]);
+            }
+        }
         "stat" => {
             main_stat();
         }

imports/wasi_snapshot_preview1/testdata/zig-cc/wasi.c

@@ -3,17 +3,24 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <stdbool.h>
 
 #define formatBool(b) ((b) ? "true" : "false")
 
-void main_ls(char *dir_name) {
+void main_ls(char *dir_name, bool repeat) {
   DIR *d;
   struct dirent *dir;
   d = opendir(dir_name);
   if (d) {
     while ((dir = readdir(d)) != NULL) {
       printf("./%s\n", dir->d_name);
     }
+    if (repeat) {
+      rewinddir(d);
+      while ((dir = readdir(d)) != NULL) {
+        printf("./%s\n", dir->d_name);
+      }
+    }
     closedir(d);
   } else if (errno == ENOTDIR) {
     printf("ENOTDIR\n");
@@ -31,7 +38,7 @@ void main_stat() {
 
 int main(int argc, char** argv) {
   if (strcmp(argv[1],"ls")==0) {
-    main_ls(argv[2]);
+    main_ls(argv[2], strcmp(argv[3],"repeat")==0);
   } else if (strcmp(argv[1],"stat")==0) {
     main_stat();
   } else {

imports/wasi_snapshot_preview1/testdata/zig/wasi.zig

@@ -22,9 +22,10 @@ pub fn main() !void {
                 return;
             },
         };
-        var iter = dir.iterate();
-        while (try iter.next()) |entry| {
-            try stdout.print("./{s}\n", .{entry.name});
+
+        try ls(dir);
+        if (args.len > 3 and std.mem.eql(u8, args[3], "repeat")) {
+            try ls(dir);
         }
     } else if (std.mem.eql(u8, args[1], "stat")) {
         try stdout.print("stdin isatty: {}\n", .{os.isatty(0)});
@@ -40,3 +41,10 @@ pub fn main() !void {
         }
     }
 }
+
+fn ls(dir: std.fs.IterableDir) !void {
+    var iter = dir.iterate();
+    while (try iter.next()) |entry| {
+        try stdout.print("./{s}\n", .{entry.name});
+    }
+}

imports/wasi_snapshot_preview1/wasi_stdlib_test.go

@@ -80,6 +80,18 @@ ENOTDIR
 `, "\n"+console)
 	})
 
+	t.Run("directory with entries - read twice", func(t *testing.T) {
+		console := compileAndRun(t, moduleConfig.WithArgs("wasi", "ls", ".", "repeat"), bin)
+		require.Equal(t, `
+./-
+./a-
+./ab-
+./-
+./a-
+./ab-
+`, "\n"+console)
+	})
+
 	t.Run("directory with tons of entries", func(t *testing.T) {
 		testFS := fstest.MapFS{}
 		count := 8096

internal/sys/fs.go

@@ -170,6 +170,10 @@ type FileEntry struct {
 	// ReadDir is present when this File is a fs.ReadDirFile and `ReadDir`
 	// was called.
 	ReadDir *ReadDir
+
+	openPath string
+	openFlag int
+	openPerm fs.FileMode
 }
 
 // IsDir returns true if the file is a directory.
@@ -298,7 +302,7 @@ func (c *FSContext) OpenFile(fs sysfs.FS, path string, flag int, perm fs.FileMod
 	if f, err := fs.OpenFile(path, flag, perm); err != nil {
 		return 0, err
 	} else {
-		fe := &FileEntry{FS: fs, File: f}
+		fe := &FileEntry{openPath: path, FS: fs, File: f, openFlag: flag, openPerm: perm}
 		if path == "/" || path == "." {
 			fe.Name = ""
 		} else {
@@ -309,6 +313,32 @@ func (c *FSContext) OpenFile(fs sysfs.FS, path string, flag int, perm fs.FileMod
 	}
 }
 
+// ReOpenDir re-opens the directory while keeping the same file descriptor.
+// TODO: this might not be necessary once we have our own File type.
+func (c *FSContext) ReOpenDir(fd uint32) (*FileEntry, error) {
+	f, ok := c.openedFiles.Lookup(fd)
+	if !ok {
+		return nil, syscall.EBADF
+	} else if !f.IsDir() {
+		return nil, syscall.EISDIR
+	}
+
+	if err := f.File.Close(); err != nil {
+		return nil, err
+	}
+
+	// Re-opens with  the same parameters as before.
+	opened, err := f.FS.OpenFile(f.openPath, f.openFlag, f.openPerm)
+	if err != nil {
+		return nil, err
+	}
+
+	// Reset the state.
+	f.File = opened
+	f.ReadDir.CountRead, f.ReadDir.Entries = 0, nil
+	return f, nil
+}
+
 // LookupFile returns a file if it is in the table.
 func (c *FSContext) LookupFile(fd uint32) (*FileEntry, bool) {
 	f, ok := c.openedFiles.Lookup(fd)