diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index 7c46e8295a..79a311807e 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -611,8 +611,8 @@ func fdReadFn(ctx context.Context, mod api.Module, stack []uint64) (nread uint32 } func fdReadOrPread(ctx context.Context, mod api.Module, stack []uint64, isPread bool) (uint32, Errno) { - sysCtx := mod.(*wasm.CallContext).Sys mem := mod.Memory() + sysCtx := mod.(*wasm.CallContext).Sys fd := uint32(stack[0]) iovs := uint32(stack[1]) @@ -698,6 +698,9 @@ var fdReaddir = proxyResultParams(&wasm.HostFunc{ }, fdReaddirName) func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, Errno) { + mem := mod.Memory() + fsc := mod.(*wasm.CallContext).Sys.FS(ctx) + fd := uint32(stack[0]) buf := uint32(stack[1]) bufLen := uint32(stack[2]) @@ -706,15 +709,21 @@ func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, E // it in such a way that becomes negative. cookie := int64(stack[3]) + // The bufLen must be enough to write a dirent. Otherwise, the caller can't + // read what the next cookie is. + if bufLen < direntSize { + return 0, ErrnoInval + } + // Validate the FD is a directory - rd, dir, errno := openedDir(ctx, mod, fd) + rd, dir, errno := openedDir(ctx, fsc, fd) if errno != ErrnoSuccess { return 0, errno } // expect a cookie only if we are continuing a read. if cookie == 0 && dir.CountRead > 0 { - return 0, ErrnoInval // invalid as a cookie is minimally one. + return 0, ErrnoInval // cookie is minimally one. } // First, determine the maximum directory entries that can be encoded as @@ -750,8 +759,6 @@ func fdReaddirFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, E } } - mem := mod.Memory() - // Determine how many dirents we can write, excluding a potentially // truncated entry. bufused, direntCount, writeTruncatedEntry := maxDirents(entries, bufLen) @@ -930,8 +937,7 @@ func writeDirent(buf []byte, dNext uint64, dNamlen uint32, dType bool) { } // openedDir returns the directory and ErrnoSuccess if the fd points to a readable directory. -func openedDir(ctx context.Context, mod api.Module, fd uint32) (fs.ReadDirFile, *internalsys.ReadDir, Errno) { - fsc := mod.(*wasm.CallContext).Sys.FS(ctx) +func openedDir(ctx context.Context, fsc *internalsys.FSContext, fd uint32) (fs.ReadDirFile, *internalsys.ReadDir, Errno) { if f, ok := fsc.OpenedFile(ctx, fd); !ok { return nil, nil, ErrnoBadf } else if d, ok := f.File.(fs.ReadDirFile); !ok { @@ -1103,8 +1109,8 @@ var fdWrite = proxyResultParams(&wasm.HostFunc{ }, fdWriteName) func fdWriteFn(ctx context.Context, mod api.Module, stack []uint64) (uint32, Errno) { - sysCtx := mod.(*wasm.CallContext).Sys mem := mod.Memory() + sysCtx := mod.(*wasm.CallContext).Sys fd := uint32(stack[0]) iovs := uint32(stack[1]) diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go index 74afa788bb..b8281923e4 100644 --- a/imports/wasi_snapshot_preview1/fs_test.go +++ b/imports/wasi_snapshot_preview1/fs_test.go @@ -1248,7 +1248,7 @@ func Test_fdReaddir(t *testing.T) { tests := []struct { name string dir func() *internalsys.FileEntry - buf, bufLen uint32 + bufLen uint32 cookie int64 expectedMem []byte expectedMemSize int @@ -1263,7 +1263,7 @@ func Test_fdReaddir(t *testing.T) { return &internalsys.FileEntry{File: dir} }, - buf: 0, bufLen: 1, + bufLen: direntSize, cookie: 0, expectedBufused: 0, expectedMem: []byte{}, @@ -1277,7 +1277,7 @@ func Test_fdReaddir(t *testing.T) { return &internalsys.FileEntry{File: dir} }, - buf: 0, bufLen: 4096, + bufLen: 4096, cookie: 0, expectedBufused: 78, // length of all entries expectedMem: append(append(dirent1, dirent2...), dirent3...), @@ -1286,23 +1286,6 @@ func Test_fdReaddir(t *testing.T) { Entries: testDirEntries, }, }, - { - name: "can't read", - dir: func() *internalsys.FileEntry { - dir, err := fdReadDirFs.Open("dir") - require.NoError(t, err) - - return &internalsys.FileEntry{File: dir} - }, - buf: 0, bufLen: 23, // length is too short for header - cookie: 0, - expectedBufused: 23, // == bufLen which is the size of the dirent - expectedMem: nil, - expectedReadDir: &internalsys.ReadDir{ - CountRead: 2, - Entries: testDirEntries[:2], - }, - }, { name: "can't read name", dir: func() *internalsys.FileEntry { @@ -1311,7 +1294,7 @@ func Test_fdReaddir(t *testing.T) { return &internalsys.FileEntry{File: dir} }, - buf: 0, bufLen: 24, // length is long enough for first, but not the name. + bufLen: 24, // length is long enough for first, but not the name. cookie: 0, expectedBufused: 24, // == bufLen which is the size of the dirent expectedMem: dirent1[:24], // header without name @@ -1328,7 +1311,7 @@ func Test_fdReaddir(t *testing.T) { return &internalsys.FileEntry{File: dir} }, - buf: 0, bufLen: 25, // length is long enough for first + the name, but not more. + bufLen: 25, // length is long enough for first + the name, but not more. cookie: 0, expectedBufused: 25, // length to read exactly first. expectedMem: dirent1, @@ -1353,7 +1336,7 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 26, // length is long enough for exactly second. + bufLen: 26, // length is long enough for exactly second. cookie: 1, // d_next of first expectedBufused: 26, // length to read exactly second. expectedMem: dirent2, @@ -1378,7 +1361,7 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 30, // length is longer than the second entry, but not long enough for a header. + bufLen: 30, // length is longer than the second entry, but not long enough for a header. cookie: 1, // d_next of first expectedBufused: 30, // length to read some more, but not enough for a header, so buf was exhausted. expectedMem: dirent2, @@ -1404,7 +1387,7 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 50, // length is longer than the second entry + enough for the header of third. + bufLen: 50, // length is longer than the second entry + enough for the header of third. cookie: 1, // d_next of first expectedBufused: 50, // length to read exactly second and the header of third. expectedMem: append(dirent2, dirent3[0:24]...), @@ -1429,7 +1412,7 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 53, // length is long enough for second and third. + bufLen: 53, // length is long enough for second and third. cookie: 1, // d_next of first expectedBufused: 53, // length to read exactly one second and third. expectedMem: append(dirent2, dirent3...), @@ -1454,7 +1437,7 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 27, // length is long enough for exactly third. + bufLen: 27, // length is long enough for exactly third. cookie: 2, // d_next of second. expectedBufused: 27, // length to read exactly third. expectedMem: dirent3, @@ -1479,9 +1462,9 @@ func Test_fdReaddir(t *testing.T) { }, } }, - buf: 0, bufLen: 100, // length is long enough for third and more, but there is nothing more. - cookie: 2, // d_next of second. - expectedBufused: 27, // length to read exactly third. + bufLen: 100, // length is long enough for third and more, but there is nothing more. + cookie: 2, // d_next of second. + expectedBufused: 27, // length to read exactly third. expectedMem: dirent3, expectedReadDir: &internalsys.ReadDir{ CountRead: 3, @@ -1506,17 +1489,17 @@ func Test_fdReaddir(t *testing.T) { maskMemory(t, testCtx, mod, int(tc.bufLen)) - // use an arbitrarily high value for the buf used position. - resultBufused := uint32(16192) + resultBufused := uint32(0) // where to write the amount used out of bufLen + buf := uint32(8) // where to start the dirents requireErrno(t, ErrnoSuccess, mod, fdReaddirName, - uint64(fd), uint64(tc.buf), uint64(tc.bufLen), uint64(tc.cookie), uint64(resultBufused)) + uint64(fd), uint64(buf), uint64(tc.bufLen), uint64(tc.cookie), uint64(resultBufused)) // read back the bufused and compare memory against it bufUsed, ok := mod.Memory().ReadUint32Le(testCtx, resultBufused) require.True(t, ok) require.Equal(t, tc.expectedBufused, bufUsed) - mem, ok := mod.Memory().Read(testCtx, tc.buf, bufUsed) + mem, ok := mod.Memory().Read(testCtx, buf, bufUsed) require.True(t, ok) if tc.expectedMem != nil { @@ -1569,26 +1552,30 @@ func Test_fdReaddir_Errors(t *testing.T) { `, }, { - name: "invalid fd", - fd: 42, // arbitrary invalid fd + name: "invalid fd", + fd: 42, // arbitrary invalid fd + buf: 0, bufLen: direntSize, // enough to read the dirent + resultBufused: 1000, // arbitrary expectedErrno: ErrnoBadf, expectedLog: ` ---> proxy.fd_readdir(fd=42,buf=0,buf_len=0,cookie=0,result.bufused=0) - --> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=0,cookie=0,result.bufused=0) - ==> wasi_snapshot_preview1.fdReaddir(fd=42,buf=0,buf_len=0,cookie=0) +--> proxy.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0,result.bufused=1000) + --> wasi_snapshot_preview1.fd_readdir(fd=42,buf=0,buf_len=24,cookie=0,result.bufused=1000) + ==> wasi_snapshot_preview1.fdReaddir(fd=42,buf=0,buf_len=24,cookie=0) <== (bufused=0,EBADF) <-- EBADF <-- 8 `, }, { - name: "not a dir", - fd: fileFD, + name: "not a dir", + fd: fileFD, + buf: 0, bufLen: direntSize, // enough to read the dirent + resultBufused: 1000, // arbitrary expectedErrno: ErrnoNotdir, expectedLog: ` ---> proxy.fd_readdir(fd=5,buf=0,buf_len=0,cookie=0,result.bufused=0) - --> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=0,cookie=0,result.bufused=0) - ==> wasi_snapshot_preview1.fdReaddir(fd=5,buf=0,buf_len=0,cookie=0) +--> proxy.fd_readdir(fd=5,buf=0,buf_len=24,cookie=0,result.bufused=1000) + --> wasi_snapshot_preview1.fd_readdir(fd=5,buf=0,buf_len=24,cookie=0,result.bufused=1000) + ==> wasi_snapshot_preview1.fdReaddir(fd=5,buf=0,buf_len=24,cookie=0) <== (bufused=0,ENOTDIR) <-- ENOTDIR <-- 54 @@ -1625,16 +1612,34 @@ func Test_fdReaddir_Errors(t *testing.T) { `, }, { - name: "resultBufused is outside memory", + name: "bufLen must be enough to write a struct", fd: dirFD, buf: 0, bufLen: 1, - resultBufused: memLen, - expectedErrno: ErrnoFault, + resultBufused: 1000, + expectedErrno: ErrnoInval, expectedLog: ` ---> proxy.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=65536) - --> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=65536) - <-- EFAULT -<-- 21 +--> proxy.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=1000) + --> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1,cookie=0,result.bufused=1000) + ==> wasi_snapshot_preview1.fdReaddir(fd=4,buf=0,buf_len=1,cookie=0) + <== (bufused=0,EINVAL) + <-- EINVAL +<-- 28 +`, + }, + { + name: "cookie invalid when no prior state", + fd: dirFD, + buf: 0, bufLen: 1000, + cookie: 1, + resultBufused: 2000, + expectedErrno: ErrnoInval, + expectedLog: ` +--> proxy.fd_readdir(fd=4,buf=0,buf_len=1000,cookie=1,result.bufused=2000) + --> wasi_snapshot_preview1.fd_readdir(fd=4,buf=0,buf_len=1000,cookie=1,result.bufused=2000) + ==> wasi_snapshot_preview1.fdReaddir(fd=4,buf=0,buf_len=1000,cookie=1) + <== (bufused=0,EINVAL) + <-- EINVAL +<-- 28 `, }, { diff --git a/imports/wasi_snapshot_preview1/wasi_bench_test.go b/imports/wasi_snapshot_preview1/wasi_bench_test.go index da640e785b..687eee73af 100644 --- a/imports/wasi_snapshot_preview1/wasi_bench_test.go +++ b/imports/wasi_snapshot_preview1/wasi_bench_test.go @@ -1,12 +1,16 @@ package wasi_snapshot_preview1 import ( + "embed" + "io/fs" + "os" "testing" "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/internal/sys" "github.com/tetratelabs/wazero/internal/testing/proxy" + "github.com/tetratelabs/wazero/internal/wasm" ) // configArgsEnviron ensures the result data are the same between args and ENV. @@ -40,10 +44,7 @@ func Benchmark_ArgsEnviron(b *testing.B) { if err != nil { b.Fatal(err) } - errno := Errno(results[0]) - if errno != 0 { - b.Fatal(ErrnoName(errno)) - } + requireEsuccess(b, results) } }) } @@ -110,15 +111,126 @@ func Benchmark_fdRead(b *testing.B) { if err != nil { b.Fatal(err) } - errno := Errno(results[0]) - if errno != 0 { - b.Fatal(ErrnoName(errno)) + requireEsuccess(b, results) + } + }) + } +} + +//go:embed testdata +var testdata embed.FS + +func Benchmark_fdReaddir(b *testing.B) { + embedFS, err := fs.Sub(testdata, "testdata") + if err != nil { + b.Fatal(err) + } + + benches := []struct { + name string + fs fs.FS + // continued is true to test performance on a follow-up call. The + // preceding will call fd_read with 24 bytes, which is enough to read + // the initial entry's size, but not enough to read its name. This + // ensures the next fd_read is allowed to pass a cookie, because it + // read fd_next, while ensuring it will write all the entries. + continued bool + }{ + { + name: "embed.FS", + fs: embedFS, + }, + { + name: "embed.FS - continued", + fs: embedFS, + continued: true, + }, + { + name: "os.DirFS", + fs: os.DirFS("testdata"), + }, + { + name: "os.DirFS - continued", + fs: os.DirFS("testdata"), + continued: true, + }, + } + + for _, bb := range benches { + bc := bb + + b.Run(bc.name, func(b *testing.B) { + r := wazero.NewRuntime(testCtx) + defer r.Close(testCtx) + + mod, err := instantiateProxyModule(r, wazero.NewModuleConfig().WithFS(bc.fs)) + if err != nil { + b.Fatal(err) + } + + fn := mod.ExportedFunction(fdReaddirName) + + // Open the root directory as a file-descriptor. + fsc := mod.(*wasm.CallContext).Sys.FS(testCtx) + fd, err := fsc.OpenFile(testCtx, ".") + if err != nil { + b.Fatal(err) + } + f, ok := fsc.OpenedFile(testCtx, fd) + if !ok { + b.Fatal("couldn't open fd ", fd) + } + defer fsc.CloseFile(testCtx, fd) + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + b.StopTimer() + + cookie := 0 // where to begin (last read d_next) + resultBufused := 0 // where to write the amount used out of bufLen + buf := 8 // where to start the dirents + bufLen := 8096 // allow up to 8KB buffer usage + + // Recreate the file under the file-descriptor + if err = f.File.Close(); err != nil { + b.Fatal(err) } + if f.File, err = bc.fs.Open("."); err != nil { + b.Fatal(err) + } + f.ReadDir = nil + + // Make an initial call to build the state of an unread directory + if bc.continued { + results, err := fn.Call(testCtx, uint64(fd), uint64(buf), uint64(24), uint64(cookie), uint64(resultBufused)) + if err != nil { + b.Fatal(err) + } + requireEsuccess(b, results) + cookie = 1 // WASI doesn't document this, but we write the first d_next as 1 + } + + // Time the call to write the dirents + b.StartTimer() + results, err := fn.Call(testCtx, uint64(fd), uint64(buf), uint64(bufLen), uint64(cookie), uint64(resultBufused)) + if err != nil { + b.Fatal(err) + } + b.StopTimer() + + requireEsuccess(b, results) } }) } } +func requireEsuccess(b *testing.B, results []uint64) { + if errno := Errno(results[0]); errno != 0 { + b.Fatal(ErrnoName(errno)) + } +} + type writerFunc func(p []byte) (n int, err error) // Write implements io.Writer by calling writerFunc. @@ -179,10 +291,7 @@ func Benchmark_fdWrite(b *testing.B) { if err != nil { b.Fatal(err) } - errno := Errno(results[0]) - if errno != 0 { - b.Fatal(ErrnoName(errno)) - } + requireEsuccess(b, results) } }) }