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 <adrian@tetrate.io>
Co-authored-by: Takeshi Yoneda <takeshi@tetrate.io>

.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,10 +3,11 @@
 #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);
@@ -14,6 +15,12 @@ void main_ls(char *dir_name) {
     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)

internal/sys/fs_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"syscall"
 	"testing"
 	"testing/fstest"
 
@@ -181,3 +182,49 @@ func TestContext_Close_Error(t *testing.T) {
 	// Paths should clear even under error
 	require.Zero(t, fsc.openedFiles.Len(), "expected no opened files")
 }
+
+func TestFSContext_ReOpenDir(t *testing.T) {
+	tmpDir := t.TempDir()
+	dirFs := sysfs.NewDirFS(tmpDir)
+
+	const dirName = "dir"
+	err := dirFs.Mkdir(dirName, 0o700)
+	require.NoError(t, err)
+
+	fsc, err := NewFSContext(nil, nil, nil, dirFs)
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, fsc.Close(context.Background()))
+	}()
+
+	t.Run("ok", func(t *testing.T) {
+		dirFd, err := fsc.OpenFile(dirFs, dirName, os.O_RDONLY, 0o600)
+		require.NoError(t, err)
+
+		ent, ok := fsc.LookupFile(dirFd)
+		require.True(t, ok)
+
+		// Set arbitrary state.
+		ent.ReadDir = &ReadDir{Entries: make([]fs.DirEntry, 10), CountRead: 12345}
+
+		// Then reopen the same file descriptor.
+		ent, err = fsc.ReOpenDir(dirFd)
+		require.NoError(t, err)
+
+		// Verify the read dir state has been reset.
+		require.Equal(t, &ReadDir{}, ent.ReadDir)
+	})
+
+	t.Run("non existing ", func(t *testing.T) {
+		_, err = fsc.ReOpenDir(12345)
+		require.ErrorIs(t, err, syscall.EBADF)
+	})
+
+	t.Run("not dir", func(t *testing.T) {
+		const fileName = "dog"
+		fd, err := fsc.OpenFile(dirFs, fileName, os.O_CREATE, 0o600)
+		require.NoError(t, err)
+		_, err = fsc.ReOpenDir(fd)
+		require.ErrorIs(t, err, syscall.EISDIR)
+	})
+}

internal/sysfs/sysfs.go

@@ -110,6 +110,16 @@ type FS interface {
 	//   - syscall.EISDIR: `path` exists, but is a directory.
 	Unlink(path string) error
 
+	// Truncate is similar to syscall.Truncate, except the path is relative to
+	// this file system.
+	//
+	// # Errors
+	//
+	// The following errors are expected:
+	//   - syscall.EINVAL: `path` is invalid or size is negative.
+	//   - syscall.ENOENT: `path` doesn't exist
+	Truncate(name string, size int64) error
+
 	// Utimes is similar to syscall.UtimesNano, except the path is relative to
 	// this file system.
 	//
@@ -125,16 +135,6 @@ type FS interface {
 	//   - syscall.UtimesNano cannot change the ctime. Also, neither WASI nor
 	//     runtime.GOOS=js support changing it. Hence, ctime it is absent here.
 	Utimes(path string, atimeNsec, mtimeNsec int64) error
-
-	// Truncate is similar to syscall.Truncate, except the path is relative to
-	// this file system.
-	//
-	// # Errors
-	//
-	// The following errors are expected:
-	//   - syscall.EINVAL: `path` is invalid or size is negative.
-	//   - syscall.ENOENT: `path` doesn't exist
-	Truncate(name string, size int64) error
 }
 
 // StatPath is a convenience that calls FS.OpenFile until there is a stat

internal/wasi_snapshot_preview1/errno.go

@@ -283,6 +283,8 @@ func ToErrno(err error) Errno {
 		return ErrnoNosys
 	case errors.Is(err, syscall.ENOTDIR):
 		return ErrnoNotdir
+	case errors.Is(err, syscall.EPERM), errors.Is(err, fs.ErrPermission):
+		return ErrnoPerm
 	default:
 		return ErrnoIo
 	}

internal/wasi_snapshot_preview1/logging/logging.go

@@ -150,6 +150,10 @@ func Config(fnd api.FunctionDefinition) (pSampler logging.ParamSampler, pLoggers
 			logger = logMemI32(idx).Log
 			rLoggers = append(rLoggers, resultParamLogger(name, logger))
 			continue
+		case "result.newoffset":
+			name = resultParamName(name)
+			logger = logMemI64(idx).Log
+			rLoggers = append(rLoggers, resultParamLogger(name, logger))
 		case "result.filestat":
 			name = resultParamName(name)
 			logger = logFilestat(idx).Log