Repository: tetratelabs/wazero (go)

Issue:
stdout always detected as a character device
After 78ac85cce1d `stdout` is always detected as a character device. Is this expected?
This breaks my usage of `wazero` in [RethinkRAW](https://rethinkraw.com).

**To Reproduce**

```sh
git clone --branch atty https://github.com/ncruces/wazero-bugs
cd wazero-bugs
go run main.go
```

This is the offending code in `dcraw`:
https://github.com/ncruces/dcraw/blob/8dcf1ff24ad0b5a09c8b0c1030d983c09d159284/dcraw.c#L10314-L10317

**Expected behavior**

IMO, `isatty` should not blindly return true in all cases. I'm capturing the output of a tool to a `byte.Buffer`. It's expected that it is binary. I can patch `dcraw`, but other tools (e.g. `gzip`) will similarly complain.

**Environment (please complete the relevant information):**
- Go version: go1.19.4
- wazero Version: 78ac85cce1d674c1c47c7ff2dacbfbc2e3a3e2b4
- Host architecture: amd64
- Runtime mode: compiler

Pull request description:
wasi: detect if stdio are char devices instead of assuming
This uses ioctl syscalls or appropriate alternative, to detect if stdin/out/err are character devices or not. This caches the result, to ensure performance is ok at runtime as executing stat can approach microsecond overhead.

Fixes #928

Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 60920da4..804b8945 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -144,6 +144,8 @@ var fdFdstatGet = newHostFunc(fdFdstatGetName, fdFdstatGetFn, []api.ValueType{i3
 // fdFdstatGetFn cannot currently use proxyResultParams because fdstat is larger
 // than api.ValueTypeI64 (i64 == 8 bytes, but fdstat is 24).
 func fdFdstatGetFn(_ context.Context, mod api.Module, params []uint64) Errno {
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+
 	fd, resultFdstat := uint32(params[0]), uint32(params[1])
 
 	// Ensure we can write the fdstat
@@ -152,40 +154,20 @@ func fdFdstatGetFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		return ErrnoFault
 	}
 
-	// Special-case the stdio character devices
-	if fd <= internalsys.FdStderr {
-		switch fd {
-		case internalsys.FdStdin:
-			copy(buf, charFdstat)
-		case internalsys.FdStdout, internalsys.FdStderr:
-			copy(buf, charOutFdstat)
-		}
-		return ErrnoSuccess
-	}
-
-	// Otherwise, look up the file corresponding to the file descriptor.
-	fsc := mod.(*wasm.CallContext).Sys.FS()
-	file, ok := fsc.OpenedFile(fd)
-	if !ok {
-		return ErrnoBadf
+	stat, err := fsc.StatFile(fd)
+	if err != nil {
+		return toErrno(err)
 	}
 
-	// see if the file is writable
-	f := file.File
-	var filetype wasiFiletype
+	filetype := getWasiFiletype(stat.Mode())
 	var fdflags wasiFdflags
-	if _, ok := f.(io.Writer); ok {
+
+	// Determine if it is writeable
+	if w := fsc.FdWriter(fd); w != nil {
 		// TODO: maybe cache flags to open instead
 		fdflags = wasiFdflagsAppend
 	}
 
-	if fdstat, err := f.Stat(); err != nil {
-		return ErrnoIo
-	} else {
-		// TODO: maybe cache file type instead
-		filetype = getWasiFiletype(fdstat.Mode())
-	}
-
 	writeFdstat(buf, filetype, fdflags)
 
 	return ErrnoSuccess
@@ -201,23 +183,16 @@ const (
 	wasiFdflagsSync
 )
 
-var charFdstat = []byte{
-	byte(wasiFiletypeCharacterDevice), 0, // filetype
+var blockFdstat = []byte{
+	byte(wasiFiletypeBlockDevice), 0, // filetype
 	0, 0, 0, 0, 0, 0, // fdflags
 	0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_base
 	0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_inheriting
 }
 
-var charOutFdstat = []byte{
-	byte(wasiFiletypeCharacterDevice), 0, // filetype
-	wasiFdflagsAppend, 0, 0, 0, 0, 0, // fdflags
-	0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_base
-	0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_inheriting
-}
-
 func writeFdstat(buf []byte, filetype wasiFiletype, fdflags wasiFdflags) {
 	// memory is re-used, so ensure the result is defaulted.
-	copy(buf, charFdstat)
+	copy(buf, blockFdstat)
 	buf[0] = uint8(filetype)
 	buf[2] = fdflags
 }
@@ -291,12 +266,6 @@ type wasiFiletype uint8
 const (
 	wasiFiletypeUnknown wasiFiletype = iota
 	wasiFiletypeBlockDevice
-	// wasiFiletypeCharacterDevice is set when the FD is a character device.
-	//
-	// Note: wazero currently returns this for stdio descriptors even if the
-	// actual file is not a TTY, to ensure python can work. This avoids
-	// dependencies needed to be more precise.
-	// See https://github.com/mattn/go-isatty
 	wasiFiletypeCharacterDevice
 	wasiFiletypeDirectory
 	wasiFiletypeRegularFile
@@ -312,29 +281,17 @@ func fdFilestatGetFn(_ context.Context, mod api.Module, params []uint64) Errno {
 }
 
 func fdFilestatGetFunc(mod api.Module, fd, resultBuf uint32) Errno {
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+
 	// Ensure we can write the filestat
 	buf, ok := mod.Memory().Read(resultBuf, 64)
 	if !ok {
 		return ErrnoFault
 	}
 
-	// Special-case the stdio character devices
-	switch fd {
-	case internalsys.FdStdin, internalsys.FdStdout, internalsys.FdStderr:
-		copy(buf, charFilestat)
-		return ErrnoSuccess
-	}
-
-	// Otherwise, look up the file corresponding to the file descriptor.
-	fsc := mod.(*wasm.CallContext).Sys.FS()
-	file, ok := fsc.OpenedFile(fd)
-	if !ok {
-		return ErrnoBadf
-	}
-
-	stat, err := file.File.Stat()
+	stat, err := fsc.StatFile(fd)
 	if err != nil {
-		return ErrnoIo
+		return toErrno(err)
 	}
 
 	writeFilestat(buf, stat)
@@ -358,10 +315,10 @@ func getWasiFiletype(fileMode fs.FileMode) wasiFiletype {
 	return wasiFileType
 }
 
-var charFilestat = []byte{
+var blockFilestat = []byte{
 	0, 0, 0, 0, 0, 0, 0, 0, // device
 	0, 0, 0, 0, 0, 0, 0, 0, // inode
-	byte(wasiFiletypeCharacterDevice), 0, 0, 0, 0, 0, 0, 0, // filetype
+	byte(wasiFiletypeBlockDevice), 0, 0, 0, 0, 0, 0, 0, // filetype
 	1, 0, 0, 0, 0, 0, 0, 0, // nlink
 	0, 0, 0, 0, 0, 0, 0, 0, // filesize
 	0, 0, 0, 0, 0, 0, 0, 0, // atim
@@ -375,7 +332,7 @@ func writeFilestat(buf []byte, stat fs.FileInfo) {
 	mtim := stat.ModTime().UnixNano()
 
 	// memory is re-used, so ensure the result is defaulted.
-	copy(buf, charFilestat[:32])
+	copy(buf, blockFilestat[:32])
 	buf[16] = uint8(filetype)
 	le.PutUint64(buf[32:], filesize)     // filesize
 	le.PutUint64(buf[40:], uint64(mtim)) // atim
@@ -1485,13 +1442,11 @@ func openFile(fsc *internalsys.FSContext, name string) (fd uint32, errno Errno)
 // statFile attempts to stat the file at the given path. Errors coerce to WASI
 // Errno.
 func statFile(fsc *internalsys.FSContext, name string) (stat fs.FileInfo, errno Errno) {
-	s, err := fsc.StatFile(name)
-	if err == nil {
-		stat = s
-		errno = ErrnoSuccess
-		return
+	var err error
+	stat, err = fsc.StatPath(name)
+	if err != nil {
+		errno = toErrno(err)
 	}
-	errno = toErrno(err)
 	return
 }
 
diff --git a/internal/platform/platform.go b/internal/platform/platform.go
index 03ef17f6..ebc19a9b 100644
--- a/internal/platform/platform.go
+++ b/internal/platform/platform.go
@@ -48,3 +48,8 @@ func MunmapCodeSegment(code []byte) error {
 	}
 	return munmapCodeSegment(code)
 }
+
+// IsTerminal returns true if the given file descriptor is a terminal.
+func IsTerminal(fd uintptr) bool {
+	return isTerminal(fd)
+}
diff --git a/internal/platform/terminal.go b/internal/platform/terminal.go
new file mode 100644
index 00000000..326a16b8
--- /dev/null
+++ b/internal/platform/terminal.go
@@ -0,0 +1,14 @@
+//go:build darwin || linux || freebsd
+
+package platform
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+func isTerminal(fd uintptr) bool {
+	var val syscall.Termios
+	_, _, err := syscall.Syscall6(syscall.SYS_IOCTL, fd, ioctlReadTermios, uintptr(unsafe.Pointer(&val)), 0, 0, 0)
+	return err == 0
+}
diff --git a/internal/platform/terminal_bsd.go b/internal/platform/terminal_bsd.go
new file mode 100644
index 00000000..658a0a40
--- /dev/null
+++ b/internal/platform/terminal_bsd.go
@@ -0,0 +1,7 @@
+//go:build darwin || freebsd
+
+package platform
+
+import "syscall"
+
+const ioctlReadTermios = syscall.TIOCGETA
diff --git a/internal/platform/terminal_linux.go b/internal/platform/terminal_linux.go
new file mode 100644
index 00000000..2f4bc3b9
--- /dev/null
+++ b/internal/platform/terminal_linux.go
@@ -0,0 +1,5 @@
+package platform
+
+import "syscall"
+
+const ioctlReadTermios = syscall.TCGETS
diff --git a/internal/platform/terminal_unsupported.go b/internal/platform/terminal_unsupported.go
new file mode 100644
index 00000000..8e05593f
--- /dev/null
+++ b/internal/platform/terminal_unsupported.go
@@ -0,0 +1,7 @@
+//go:build !(darwin || linux || freebsd || windows)
+
+package platform
+
+func isTerminal(fd uintptr) bool {
+	return false
+}
diff --git a/internal/platform/terminal_windows.go b/internal/platform/terminal_windows.go
new file mode 100644
index 00000000..6f84c5ff
--- /dev/null
+++ b/internal/platform/terminal_windows.go
@@ -0,0 +1,14 @@
+package platform
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+var procGetConsoleMode = kernel32.NewProc("GetConsoleMode")
+
+func isTerminal(fd uintptr) bool {
+	var st uint32
+	r, _, e := syscall.Syscall(procGetConsoleMode.Addr(), 2, uintptr(fd), uintptr(unsafe.Pointer(&st)), 0)
+	return r != 0 && e == 0
+}
diff --git a/internal/sys/fs.go b/internal/sys/fs.go
index ab4e98ba..ca56c4b5 100644
--- a/internal/sys/fs.go
+++ b/internal/sys/fs.go
@@ -6,10 +6,13 @@ import (
 	"io"
 	"io/fs"
 	"math"
+	"os"
 	"path"
 	"sync/atomic"
 	"syscall"
 	"time"
+
+	"github.com/tetratelabs/wazero/internal/platform"
 )
 
 const (
@@ -99,8 +102,9 @@ type FSContext struct {
 	// fs is the root ("/") mount.
 	fs fs.FS
 
-	stdin          io.Reader
-	stdout, stderr io.Writer
+	stdin                             io.Reader
+	stdout, stderr                    io.Writer
+	stdinStat, stdoutStat, stderrStat fs.FileInfo
 
 	// openedFiles is a map of file descriptor numbers (>=FdRoot) to open files
 	// (or directories) and defaults to empty.
@@ -140,6 +144,26 @@ func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, root fs.FS) (fsc *F
 		lastFD:      FdStderr,
 	}
 
+	// Special case cached stat for stdio, notably using features that work in
+	// windows. This ensures tty detection can occur for python.
+	if stdin, ok := stdin.(*os.File); ok && platform.IsTerminal(stdin.Fd()) {
+		fsc.stdinStat = fileModeStat(fs.ModeCharDevice)
+	} else {
+		fsc.stdinStat = fileModeStat(fs.ModeDevice)
+	}
+
+	if stdout, ok := stdout.(*os.File); ok && platform.IsTerminal(stdout.Fd()) {
+		fsc.stdoutStat = fileModeStat(fs.ModeCharDevice)
+	} else {
+		fsc.stdoutStat = fileModeStat(fs.ModeDevice)
+	}
+
+	if stderr, ok := stderr.(*os.File); ok && platform.IsTerminal(stderr.Fd()) {
+		fsc.stderrStat = fileModeStat(fs.ModeCharDevice)
+	} else {
+		fsc.stderrStat = fileModeStat(fs.ModeDevice)
+	}
+
 	if root == EmptyFS {
 		return fsc, nil
 	}
@@ -189,6 +213,35 @@ func (c *FSContext) OpenedFile(fd uint32) (*FileEntry, bool) {
 	return f, ok
 }
 
+func (c *FSContext) StatFile(fd uint32) (fs.FileInfo, error) {
+	switch fd {
+	case FdStdin:
+		return c.stdinStat, nil
+	case FdStdout:
+		return c.stdoutStat, nil
+	case FdStderr:
+		return c.stderrStat, nil
+	}
+	f, ok := c.openedFiles[fd]
+	if !ok {
+		return nil, syscall.EBADF
+	}
+	return f.File.Stat()
+}
+
+// fileModeStat is a fake fs.FileInfo which only returns its mode.
+// This is used for character devices.
+type fileModeStat fs.FileMode
+
+var _ fs.FileInfo = fileModeStat(0)
+
+func (s fileModeStat) Size() int64        { return 0 }
+func (s fileModeStat) Mode() fs.FileMode  { return fs.FileMode(s) }
+func (s fileModeStat) ModTime() time.Time { return time.Unix(0, 0) }
+func (s fileModeStat) Sys() interface{}   { return nil }
+func (s fileModeStat) Name() string       { return "" }
+func (s fileModeStat) IsDir() bool        { return false }
+
 // OpenFile is like syscall.Open and returns the file descriptor of the new file or an error.
 //
 // TODO: Consider dirflags and oflags. Also, allow non-read-only open based on config about the mount.
@@ -209,7 +262,7 @@ func (c *FSContext) OpenFile(name string /* TODO: flags int, perm int */) (uint3
 	return newFD, nil
 }
 
-func (c *FSContext) StatFile(name string) (fs.FileInfo, error) {
+func (c *FSContext) StatPath(name string) (fs.FileInfo, error) {
 	f, err := c.openFile(name)
 	if err != nil {
 		return nil, err


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index 63760998..7c15bb5d 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -127,7 +127,7 @@ func Test_fdFdstatGet(t *testing.T) {
 			name: "stdin",
 			fd:   internalsys.FdStdin,
 			expectedMemory: []byte{
-				2, 0, // fs_filetype
+				1, 0, // fs_filetype
 				0, 0, 0, 0, 0, 0, // fs_flags
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_base
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_inheriting
@@ -143,7 +143,7 @@ func Test_fdFdstatGet(t *testing.T) {
 			name: "stdout",
 			fd:   internalsys.FdStdout,
 			expectedMemory: []byte{
-				2, 0, // fs_filetype
+				1, 0, // fs_filetype
 				1, 0, 0, 0, 0, 0, // fs_flags
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_base
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_inheriting
@@ -159,7 +159,7 @@ func Test_fdFdstatGet(t *testing.T) {
 			name: "stderr",
 			fd:   internalsys.FdStderr,
 			expectedMemory: []byte{
-				2, 0, // fs_filetype
+				1, 0, // fs_filetype
 				1, 0, 0, 0, 0, 0, // fs_flags
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_base
 				0, 0, 0, 0, 0, 0, 0, 0, // fs_rights_inheriting
@@ -318,7 +318,8 @@ func Test_fdFilestatGet(t *testing.T) {
 			expectedMemory: []byte{
 				0, 0, 0, 0, 0, 0, 0, 0, // dev
 				0, 0, 0, 0, 0, 0, 0, 0, // ino
-				2, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
+				// expect block device because stdin isn't a real file
+				1, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
 				1, 0, 0, 0, 0, 0, 0, 0, // nlink
 				0, 0, 0, 0, 0, 0, 0, 0, // size
 				0, 0, 0, 0, 0, 0, 0, 0, // atim
@@ -338,7 +339,8 @@ func Test_fdFilestatGet(t *testing.T) {
 			expectedMemory: []byte{
 				0, 0, 0, 0, 0, 0, 0, 0, // dev
 				0, 0, 0, 0, 0, 0, 0, 0, // ino
-				2, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
+				// expect block device because stdout isn't a real file
+				1, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
 				1, 0, 0, 0, 0, 0, 0, 0, // nlink
 				0, 0, 0, 0, 0, 0, 0, 0, // size
 				0, 0, 0, 0, 0, 0, 0, 0, // atim
@@ -358,7 +360,8 @@ func Test_fdFilestatGet(t *testing.T) {
 			expectedMemory: []byte{
 				0, 0, 0, 0, 0, 0, 0, 0, // dev
 				0, 0, 0, 0, 0, 0, 0, 0, // ino
-				2, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
+				// expect block device because stderr isn't a real file
+				1, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
 				1, 0, 0, 0, 0, 0, 0, 0, // nlink
 				0, 0, 0, 0, 0, 0, 0, 0, // size
 				0, 0, 0, 0, 0, 0, 0, 0, // atim
diff --git a/imports/wasi_snapshot_preview1/wasi_stdlib_test.go b/imports/wasi_snapshot_preview1/wasi_stdlib_test.go
index 56bcf485..6cbcfc26 100644
--- a/imports/wasi_snapshot_preview1/wasi_stdlib_test.go
+++ b/imports/wasi_snapshot_preview1/wasi_stdlib_test.go
@@ -110,10 +110,11 @@ func testFdReaddirStat(t *testing.T, bin []byte) {
 
 	console := compileAndRun(t, moduleConfig.WithFS(fstest.MapFS{}), bin)
 
+	// TODO: switch this to a real stat test
 	require.Equal(t, `
-stdin isatty: true
-stdout isatty: true
-stderr isatty: true
+stdin isatty: false
+stdout isatty: false
+stderr isatty: false
 / isatty: false
 `, "\n"+console)
 }
diff --git a/internal/platform/bench_test.go b/internal/platform/bench_test.go
new file mode 100644
index 00000000..b4ebca58
--- /dev/null
+++ b/internal/platform/bench_test.go
@@ -0,0 +1,12 @@
+package platform
+
+import (
+	"os"
+	"testing"
+)
+
+func Benchmark_IsTerminal(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		IsTerminal(os.Stdout.Fd())
+	}
+}
diff --git a/internal/platform/terminal_test.go b/internal/platform/terminal_test.go
new file mode 100644
index 00000000..37926541
--- /dev/null
+++ b/internal/platform/terminal_test.go
@@ -0,0 +1,44 @@
+package platform
+
+import (
+	"os"
+	"path"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func Test_IsTerminal(t *testing.T) {
+	if !CompilerSupported() {
+		t.Skip() // because it will always return false
+	}
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(path.Join(dir, "foo"), nil, 0o400))
+	file, err := os.Open(path.Join(dir, "foo"))
+	require.NoError(t, err)
+	defer file.Close()
+
+	// We aren't guaranteed to have a terminal device for os.Stdout, due to how
+	// `go test` forks processes. Instead, we test if this is consistent. For
+	// example, when run in a debugger, this could end up true.
+	stdioIsTTY := IsTerminal(os.Stdout.Fd())
+
+	tests := []struct {
+		name     string
+		file     *os.File
+		expected bool
+	}{
+		{name: "Stdin", file: os.Stdin, expected: stdioIsTTY},
+		{name: "Stdout", file: os.Stdout, expected: stdioIsTTY},
+		{name: "Stderr", file: os.Stderr, expected: stdioIsTTY},
+		{name: "File", file: file, expected: false},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.expected, IsTerminal(tc.file.Fd()))
+		})
+	}
+}
diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go
index 5aed30ee..1a4ed85e 100644
--- a/internal/sys/fs_test.go
+++ b/internal/sys/fs_test.go
@@ -89,8 +89,11 @@ func TestEmptyFSContext(t *testing.T) {
 
 	expected := &FSContext{
 		stdin:       eofReader{},
+		stdinStat:   fileModeStat(fs.ModeDevice),
 		stdout:      io.Discard,
+		stdoutStat:  fileModeStat(fs.ModeDevice),
 		stderr:      io.Discard,
+		stderrStat:  fileModeStat(fs.ModeDevice),
 		fs:          EmptyFS,
 		openedFiles: map[uint32]*FileEntry{},
 		lastFD:      2,