Repository: tetratelabs/wazero
Pull request #1225 (2023-03-13 04:37:12)

Fixes file type mapping problem identified in /dev/null
This fixes a file type mapping bug running go's `TestDevNullFile` test via gojs.

See #1222


Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 292a5023..d9a47aba 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -362,20 +362,24 @@ func fdFilestatGetFunc(mod api.Module, fd, resultBuf uint32) Errno {
 	return ErrnoSuccess
 }
 
-func getWasiFiletype(fileMode fs.FileMode) uint8 {
-	wasiFileType := FILETYPE_UNKNOWN
-	if fileMode&fs.ModeDevice != 0 {
-		wasiFileType = FILETYPE_BLOCK_DEVICE
-	} else if fileMode&fs.ModeCharDevice != 0 {
-		wasiFileType = FILETYPE_CHARACTER_DEVICE
-	} else if fileMode&fs.ModeDir != 0 {
-		wasiFileType = FILETYPE_DIRECTORY
-	} else if fileMode&fs.ModeType == 0 {
-		wasiFileType = FILETYPE_REGULAR_FILE
-	} else if fileMode&fs.ModeSymlink != 0 {
-		wasiFileType = FILETYPE_SYMBOLIC_LINK
-	}
-	return wasiFileType
+func getWasiFiletype(fm fs.FileMode) uint8 {
+	switch {
+	case fm.IsRegular():
+		return FILETYPE_REGULAR_FILE
+	case fm.IsDir():
+		return FILETYPE_DIRECTORY
+	case fm&fs.ModeSymlink != 0:
+		return FILETYPE_SYMBOLIC_LINK
+	case fm&fs.ModeDevice != 0:
+		// Unlike ModeDevice and ModeCharDevice, FILETYPE_CHARACTER_DEVICE and
+		// FILETYPE_BLOCK_DEVICE are set mutually exclusively.
+		if fm&fs.ModeCharDevice != 0 {
+			return FILETYPE_CHARACTER_DEVICE
+		}
+		return FILETYPE_BLOCK_DEVICE
+	default: // unknown
+		return FILETYPE_UNKNOWN
+	}
 }
 
 func writeFilestat(buf []byte, st *platform.Stat_t) (err error) {
diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go
index e8fc592b..41bdbb83 100644
--- a/internal/gojs/fs.go
+++ b/internal/gojs/fs.go
@@ -204,50 +204,55 @@ func syscallFstat(fsc *internalsys.FSContext, fd uint32) (*jsSt, error) {
 	return newJsSt(&st), nil
 }
 
-func newJsSt(stat *platform.Stat_t) *jsSt {
+func newJsSt(st *platform.Stat_t) *jsSt {
 	ret := &jsSt{}
-	ret.isDir = stat.Mode.IsDir()
-	ret.dev = stat.Dev
-	ret.ino = stat.Ino
-	ret.mode = getJsMode(stat.Mode)
-	ret.nlink = uint32(stat.Nlink)
-	ret.size = stat.Size
-	ret.atimeMs = stat.Atim / 1e6
-	ret.mtimeMs = stat.Mtim / 1e6
-	ret.ctimeMs = stat.Ctim / 1e6
+	ret.isDir = st.Mode.IsDir()
+	ret.dev = st.Dev
+	ret.ino = st.Ino
+	ret.mode = getJsMode(st.Mode)
+	ret.nlink = uint32(st.Nlink)
+	ret.size = st.Size
+	ret.atimeMs = st.Atim / 1e6
+	ret.mtimeMs = st.Mtim / 1e6
+	ret.ctimeMs = st.Ctim / 1e6
 	return ret
 }
 
 // getJsMode is required because the mode property read in `GOOS=js` is
 // incompatible with normal go. Particularly the directory flag isn't the same.
-func getJsMode(mode fs.FileMode) (jsMode uint32) {
-	jsMode = uint32(mode & fs.ModePerm)
-	switch mode & fs.ModeType {
-	case 0:
-		jsMode |= S_IFREG
-	case fs.ModeDir:
-		jsMode |= S_IFDIR
-	case fs.ModeSymlink:
-		jsMode |= S_IFLNK
-	case fs.ModeNamedPipe:
-		jsMode |= S_IFIFO
-	case fs.ModeSocket:
-		jsMode |= S_IFSOCK
-	case fs.ModeDevice:
-		jsMode |= S_IFBLK
-	case fs.ModeCharDevice:
-		jsMode |= S_IFCHR
-	case fs.ModeIrregular:
-		// unmapped to js
+func getJsMode(fm fs.FileMode) (jsMode uint32) {
+	switch {
+	case fm.IsRegular():
+		jsMode = S_IFREG
+	case fm.IsDir():
+		jsMode = S_IFDIR
+	case fm&fs.ModeSymlink != 0:
+		jsMode = S_IFLNK
+	case fm&fs.ModeDevice != 0:
+		// Unlike ModeDevice and ModeCharDevice, S_IFCHR and S_IFBLK are set
+		// mutually exclusively.
+		if fm&fs.ModeCharDevice != 0 {
+			jsMode = S_IFCHR
+		} else {
+			jsMode = S_IFBLK
+		}
+	case fm&fs.ModeNamedPipe != 0:
+		jsMode = S_IFIFO
+	case fm&fs.ModeSocket != 0:
+		jsMode = S_IFSOCK
+	default: // unknown
+		jsMode = 0
 	}
 
-	if mode&fs.ModeSetgid != 0 {
+	jsMode |= uint32(fm & fs.ModePerm)
+
+	if fm&fs.ModeSetgid != 0 {
 		jsMode |= S_ISGID
 	}
-	if mode&fs.ModeSetuid != 0 {
+	if fm&fs.ModeSetuid != 0 {
 		jsMode |= S_ISUID
 	}
-	if mode&fs.ModeSticky != 0 {
+	if fm&fs.ModeSticky != 0 {
 		jsMode |= S_ISVTX
 	}
 	return


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_unit_test.go b/imports/wasi_snapshot_preview1/fs_unit_test.go
index 096ded41..ab7bf51d 100644
--- a/imports/wasi_snapshot_preview1/fs_unit_test.go
+++ b/imports/wasi_snapshot_preview1/fs_unit_test.go
@@ -2,6 +2,7 @@ package wasi_snapshot_preview1
 
 import (
 	"io"
+	"os"
 	"syscall"
 	"testing"
 
@@ -422,3 +423,13 @@ func Test_openFlags(t *testing.T) {
 		})
 	}
 }
+
+func Test_getWasiFiletype_DevNull(t *testing.T) {
+	st, err := os.Stat(os.DevNull)
+	require.NoError(t, err)
+
+	ft := getWasiFiletype(st.Mode())
+
+	// Should be a character device, and not contain permissions
+	require.Equal(t, FILETYPE_CHARACTER_DEVICE, ft)
+}
diff --git a/internal/gojs/fs_unit_test.go b/internal/gojs/fs_unit_test.go
new file mode 100644
index 00000000..3ad7eb7b
--- /dev/null
+++ b/internal/gojs/fs_unit_test.go
@@ -0,0 +1,18 @@
+package gojs
+
+import (
+	"os"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func Test_getWasiFiletype_DevNull(t *testing.T) {
+	st, err := os.Stat(os.DevNull)
+	require.NoError(t, err)
+
+	fm := getJsMode(st.Mode())
+
+	// Should be a character device, and retain the permissions.
+	require.Equal(t, S_IFCHR|uint32(st.Mode().Perm()), fm)
+}