From b5198a46ff541e4be6ae45c88b766bd7e061888e Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Fri, 5 May 2023 13:23:30 +0800 Subject: [PATCH] Adds AccessMode Write and Pwrite to platform.File (#1438) Signed-off-by: Adrian Cole --- imports/assemblyscript/assemblyscript.go | 33 +++-- imports/wasi_snapshot_preview1/fs.go | 49 ++++--- imports/wasi_snapshot_preview1/fs_test.go | 38 +++--- imports/wasi_snapshot_preview1/poll.go | 8 +- internal/gojs/fs.go | 33 ++--- internal/gojs/runtime.go | 10 +- internal/platform/file.go | 96 +++++++++++++- internal/platform/file_test.go | 19 ++- internal/platform/open_file_test.go | 9 +- internal/sys/fs.go | 30 +++-- internal/sys/fs_test.go | 18 +-- internal/sysfs/adapter.go | 4 +- internal/sysfs/dirfs.go | 2 +- internal/sysfs/readfs.go | 2 +- internal/sysfs/rootfs.go | 4 +- internal/sysfs/sysfs.go | 34 ----- internal/sysfs/sysfs_test.go | 151 +--------------------- 17 files changed, 238 insertions(+), 302 deletions(-) diff --git a/imports/assemblyscript/assemblyscript.go b/imports/assemblyscript/assemblyscript.go index 033d8f9e84..96ad86550d 100644 --- a/imports/assemblyscript/assemblyscript.go +++ b/imports/assemblyscript/assemblyscript.go @@ -31,11 +31,13 @@ import ( "io" "strconv" "strings" + "syscall" "unicode/utf16" "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" . "github.com/tetratelabs/wazero/internal/assemblyscript" + "github.com/tetratelabs/wazero/internal/platform" internalsys "github.com/tetratelabs/wazero/internal/sys" "github.com/tetratelabs/wazero/internal/wasm" "github.com/tetratelabs/wazero/sys" @@ -163,10 +165,13 @@ func abortWithMessage(ctx context.Context, mod api.Module, stack []uint64) { columnNumber := uint32(stack[3]) // Don't panic if there was a problem reading the message - stderr := internalsys.WriterForFile(fsc, internalsys.FdStderr) - if msg, msgOk := readAssemblyScriptString(mem, message); msgOk && stderr != nil { - if fn, fnOk := readAssemblyScriptString(mem, fileName); fnOk { - _, _ = fmt.Fprintf(stderr, "%s at %s:%d:%d\n", msg, fn, lineNumber, columnNumber) + stderr := stdioFile(fsc, internalsys.FdStderr) + if stderr != nil { + if msg, msgOk := readAssemblyScriptString(mem, message); msgOk && stderr != nil { + if fn, fnOk := readAssemblyScriptString(mem, fileName); fnOk { + s := fmt.Sprintf("%s at %s:%d:%d\n", msg, fn, lineNumber, columnNumber) + _, _ = stderr.Write([]byte(s)) + } } } abort(ctx, mod, stack) @@ -197,7 +202,7 @@ var traceStdout = &wasm.HostFunc{ Code: wasm.Code{ GoFunc: api.GoModuleFunc(func(_ context.Context, mod api.Module, stack []uint64) { fsc := mod.(*wasm.ModuleInstance).Sys.FS() - traceTo(mod, stack, internalsys.WriterForFile(fsc, internalsys.FdStdout)) + traceTo(mod, stack, stdioFile(fsc, internalsys.FdStdout)) }), }, } @@ -205,7 +210,7 @@ var traceStdout = &wasm.HostFunc{ // traceStderr implements trace to the configured Stderr. var traceStderr = traceStdout.WithGoModuleFunc(func(_ context.Context, mod api.Module, stack []uint64) { fsc := mod.(*wasm.ModuleInstance).Sys.FS() - traceTo(mod, stack, internalsys.WriterForFile(fsc, internalsys.FdStderr)) + traceTo(mod, stack, stdioFile(fsc, internalsys.FdStderr)) }) // traceTo implements the function "trace" in AssemblyScript. e.g. @@ -218,8 +223,8 @@ var traceStderr = traceStdout.WithGoModuleFunc(func(_ context.Context, mod api.M // (import "env" "trace" (func $~lib/builtins/trace (param i32 i32 f64 f64 f64 f64 f64))) // // See https://github.com/AssemblyScript/assemblyscript/blob/fa14b3b03bd4607efa52aaff3132bea0c03a7989/std/assembly/wasi/index.ts#L61 -func traceTo(mod api.Module, params []uint64, writer io.Writer) { - if writer == nil { +func traceTo(mod api.Module, params []uint64, file platform.File) { + if file == nil { return // closed } message := uint32(params[0]) @@ -258,7 +263,7 @@ func traceTo(mod api.Module, params []uint64, writer io.Writer) { ret.WriteString(formatFloat(arg4)) } ret.WriteByte('\n') - _, _ = writer.Write([]byte(ret.String())) // don't crash if trace logging fails + _, _ = file.Write([]byte(ret.String())) // don't crash if trace logging fails } func formatFloat(f float64) string { @@ -318,3 +323,13 @@ func decodeUTF16(b []byte) string { return string(utf16.Decode(u16s)) } + +func stdioFile(fsc *internalsys.FSContext, fd int32) platform.File { + if f, ok := fsc.LookupFile(fd); !ok { + return nil + } else if f.File.AccessMode() == syscall.O_RDONLY { + return nil + } else { + return f.File + } +} diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index 203da61ee3..26616996d1 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -203,8 +203,7 @@ func fdFdstatGetFn(_ context.Context, mod api.Module, params []uint64) syscall.E return syscall.EBADF } else if st, errno = f.Stat(); errno != 0 { return errno - } else if _, ok := f.File.File().(io.Writer); ok { - // TODO: maybe cache flags to open instead + } else if f.File.AccessMode() != syscall.O_RDONLY { fdflags = wasip1.FD_APPEND } @@ -1287,6 +1286,23 @@ func fdWriteFn(_ context.Context, mod api.Module, params []uint64) syscall.Errno return fdWriteOrPwrite(mod, params, false) } +// pwriter tracks an offset across multiple writes. +type pwriter struct { + f platform.File + offset int64 +} + +// Write implements the same function as documented on platform.File. +func (w *pwriter) Write(p []byte) (n int, errno syscall.Errno) { + if len(p) == 0 { + return 0, 0 // less overhead on zero-length writes. + } + + n, err := w.f.Pwrite(p, w.offset) + w.offset += int64(n) + return n, err +} + func fdWriteOrPwrite(mod api.Module, params []uint64, isPwrite bool) syscall.Errno { mem := mod.Memory() fsc := mod.(*wasm.ModuleInstance).Sys.FS() @@ -1296,20 +1312,20 @@ func fdWriteOrPwrite(mod api.Module, params []uint64, isPwrite bool) syscall.Err iovsCount := uint32(params[2]) var resultNwritten uint32 - var writer io.Writer + var writer func(p []byte) (n int, errno syscall.Errno) if f, ok := fsc.LookupFile(fd); !ok { return syscall.EBADF + } else if f.File.AccessMode() == syscall.O_RDONLY { + return syscall.EBADF } else if isPwrite { offset := int64(params[3]) - writer = sysfs.WriterAtOffset(f.File.File(), offset) + writer = (&pwriter{f: f.File, offset: offset}).Write resultNwritten = uint32(params[4]) - } else if writer, ok = f.File.File().(io.Writer); !ok { - return syscall.EBADF } else { + writer = f.File.Write resultNwritten = uint32(params[3]) } - var err error var nwritten uint32 iovsStop := iovsCount << 3 // iovsCount * 8 iovsBuf, ok := mem.Read(iovs, iovsStop) @@ -1321,20 +1337,15 @@ func fdWriteOrPwrite(mod api.Module, params []uint64, isPwrite bool) syscall.Err offset := le.Uint32(iovsBuf[iovsPos:]) l := le.Uint32(iovsBuf[iovsPos+4:]) - var n int - if writer == io.Discard { // special-case default - n = int(l) - } else { - b, ok := mem.Read(offset, l) - if !ok { - return syscall.EFAULT - } - n, err = writer.Write(b) - if err != nil { - return platform.UnwrapOSError(err) - } + b, ok := mem.Read(offset, l) + if !ok { + return syscall.EFAULT } + n, errno := writer(b) nwritten += uint32(n) + if errno != 0 { + return errno + } } if !mod.Memory().WriteUint32Le(resultNwritten, nwritten) { diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go index bb825745f3..6f22aa68a8 100644 --- a/imports/wasi_snapshot_preview1/fs_test.go +++ b/imports/wasi_snapshot_preview1/fs_test.go @@ -2884,7 +2884,7 @@ func Test_fdWrite_discard(t *testing.T) { func Test_fdWrite_Errors(t *testing.T) { tmpDir := t.TempDir() // open before loop to ensure no locking problems. pathName := "test_path" - mod, fd, log, r := requireOpenFile(t, tmpDir, pathName, nil, false) + mod, fd, log, r := requireOpenFile(t, tmpDir, pathName, []byte{1, 2, 3, 4}, false) defer r.Close(testCtx) // Setup valid test memory @@ -3789,10 +3789,7 @@ func Test_pathOpen(t *testing.T) { path: func(t *testing.T) (file string) { return appendName }, fdflags: wasip1.FD_APPEND, expected: func(t *testing.T, fsc *sys.FSContext) { - contents := []byte("hello") - _, err := sys.WriterForFile(fsc, expectedOpenedFd).Write(contents) - require.NoError(t, err) - require.Zero(t, fsc.CloseFile(expectedOpenedFd)) + contents := writeAndCloseFile(t, fsc, expectedOpenedFd) // verify the contents were appended b := readFile(t, dir, appendName) @@ -3821,10 +3818,7 @@ func Test_pathOpen(t *testing.T) { oflags: wasip1.O_CREAT, expected: func(t *testing.T, fsc *sys.FSContext) { // expect to create a new file - contents := []byte("hello") - _, err := sys.WriterForFile(fsc, expectedOpenedFd).Write(contents) - require.NoError(t, err) - require.Zero(t, fsc.CloseFile(expectedOpenedFd)) + contents := writeAndCloseFile(t, fsc, expectedOpenedFd) // verify the contents were written b := readFile(t, dir, "creat") @@ -3853,10 +3847,7 @@ func Test_pathOpen(t *testing.T) { oflags: wasip1.O_CREAT | wasip1.O_TRUNC, expected: func(t *testing.T, fsc *sys.FSContext) { // expect to create a new file - contents := []byte("hello") - _, err := sys.WriterForFile(fsc, expectedOpenedFd).Write(contents) - require.NoError(t, err) - require.Zero(t, fsc.CloseFile(expectedOpenedFd)) + contents := writeAndCloseFile(t, fsc, expectedOpenedFd) // verify the contents were written b := readFile(t, dir, joinPath(dirName, "O_CREAT-O_TRUNC")) @@ -3918,10 +3909,7 @@ func Test_pathOpen(t *testing.T) { path: func(t *testing.T) (file string) { return "trunc" }, oflags: wasip1.O_TRUNC, expected: func(t *testing.T, fsc *sys.FSContext) { - contents := []byte("hello") - _, err := sys.WriterForFile(fsc, expectedOpenedFd).Write(contents) - require.NoError(t, err) - require.Zero(t, fsc.CloseFile(expectedOpenedFd)) + contents := writeAndCloseFile(t, fsc, expectedOpenedFd) // verify the contents were truncated b := readFile(t, dir, "trunc") @@ -3985,6 +3973,16 @@ func Test_pathOpen(t *testing.T) { } } +func writeAndCloseFile(t *testing.T, fsc *sys.FSContext, fd int32) []byte { + contents := []byte("hello") + f, ok := fsc.LookupFile(fd) + require.True(t, ok) + _, errno := f.File.Write([]byte("hello")) + require.EqualErrno(t, 0, errno) + require.EqualErrno(t, 0, fsc.CloseFile(fd)) + return contents +} + func requireOpenFD(t *testing.T, mod api.Module, path string) int32 { fsc := mod.(*wasm.ModuleInstance).Sys.FS() preopen := fsc.RootFS() @@ -4911,6 +4909,9 @@ func Test_pathUnlinkFile_Errors(t *testing.T) { func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte, readOnly bool) (api.Module, int32, *bytes.Buffer, api.Closer) { oflags := os.O_RDWR + if readOnly { + oflags = os.O_RDONLY + } realPath := joinPath(tmpDir, pathName) if data == nil { @@ -4923,7 +4924,6 @@ func requireOpenFile(t *testing.T, tmpDir string, pathName string, data []byte, fsConfig := wazero.NewFSConfig() if readOnly { - oflags = os.O_RDONLY fsConfig = fsConfig.WithReadOnlyDirMount(tmpDir, "/") } else { fsConfig = fsConfig.WithDirMount(tmpDir, "/") @@ -5076,5 +5076,5 @@ func joinPath(dirName, baseName string) string { func openFsFile(t *testing.T, path string, flag int, perm fs.FileMode) platform.File { f, errno := platform.OpenFile(path, flag, perm) require.EqualErrno(t, 0, errno) - return platform.NewFsFile(path, f) + return platform.NewFsFile(path, flag, f) } diff --git a/imports/wasi_snapshot_preview1/poll.go b/imports/wasi_snapshot_preview1/poll.go index b1557fa6fc..1a92990724 100644 --- a/imports/wasi_snapshot_preview1/poll.go +++ b/imports/wasi_snapshot_preview1/poll.go @@ -232,10 +232,12 @@ func processFDEventRead(fsc *internalsys.FSContext, fd int32) wasip1.Errno { // processFDEventWrite returns ErrnoNotsup if the file exists and ErrnoBadf otherwise. func processFDEventWrite(fsc *internalsys.FSContext, fd int32) wasip1.Errno { - if internalsys.WriterForFile(fsc, fd) == nil { - return wasip1.ErrnoBadf + if f, ok := fsc.LookupFile(fd); ok { + if f.File.AccessMode() != syscall.O_RDONLY { + return wasip1.ErrnoNotsup + } } - return wasip1.ErrnoNotsup + return wasip1.ErrnoBadf } // writeEvent writes the event corresponding to the processed subscription. diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go index ee3ee4bbb8..42111b739a 100644 --- a/internal/gojs/fs.go +++ b/internal/gojs/fs.go @@ -281,35 +281,28 @@ func (jsfsWrite) invoke(ctx context.Context, mod api.Module, args ...interface{} callback := args[5].(funcWrapper) if byteCount > 0 { // empty is possible on EOF - n, err := syscallWrite(mod, fd, fOffset, buf.Unwrap()[offset:offset+byteCount]) - return callback.invoke(ctx, mod, goos.RefJsfs, err, n) // note: error first + n, errno := syscallWrite(mod, fd, fOffset, buf.Unwrap()[offset:offset+byteCount]) + var err error + if errno != 0 { + err = errno + } + // It is safe to cast to uint32 because n <= uint32(byteCount). + return callback.invoke(ctx, mod, goos.RefJsfs, err, uint32(n)) // note: error first } return callback.invoke(ctx, mod, goos.RefJsfs, nil, goos.RefValueZero) } // syscallWrite is like syscall.Write -func syscallWrite(mod api.Module, fd int32, offset interface{}, p []byte) (n uint32, err error) { +func syscallWrite(mod api.Module, fd int32, offset interface{}, p []byte) (n int, errno syscall.Errno) { fsc := mod.(*wasm.ModuleInstance).Sys.FS() - - var writer io.Writer if f, ok := fsc.LookupFile(fd); !ok { - err = syscall.EBADF + errno = syscall.EBADF + } else if f.File.AccessMode() == syscall.O_RDONLY { + errno = syscall.EBADF } else if offset != nil { - writer = sysfs.WriterAtOffset(f.File.File(), toInt64(offset)) - } else if writer, ok = f.File.File().(io.Writer); !ok { - err = syscall.EBADF - } - - if err != nil { - return - } - - if nWritten, e := writer.Write(p); e == nil || e == io.EOF { - // fs_js.go cannot parse io.EOF so coerce it to nil. - // See https://github.com/golang/go/issues/43913 - n = uint32(nWritten) + n, errno = f.File.Pwrite(p, toInt64(offset)) } else { - err = e + n, errno = f.File.Write(p) } return } diff --git a/internal/gojs/runtime.go b/internal/gojs/runtime.go index ad9ecf6dc4..195687f847 100644 --- a/internal/gojs/runtime.go +++ b/internal/gojs/runtime.go @@ -3,12 +3,12 @@ package gojs import ( "context" "fmt" + "syscall" "time" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/internal/gojs/custom" "github.com/tetratelabs/wazero/internal/gojs/goarch" - internalsys "github.com/tetratelabs/wazero/internal/sys" "github.com/tetratelabs/wazero/internal/wasm" ) @@ -42,10 +42,12 @@ func wasmWrite(_ context.Context, mod api.Module, stack goarch.Stack) { p := stack.ParamBytes(mod.Memory(), 1 /*, 2 */) fsc := mod.(*wasm.ModuleInstance).Sys.FS() - if writer := internalsys.WriterForFile(fsc, fd); writer == nil { + if f, ok := fsc.LookupFile(fd); ok && f.File.AccessMode() != syscall.O_RDONLY { + if _, err := f.File.Write(p); err != 0 { + panic(fmt.Errorf("error writing p: %w", err)) + } + } else { panic(fmt.Errorf("fd %d invalid", fd)) - } else if _, err := writer.Write(p); err != nil { - panic(fmt.Errorf("error writing p: %w", err)) } } diff --git a/internal/platform/file.go b/internal/platform/file.go index 45711c16b7..1a5aadf3d8 100644 --- a/internal/platform/file.go +++ b/internal/platform/file.go @@ -33,6 +33,14 @@ type File interface { // Note: This can drift on rename. Path() string + // AccessMode returns the access mode the file was opened with. + // + // This returns exclusively one of the following: + // - syscall.O_RDONLY: read-only, e.g. os.Stdin + // - syscall.O_WRONLY: write-only, e.g. os.Stdout + // - syscall.O_RDWR: read-write, e.g. os.CreateTemp + AccessMode() int + // Stat is similar to syscall.Fstat. // // # Errors @@ -131,6 +139,37 @@ type File interface { // - Windows does not error when calling Truncate on a closed file. Truncate(size int64) syscall.Errno + // Write attempts to write all bytes in `p` to the file, and returns the + // count written even on error. + // + // # Errors + // + // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS: the implementation does not support this function. + // - syscall.EBADF: the file or directory was closed or not writeable. + // + // # Notes + // + // - This is like io.Writer and `write` in POSIX, preferring semantics of + // io.Writer. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html + Write(p []byte) (n int, errno syscall.Errno) + + // Pwrite attempts to write all bytes in `p` to the file at the given + // offset `off`, and returns the count written even on error. + // + // # Errors + // + // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS: the implementation does not support this function. + // - syscall.EBADF: the file or directory was closed or not writeable. + // - syscall.EINVAL: the offset was negative. + // + // # Notes + // + // - This is like io.WriterAt and `pwrite` in POSIX, preferring semantics + // of io.WriterAt. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/pwrite.html + Pwrite(p []byte, off int64) (n int, errno syscall.Errno) + // Close closes the underlying file. // // A zero syscall.Errno is success. The below are expected otherwise: @@ -180,13 +219,28 @@ func (UnimplementedFile) Truncate(int64) syscall.Errno { return syscall.ENOSYS } -func NewFsFile(path string, f fs.File) File { - return &fsFile{path, f} +// Write implements File.Write +func (UnimplementedFile) Write([]byte) (int, syscall.Errno) { + return 0, syscall.ENOSYS +} + +// Pwrite implements File.Pwrite +func (UnimplementedFile) Pwrite([]byte, int64) (int, syscall.Errno) { + return 0, syscall.ENOSYS +} + +func NewFsFile(openPath string, openFlag int, f fs.File) File { + return &fsFile{ + path: openPath, + accessMode: openFlag & (syscall.O_RDONLY | syscall.O_WRONLY | syscall.O_RDWR), + file: f, + } } type fsFile struct { - path string - file fs.File + path string + accessMode int + file fs.File } // Path implements File.Path @@ -194,6 +248,11 @@ func (f *fsFile) Path() string { return f.path } +// AccessMode implements File.AccessMode +func (f *fsFile) AccessMode() int { + return f.accessMode +} + // Stat implements File.Stat func (f *fsFile) Stat() (Stat_t, syscall.Errno) { st, errno := statFile(f.file) @@ -247,6 +306,35 @@ func (f *fsFile) Truncate(size int64) syscall.Errno { return syscall.ENOSYS } +// Write implements File.Write +func (f *fsFile) Write(p []byte) (n int, errno syscall.Errno) { + if len(p) == 0 { + return 0, 0 // less overhead on zero-length writes. + } + + if f.accessMode == syscall.O_RDONLY { + return 0, syscall.EBADF + } + if w, ok := f.File().(io.Writer); ok { + n, err := w.Write(p) + return n, UnwrapOSError(err) + } + return 0, syscall.EBADF +} + +// Pwrite implements File.Pwrite +func (f *fsFile) Pwrite(p []byte, off int64) (n int, errno syscall.Errno) { + if len(p) == 0 { + return 0, 0 // less overhead on zero-length writes. + } + + if w, ok := f.File().(io.WriterAt); ok { + n, err := w.WriteAt(p, off) + return n, UnwrapOSError(err) + } + return 0, syscall.EBADF +} + // Close implements File.Close func (f *fsFile) Close() syscall.Errno { return UnwrapOSError(f.file.Close()) diff --git a/internal/platform/file_test.go b/internal/platform/file_test.go index 075401a90e..0f02b983d2 100644 --- a/internal/platform/file_test.go +++ b/internal/platform/file_test.go @@ -24,7 +24,12 @@ type NoopFile struct { // The current design requires the user to implement Path. func (NoopFile) Path() string { - return "noop" + return "" +} + +// The current design requires the user to implement AccessMode. +func (NoopFile) AccessMode() int { + return syscall.O_RDONLY } // The current design requires the user to consciously implement Close. @@ -66,11 +71,11 @@ func testSync_NoError(t *testing.T, sync func(File) syscall.Errno) { }, { name: "File of read-only fs.File", - f: NewFsFile(roPath, ro), + f: NewFsFile(roPath, syscall.O_RDONLY, ro), }, { name: "File of os.File", - f: NewFsFile(rwPath, rw), + f: NewFsFile(rwPath, syscall.O_RDWR, rw), }, } @@ -101,7 +106,7 @@ func testSync(t *testing.T, sync func(File) syscall.Errno) { defer d.Close() // Even though it is invalid, try to sync a directory - errno := sync(NewFsFile(dPath, d)) + errno := sync(NewFsFile(dPath, syscall.O_RDONLY, d)) require.EqualErrno(t, 0, errno) fPath := path.Join(dPath, t.Name()) @@ -112,8 +117,8 @@ func testSync(t *testing.T, sync func(File) syscall.Errno) { expected := "hello world!" // Write the expected data - _, err = f.File().(io.Writer).Write([]byte(expected)) - require.NoError(t, err) + _, errno = f.Write([]byte(expected)) + require.EqualErrno(t, 0, errno) // Sync the data. errno = sync(f) @@ -238,5 +243,5 @@ func openForWrite(t *testing.T, path string, content []byte) File { func openFsFile(t *testing.T, path string, flag int, perm fs.FileMode) File { f, errno := OpenFile(path, flag, perm) require.EqualErrno(t, 0, errno) - return NewFsFile(path, f) + return NewFsFile(path, flag, f) } diff --git a/internal/platform/open_file_test.go b/internal/platform/open_file_test.go index 37bef1aec8..b8f52ae710 100644 --- a/internal/platform/open_file_test.go +++ b/internal/platform/open_file_test.go @@ -1,7 +1,6 @@ package platform import ( - "io" "os" path "path/filepath" "runtime" @@ -73,8 +72,8 @@ func TestOpenFile_Errors(t *testing.T) { f := openFsFile(t, path, os.O_RDONLY, 0) defer f.Close() - _, err := f.File().(io.Writer).Write([]byte{1, 2, 3, 4}) - require.EqualErrno(t, syscall.EBADF, UnwrapOSError(err)) + _, errno := f.Write([]byte{1, 2, 3, 4}) + require.EqualErrno(t, syscall.EBADF, errno) }) t.Run("writing to a directory is EBADF", func(t *testing.T) { @@ -84,8 +83,8 @@ func TestOpenFile_Errors(t *testing.T) { f := openFsFile(t, path, os.O_RDONLY, 0) defer f.Close() - _, err := f.File().(io.Writer).Write([]byte{1, 2, 3, 4}) - require.EqualErrno(t, syscall.EBADF, UnwrapOSError(err)) + _, errno := f.Write([]byte{1, 2, 3, 4}) + require.EqualErrno(t, syscall.EBADF, errno) }) // This is similar to https://github.com/WebAssembly/wasi-testsuite/blob/dc7f8d27be1030cd4788ebdf07d9b57e5d23441e/tests/rust/src/bin/dangling_symlink.rs diff --git a/internal/sys/fs.go b/internal/sys/fs.go index 8e29e0344e..4d6aff16e3 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -165,6 +165,11 @@ func (r *lazyDir) Path() string { return "." } +// AccessMode implements the same method as documented on platform.File +func (r *lazyDir) AccessMode() int { + return syscall.O_RDONLY +} + // Stat implements the same method as documented on platform.File func (r *lazyDir) Stat() (platform.Stat_t, syscall.Errno) { if f, ok := r.file(); !ok { @@ -215,6 +220,16 @@ func (r *lazyDir) Truncate(int64) syscall.Errno { return syscall.EISDIR } +// Write implements the same method as documented on platform.File +func (r *lazyDir) Write([]byte) (int, syscall.Errno) { + return 0, syscall.EBADF +} + +// Pwrite implements the same method as documented on platform.File +func (r *lazyDir) Pwrite([]byte, int64) (int, syscall.Errno) { + return 0, syscall.EBADF +} + // File implements the same method as documented on platform.File func (r *lazyDir) File() fs.File { if f, ok := r.file(); !ok { @@ -417,7 +432,7 @@ func stdinReader(r io.Reader) (*FileEntry, error) { freader = NewStdioFileReader(r, s, PollerDefaultStdin) } return &FileEntry{ - Name: noopStdinStat.Name(), File: platform.NewFsFile("", freader), + Name: noopStdinStat.Name(), File: platform.NewFsFile("", syscall.O_RDONLY, freader), }, nil } @@ -430,7 +445,7 @@ func stdioWriter(w io.Writer, defaultStat stdioFileInfo) (*FileEntry, error) { return nil, err } return &FileEntry{ - Name: s.Name(), File: platform.NewFsFile("", &stdioFileWriter{w: w, s: s}), + Name: s.Name(), File: platform.NewFsFile("", syscall.O_WRONLY, &stdioFileWriter{w: w, s: s}), }, nil } @@ -596,14 +611,3 @@ func (c *FSContext) Close() (err error) { c.openedFiles = FileTable{} return } - -// WriterForFile returns a writer for the given file descriptor or nil if not -// opened or not writeable (e.g. a directory or a file not opened for writes). -func WriterForFile(fsc *FSContext, fd int32) (writer io.Writer) { - if f, ok := fsc.LookupFile(fd); !ok { - return - } else if w, ok := f.File.File().(io.Writer); ok { - writer = w - } - return -} diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go index 147cbb1eca..fd94401370 100644 --- a/internal/sys/fs_test.go +++ b/internal/sys/fs_test.go @@ -19,9 +19,9 @@ import ( ) var ( - noopStdin = &FileEntry{Name: "stdin", File: platform.NewFsFile("", NewStdioFileReader(eofReader{}, noopStdinStat, PollerDefaultStdin))} - noopStdout = &FileEntry{Name: "stdout", File: platform.NewFsFile("", &stdioFileWriter{w: io.Discard, s: noopStdoutStat})} - noopStderr = &FileEntry{Name: "stderr", File: platform.NewFsFile("", &stdioFileWriter{w: io.Discard, s: noopStderrStat})} + noopStdin = &FileEntry{Name: "stdin", File: platform.NewFsFile("", syscall.O_RDONLY, NewStdioFileReader(eofReader{}, noopStdinStat, PollerDefaultStdin))} + noopStdout = &FileEntry{Name: "stdout", File: platform.NewFsFile("", syscall.O_WRONLY, &stdioFileWriter{w: io.Discard, s: noopStdoutStat})} + noopStderr = &FileEntry{Name: "stderr", File: platform.NewFsFile("", syscall.O_WRONLY, &stdioFileWriter{w: io.Discard, s: noopStderrStat})} ) //go:embed testdata @@ -367,18 +367,6 @@ func TestFSContext_ChangeOpenFlag(t *testing.T) { require.Equal(t, f2.openFlag&syscall.O_APPEND, 0) } -func TestWriterForFile(t *testing.T) { - c := Context{} - err := c.NewFSContext(nil, nil, nil, sysfs.UnimplementedFS{}) - require.NoError(t, err) - testFS := &c.fsc - - require.Nil(t, WriterForFile(testFS, FdStdin)) - require.Equal(t, noopStdout.File.File(), WriterForFile(testFS, FdStdout)) - require.Equal(t, noopStderr.File.File(), WriterForFile(testFS, FdStderr)) - require.Nil(t, WriterForFile(testFS, FdPreopen)) -} - func TestStdioStat(t *testing.T) { stat, err := stdioStat(os.Stdin, noopStdinStat) require.NoError(t, err) diff --git a/internal/sysfs/adapter.go b/internal/sysfs/adapter.go index 762daa6689..102a748faf 100644 --- a/internal/sysfs/adapter.go +++ b/internal/sysfs/adapter.go @@ -48,7 +48,7 @@ func (a *adapter) Open(name string) (fs.File, error) { func (a *adapter) OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) { path = cleanPath(path) f, err := a.fs.Open(path) - return platform.NewFsFile(path, f), platform.UnwrapOSError(err) + return platform.NewFsFile(path, flag, f), platform.UnwrapOSError(err) } // Stat implements FS.Stat @@ -59,7 +59,7 @@ func (a *adapter) Stat(path string) (platform.Stat_t, syscall.Errno) { return platform.Stat_t{}, platform.UnwrapOSError(err) } defer f.Close() - return platform.NewFsFile(path, f).Stat() + return platform.NewFsFile(path, syscall.O_RDONLY, f).Stat() } // Lstat implements FS.Lstat diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go index bdad24bd3f..65c5f3427f 100644 --- a/internal/sysfs/dirfs.go +++ b/internal/sysfs/dirfs.go @@ -46,7 +46,7 @@ func (d *dirFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.File if errno != 0 { return nil, errno } - return platform.NewFsFile(path, f), 0 + return platform.NewFsFile(path, flag, f), 0 } // Lstat implements FS.Lstat diff --git a/internal/sysfs/readfs.go b/internal/sysfs/readfs.go index 47ed66fdc5..e3af0a10a9 100644 --- a/internal/sysfs/readfs.go +++ b/internal/sysfs/readfs.go @@ -67,7 +67,7 @@ func (r *readFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.Fil if errno != 0 { return nil, errno } - return platform.NewFsFile(path, maskForReads(f.File())), 0 + return platform.NewFsFile(path, flag, maskForReads(f.File())), 0 } // maskForReads masks the file with read-only interfaces used by wazero. diff --git a/internal/sysfs/rootfs.go b/internal/sysfs/rootfs.go index bd97392344..a3fcc2d593 100644 --- a/internal/sysfs/rootfs.go +++ b/internal/sysfs/rootfs.go @@ -137,7 +137,7 @@ func (c *CompositeFS) OpenFile(path string, flag int, perm fs.FileMode) (f platf case ".", "/", "": if len(c.rootGuestPaths) > 0 { dir := &openRootDir{c: c, f: f.File().(fs.ReadDirFile)} - f = platform.NewFsFile(path, dir) + f = platform.NewFsFile(path, syscall.O_RDONLY, dir) } } } @@ -483,7 +483,7 @@ type fakeRootFS struct{ UnimplementedFS } func (*fakeRootFS) OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) { switch path { case ".", "/", "": - return platform.NewFsFile(path, fakeRootDir{}), 0 + return platform.NewFsFile(path, flag, fakeRootDir{}), 0 } return nil, syscall.ENOENT } diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go index 87294a4585..95d73a5f76 100644 --- a/internal/sysfs/sysfs.go +++ b/internal/sysfs/sysfs.go @@ -450,37 +450,3 @@ func (rs *seekToOffsetReader) Read(p []byte) (int, error) { rs.offset += int64(n) return n, err } - -// WriterAtOffset gets an io.Writer from a fs.File that writes to an offset, -// yet doesn't affect the underlying position. This is used to implement -// syscall.Pwrite. -func WriterAtOffset(f fs.File, offset int64) io.Writer { - if ret, ok := f.(io.WriterAt); ok { - return &writerAtOffset{ret, offset} - } else { - return enosysWriter{} - } -} - -type enosysWriter struct{} - -// enosysWriter implements io.Writer -func (rs enosysWriter) Write([]byte) (n int, err error) { - return 0, syscall.ENOSYS -} - -type writerAtOffset struct { - r io.WriterAt - offset int64 -} - -// Write implements io.Writer -func (r *writerAtOffset) Write(p []byte) (int, error) { - if len(p) == 0 { - return 0, nil // less overhead on zero-length writes. - } - - n, err := r.r.WriteAt(p, r.offset) - r.offset += int64(n) - return n, err -} diff --git a/internal/sysfs/sysfs_test.go b/internal/sysfs/sysfs_test.go index aa33114492..6755c1f794 100644 --- a/internal/sysfs/sysfs_test.go +++ b/internal/sysfs/sysfs_test.go @@ -27,13 +27,10 @@ func testOpen_O_RDWR(t *testing.T, tmpDir string, testFS FS) { require.EqualErrno(t, 0, errno) defer f.Close() - w, ok := f.File().(io.Writer) - require.True(t, ok) - // If the write flag was honored, we should be able to write! fileContents := []byte{1, 2, 3, 4} - n, err := w.Write(fileContents) - require.NoError(t, err) + n, errno := f.Write(fileContents) + require.EqualErrno(t, 0, errno) require.Equal(t, len(fileContents), n) // Verify the contents actually wrote. @@ -49,12 +46,9 @@ func testOpen_O_RDWR(t *testing.T, tmpDir string, testFS FS) { require.EqualErrno(t, 0, errno) defer f.Close() - w, ok = f.File().(io.Writer) - require.True(t, ok) - if runtime.GOOS != "windows" { // If the read-only flag was honored, we should not be able to write! - _, err = w.Write(fileContents) + _, err = f.Write(fileContents) require.EqualErrno(t, syscall.EBADF, platform.UnwrapOSError(err)) } @@ -239,12 +233,8 @@ human defer require.Zero(t, f.Close()) require.EqualErrno(t, 0, errno) - if w, ok := f.File().(io.Writer); ok { - _, err := w.Write([]byte{1, 2, 3, 4}) - require.EqualErrno(t, syscall.EBADF, platform.UnwrapOSError(err)) - } else { - t.Skip("not an io.Writer") - } + _, err := f.Write([]byte{1, 2, 3, 4}) + require.EqualErrno(t, syscall.EBADF, platform.UnwrapOSError(err)) }) t.Run("writing to a directory is EBADF", func(t *testing.T) { @@ -252,12 +242,8 @@ human defer require.Zero(t, f.Close()) require.EqualErrno(t, 0, errno) - if w, ok := f.File().(io.Writer); ok { - _, err := w.Write([]byte{1, 2, 3, 4}) - require.EqualErrno(t, syscall.EBADF, platform.UnwrapOSError(err)) - } else { - t.Skip("not an io.Writer") - } + _, err := f.Write([]byte{1, 2, 3, 4}) + require.EqualErrno(t, syscall.EBADF, platform.UnwrapOSError(err)) }) } @@ -562,129 +548,6 @@ func TestReaderAtOffset_Unsupported(t *testing.T) { require.Equal(t, syscall.ENOSYS, err) } -func TestWriterAtOffset(t *testing.T) { - tmpDir := t.TempDir() - dirFS := NewDirFS(tmpDir) - - // fs.FS doesn't support writes, and there is no other built-in - // implementation except os.File. - tests := []struct { - name string - fs FS - }{ - {name: "sysfs.dirFS", fs: dirFS}, - } - - for _, tc := range tests { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - f, errno := tc.fs.OpenFile(readerAtFile, os.O_RDWR|os.O_CREATE, 0o600) - require.EqualErrno(t, 0, errno) - defer f.Close() - - w := f.File().(io.Writer) - wa := WriterAtOffset(f.File(), 6) - - text := "wazero" - buf := make([]byte, 3) - copy(buf, text[:3]) - - requireWrite3 := func(r io.Writer, buf []byte) { - n, err := r.Write(buf) - require.NoError(t, err) - require.Equal(t, 3, n) - } - - // The file should work as a writer (base case) - requireWrite3(w, buf) - - // The writerAt impl should be able to start from zero also - requireWrite3(wa, buf) - - copy(buf, text[3:]) - - // If the offset didn't change, the next chars will write after the - // first - requireWrite3(w, buf) - - // If state was held between writer-at, we expect the same - requireWrite3(wa, buf) - - // We should also be able to make another writer-at - wa = WriterAtOffset(f.File(), 12) - requireWrite3(wa, buf) - - r := ReaderAtOffset(f.File(), 0) - b, err := io.ReadAll(r) - require.NoError(t, err) - - // We expect to have written the text two and a half times: - // 1. io.Write: offset 0 - // 2. io.WriterAt: offset 6 - // 3. second io.WriterAt: offset 12, writing "ero" - require.Equal(t, text+text+text[3:], string(b)) - }) - } -} - -func TestWriterAtOffset_empty(t *testing.T) { - tmpDir := t.TempDir() - dirFS := NewDirFS(tmpDir) - - // fs.FS doesn't support writes, and there is no other built-in - // implementation except os.File. - tests := []struct { - name string - fs FS - }{ - {name: "sysfs.dirFS", fs: dirFS}, - } - - for _, tc := range tests { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - f, errno := tc.fs.OpenFile(emptyFile, os.O_RDWR|os.O_CREATE, 0o600) - require.EqualErrno(t, 0, errno) - defer f.Close() - - r := f.File().(io.Writer) - ra := WriterAtOffset(f.File(), 0) - - var emptyBuf []byte - - requireWrite := func(r io.Writer) { - n, err := r.Write(emptyBuf) - require.NoError(t, err) - require.Equal(t, 0, n) // file is empty - } - - // The file should work as a writer (base case) - requireWrite(r) - - // The writerAt impl should be able to start from zero also - requireWrite(ra) - }) - } -} - -func TestWriterAtOffset_Unsupported(t *testing.T) { - tmpDir := t.TempDir() - dirFS := NewDirFS(tmpDir) - - f, errno := dirFS.OpenFile(readerAtFile, os.O_RDWR|os.O_CREATE, 0o600) - require.EqualErrno(t, 0, errno) - defer f.Close() - - // mask both io.WriterAt and io.Seeker - ra := WriterAtOffset(struct{ fs.File }{f.File()}, 0) - - buf := make([]byte, 3) - _, err := ra.Write(buf) - require.Equal(t, syscall.ENOSYS, err) -} - func requireIno(t *testing.T, dirents []*platform.Dirent, expectIno bool) { for _, e := range dirents { if expectIno {