Repository: tetratelabs/wazero
Pull request #388 (2022-03-17 05:34:04)

wasi: clarifies errno results of FdPrestatGet and PathOpen
Extracts some of #362 so that the PR is less dense with unrelated changes.

Other WASI API changes were made in a way fused with fs changes, which means they are pinned to the other PR.


Patch:
diff --git a/internal/wasi/fs.go b/internal/wasi/fs.go
index 95d0682c..71e4d45d 100644
--- a/internal/wasi/fs.go
+++ b/internal/wasi/fs.go
@@ -13,22 +13,6 @@ import (
 
 type DirFS string
 
-func posixOpenFlags(oFlags uint32, fsRights uint64) (pFlags int) {
-	if fsRights&wasi.R_FD_WRITE != 0 {
-		pFlags |= os.O_RDWR
-	}
-	if oFlags&wasi.O_CREATE != 0 {
-		pFlags |= os.O_CREATE
-	}
-	if oFlags&wasi.O_EXCL != 0 {
-		pFlags |= os.O_EXCL
-	}
-	if oFlags&wasi.O_TRUNC != 0 {
-		pFlags |= os.O_TRUNC
-	}
-	return
-}
-
 func (dir DirFS) OpenWASI(dirFlags uint32, path string, oFlags uint32, fsRights, fsRightsInheriting uint64, fdFlags uint32) (wasi.File, error) {
 	// I'm not sure how to use all these passed flags and rights yet
 	if !fs.ValidPath(path) || runtime.GOOS == "windows" && strings.ContainsAny(path, `\:`) {
diff --git a/internal/wasi/wasi.go b/internal/wasi/wasi.go
index d80819cb..98c0d634 100644
--- a/internal/wasi/wasi.go
+++ b/internal/wasi/wasi.go
@@ -636,11 +636,14 @@ type SnapshotPreview1 interface {
 	FdPread(ctx wasm.Module, fd, iovs uint32, offset uint64, resultNread uint32) wasi.Errno
 
 	// FdPrestatGet is the WASI function to return the prestat data of a file descriptor.
-	// This returns wasi.ErrnoBadf if the fd is invalid.
 	//
 	// * fd - the file descriptor to get the prestat
 	// * resultPrestat - the offset to write the result prestat data
 	//
+	// The wasi.Errno returned is wasi.ErrnoSuccess except the following error conditions:
+	// * wasi.ErrnoBadf - if `fd` is invalid or the `fd` is not a pre-opened directory.
+	// * wasi.ErrnoFault - if `resultPrestat` is an invalid offset due to the memory constraint
+	//
 	// prestat byte layout is 8 bytes, beginning with an 8-bit tag and 3 pad bytes. The only valid tag is `prestat_dir`,
 	// which is tag zero. This simplifies the byte layout to 4 empty bytes followed by the uint32le encoded path length.
 	//
@@ -857,6 +860,14 @@ type SnapshotPreview1 interface {
 	// * resultOpenedFd - the offset in `ctx.Memory` to write the newly created file descriptor to.
 	//     * The result FD value is guaranteed to be less than 2**31
 	//
+	// The wasi.Errno returned is wasi.ErrnoSuccess except the following error conditions:
+	// * wasi.ErrnoBadf - if `fd` is invalid
+	// * wasi.ErrnoFault - if `resultOpenedFd` contains an invalid offset due to the memory constraint
+	// * wasi.ErrnoNoent - if `path` does not exist.
+	// * wasi.ErrnoExist - if `path` exists, while `oFlags` requires that it must not.
+	// * wasi.ErrnoNotdir - if `path` is not a directory, while `oFlags` requires that it must be.
+	// * wasi.ErrnoIo - if other error happens during the operation of the underying file system.
+	//
 	// For example, this function needs to first read `path` to determine the file to open.
 	//    If parameters `path` = 1, `pathLen` = 6, and the path is "wazero", PathOpen reads the path from `ctx.Memory`:
 	//
@@ -866,13 +877,13 @@ type SnapshotPreview1 interface {
 	//   []byte{ ?, 'w', 'a', 'z', 'e', 'r', 'o', ?... }
 	//        path --^
 	//
-	// Then, if parameters resultOpenedFd = 8, and this function opened a new file descriptor 3 with the given flags,
+	// Then, if parameters resultOpenedFd = 8, and this function opened a new file descriptor 5 with the given flags,
 	// this function writes the blow to `ctx.Memory`:
 	//
 	//                          uint32le
 	//                         +--------+
 	//                         |        |
-	//        []byte{ 0..6, ?, 4, 0, 0, 0, ?}
+	//        []byte{ 0..6, ?, 5, 0, 0, 0, ?}
 	//        resultOpenedFd --^
 	//
 	// Note: ImportPathOpen shows this signature in the WebAssembly 1.0 (20191205) Text Format.
@@ -1129,20 +1140,30 @@ func (a *wasiAPI) FdFdstatGet(ctx wasm.Module, fd uint32, resultStat uint32) was
 	if _, ok := cfg.opened[fd]; !ok {
 		return wasi.ErrnoBadf
 	}
-	if !ctx.Memory().WriteUint64Le(resultStat+16, wasi.R_FD_READ|wasi.R_FD_WRITE) {
+	if !ctx.Memory().WriteUint64Le(resultStat+16, rightFDRead|rightFDWrite) {
 		return wasi.ErrnoFault
 	}
 	return wasi.ErrnoSuccess
 }
 
 // FdPrestatGet implements SnapshotPreview1.FdPrestatGet
-// TODO: Currently FdPrestatGet implements nothing except returning ErrnoBadf
-func (a *wasiAPI) FdPrestatGet(ctx wasm.Module, fd uint32, bufPtr uint32) wasi.Errno {
+func (a *wasiAPI) FdPrestatGet(ctx wasm.Module, fd uint32, resultPrestat uint32) wasi.Errno {
 	cfg := a.config(ctx.Context())
 
-	if _, ok := cfg.opened[fd]; !ok {
+	entry, ok := cfg.opened[fd]
+	if !ok || entry.path == "" {
 		return wasi.ErrnoBadf
 	}
+
+	// Zero-value 8-bit tag, and 3-byte zero-value paddings, which is uint32le(0) in short.
+	if !ctx.Memory().WriteUint32Le(resultPrestat, uint32(0)) {
+		return wasi.ErrnoFault
+	}
+	// Write the length of the directory name at offset 4.
+	if !ctx.Memory().WriteUint32Le(resultPrestat+4, uint32(len(entry.path))) {
+		return wasi.ErrnoFault
+	}
+
 	return wasi.ErrnoSuccess
 }
 
@@ -1266,11 +1287,15 @@ func (a *wasiAPI) FdSeek(ctx wasm.Module, fd uint32, offset uint64, whence uint3
 	if !ok || f.file == nil {
 		return wasi.ErrnoBadf
 	}
+	seeker, ok := f.file.(io.Seeker)
+	if !ok {
+		return wasi.ErrnoBadf
+	}
 
 	if whence > io.SeekEnd /* exceeds the largest valid whence */ {
 		return wasi.ErrnoInval
 	}
-	newOffst, err := f.file.Seek(int64(offset), int(whence))
+	newOffst, err := seeker.Seek(int64(offset), int(whence))
 	if err != nil {
 		return wasi.ErrnoIo
 	}
@@ -1358,8 +1383,42 @@ func (a *wasiAPI) PathLink(ctx wasm.Module, oldFd, oldFlags, oldPath, oldPathLen
 	return wasi.ErrnoNosys // stubbed for GrainLang per #271
 }
 
+const (
+	// WASI open flags
+	oflagCreate = 1 << iota
+	// TODO: oflagDir
+	oflagExclusive
+	oflagTrunc
+
+	// WASI FS rights
+	rightFDRead  = 1 << iota
+	rightFDWrite = 0x200
+)
+
+func posixOpenFlags(oFlags uint32, fsRights uint64) (pFlags int) {
+	// TODO: handle dirflags, which decides whether to follow symbolic links or not,
+	//       by O_NOFOLLOW. Note O_NOFOLLOW doesn't exist on Windows.
+	if fsRights&rightFDWrite != 0 {
+		if fsRights&rightFDRead != 0 {
+			pFlags |= os.O_RDWR
+		} else {
+			pFlags |= os.O_WRONLY
+		}
+	}
+	if oFlags&oflagCreate != 0 {
+		pFlags |= os.O_CREATE
+	}
+	if oFlags&oflagExclusive != 0 {
+		pFlags |= os.O_EXCL
+	}
+	if oFlags&oflagTrunc != 0 {
+		pFlags |= os.O_TRUNC
+	}
+	return
+}
+
 // PathOpen implements SnapshotPreview1.PathOpen
-func (a *wasiAPI) PathOpen(ctx wasm.Module, fd, dirflags, path, pathLen, oflags uint32, fsRightsBase,
+func (a *wasiAPI) PathOpen(ctx wasm.Module, fd, dirflags, pathPtr, pathLen, oflags uint32, fsRightsBase,
 	fsRightsInheriting uint64, fdflags, resultOpenedFd uint32) (errno wasi.Errno) {
 	cfg := a.config(ctx.Context())
 
@@ -1368,7 +1427,7 @@ func (a *wasiAPI) PathOpen(ctx wasm.Module, fd, dirflags, path, pathLen, oflags
 		return wasi.ErrnoBadf
 	}
 
-	b, ok := ctx.Memory().Read(path, pathLen)
+	b, ok := ctx.Memory().Read(pathPtr, pathLen)
 	if !ok {
 		return wasi.ErrnoFault
 	}
@@ -1378,11 +1437,16 @@ func (a *wasiAPI) PathOpen(ctx wasm.Module, fd, dirflags, path, pathLen, oflags
 		switch {
 		case errors.Is(err, fs.ErrNotExist):
 			return wasi.ErrnoNoent
+		case errors.Is(err, fs.ErrExist):
+			return wasi.ErrnoExist
 		default:
-			return wasi.ErrnoInval
+			return wasi.ErrnoIo
 		}
 	}
 
+	// when ofagDir is set, the opened file must be a directory.
+	// TODO if oflags&oflagDir != 0 return wasi.ErrnoNotdir if stat != dir
+
 	newFD, err := a.randUnusedFD(cfg)
 	if err != nil {
 		return wasi.ErrnoIo


Test patch:
diff --git a/internal/wasi/wasi_test.go b/internal/wasi/wasi_test.go
index 4f71dec0..4d6d14b6 100644
--- a/internal/wasi/wasi_test.go
+++ b/internal/wasi/wasi_test.go
@@ -780,7 +780,96 @@ func TestSnapshotPreview1_FdPread(t *testing.T) {
 	})
 }
 
-// TODO: TestSnapshotPreview1_FdPrestatGet TestSnapshotPreview1_FdPrestatGet_Errors
+func TestSnapshotPreview1_FdPrestatGet(t *testing.T) {
+	fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err
+
+	ctx := context.Background()
+	config := NewConfig()
+	config.opened[fd] = fileEntry{
+		path: "/tmp",
+	}
+
+	mod, fn := instantiateModule(t, ctx, FunctionFdPrestatGet, ImportFdPrestatGet, moduleName, config)
+
+	resultPrestat := uint32(1) // arbitrary offset
+	expectedMemory := []byte{
+		'?',     // resultPrstat after this
+		0,       // 8-bit tag indicating `prestat_dir`, the only available tag
+		0, 0, 0, // 3-byte padding
+		// the result path length field after this
+		4, 0, 0, 0, // = 4, which is len("/tmp")
+		'?',
+	}
+
+	t.Run("SnapshotPreview1.FdPrestatGet", func(t *testing.T) {
+		maskMemory(t, mod, len(expectedMemory))
+
+		errno := NewAPI(config).FdPrestatGet(mod, fd, resultPrestat)
+		require.Equal(t, wasi.ErrnoSuccess, errno)
+
+		actual, ok := mod.Memory().Read(0, uint32(len(expectedMemory)))
+		require.True(t, ok)
+		require.Equal(t, expectedMemory, actual)
+	})
+
+	t.Run(FunctionFdPrestatDirName, func(t *testing.T) {
+		maskMemory(t, mod, len(expectedMemory))
+
+		ret, err := fn.Call(mod, uint64(fd), uint64(resultPrestat))
+		require.NoError(t, err)
+		require.Equal(t, wasi.ErrnoSuccess, wasi.Errno(ret[0])) // cast because results are always uint64
+
+		actual, ok := mod.Memory().Read(0, uint32(len(expectedMemory)))
+		require.True(t, ok)
+		require.Equal(t, expectedMemory, actual)
+	})
+}
+
+func TestSnapshotPreview1_FdPrestatGet_Errors(t *testing.T) {
+	fd := uint32(3)           // fd 3 will be opened for the "/tmp" directory after 0, 1, and 2, that are stdin/out/err
+	validAddress := uint32(0) // Arbitrary valid address as arguments to fd_prestat_get. We chose 0 here.
+
+	ctx := context.Background()
+	config := NewConfig()
+	config.opened = map[uint32]fileEntry{
+		fd: {path: "/tmp"},
+	}
+
+	mod, fn := instantiateModule(t, ctx, FunctionFdPrestatGet, ImportFdPrestatGet, moduleName, config)
+
+	memorySize := mod.Memory().Size()
+
+	tests := []struct {
+		name          string
+		fd            uint32
+		resultPrestat uint32
+		expectedErrno wasi.Errno
+	}{
+		{
+			name:          "invalid FD",
+			fd:            42, // arbitrary invalid FD
+			resultPrestat: validAddress,
+			expectedErrno: wasi.ErrnoBadf,
+		},
+		{
+			name:          "out-of-memory resultPrestat",
+			fd:            fd,
+			resultPrestat: memorySize,
+			expectedErrno: wasi.ErrnoFault,
+		},
+		// TODO: non pre-opened file == wasi.ErrnoBadf
+	}
+
+	for _, tt := range tests {
+		tc := tt
+
+		t.Run(tc.name, func(t *testing.T) {
+			results, err := fn.Call(mod, uint64(tc.fd), uint64(tc.resultPrestat))
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedErrno, wasi.Errno(results[0])) // results[0] is the errno
+		})
+	}
+}
 
 func TestSnapshotPreview1_FdPrestatDirName(t *testing.T) {
 	fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err
@@ -827,16 +916,19 @@ func TestSnapshotPreview1_FdPrestatDirName(t *testing.T) {
 }
 
 func TestSnapshotPreview1_FdPrestatDirName_Errors(t *testing.T) {
-	dirName := "/tmp"
+	fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err
+
 	ctx := context.Background()
 	config := NewConfig()
-	config.Preopen(dirName, &MemFS{})
+	config.opened = map[uint32]fileEntry{
+		fd: {path: "/tmp"},
+	}
+	pathLen := uint32(len("/tmp"))
 
 	mod, fn := instantiateModule(t, ctx, FunctionFdPrestatDirName, ImportFdPrestatDirName, moduleName, config)
 
 	memorySize := mod.Memory().Size()
 	validAddress := uint32(0) // Arbitrary valid address as arguments to fd_prestat_dir_name. We chose 0 here.
-	fd := uint32(3)           // fd 3 will be opened for the "/tmp" directory after 0, 1, and 2, that are stdin/out/err
 
 	tests := []struct {
 		name          string
@@ -849,30 +941,31 @@ func TestSnapshotPreview1_FdPrestatDirName_Errors(t *testing.T) {
 			name:          "out-of-memory path",
 			fd:            fd,
 			path:          memorySize,
-			pathLen:       uint32(len(dirName)),
+			pathLen:       pathLen,
 			expectedErrno: wasi.ErrnoFault,
 		},
 		{
 			name:          "path exceeds the maximum valid address by 1",
 			fd:            fd,
-			path:          memorySize - uint32(len(dirName)) + 1,
-			pathLen:       uint32(len(dirName)),
+			path:          memorySize - pathLen + 1,
+			pathLen:       pathLen,
 			expectedErrno: wasi.ErrnoFault,
 		},
 		{
 			name:          "pathLen exceeds the length of the dir name",
 			fd:            fd,
 			path:          validAddress,
-			pathLen:       uint32(len(dirName)) + 1,
+			pathLen:       pathLen + 1,
 			expectedErrno: wasi.ErrnoNametoolong,
 		},
 		{
 			name:          "invalid fd",
 			fd:            42, // arbitrary invalid fd
 			path:          validAddress,
-			pathLen:       uint32(len(dirName)) + 1,
+			pathLen:       pathLen,
 			expectedErrno: wasi.ErrnoBadf,
 		},
+		// TODO: non pre-opened file == wasi.ErrnoBadf
 	}
 
 	for _, tt := range tests {
@@ -1521,26 +1614,34 @@ func TestSnapshotPreview1_PathLink(t *testing.T) {
 }
 
 func TestSnapshotPreview1_PathOpen(t *testing.T) {
-	fd := uint32(3)                              // arbitrary fd after 0, 1, and 2, that are stdin/out/err
-	dirflags := uint32(0)                        // arbitrary dirflags
-	path := uint32(1)                            // arbitrary offset
-	pathLen := uint32(6)                         // The length of path
-	oflags := uint32(0)                          // arbitrary oflags
-	fsRightsBase := uint64(wasi.R_FD_READ)       // arbitrary right
-	fsRightsInheriting := uint64(wasi.R_FD_READ) // arbitrary right
+	workdirFD := uint32(3)                    // arbitrary fd after 0, 1, and 2, that are stdin/out/err
+	dirflags := uint32(0)                     // arbitrary dirflags
+	path := uint32(1)                         // arbitrary offset
+	pathLen := uint32(6)                      // The length of path
+	oflags := uint32(0)                       // arbitrary oflags
+	fsRightsBase := uint64(rightFDRead)       // arbitrary right
+	fsRightsInheriting := uint64(rightFDRead) // arbitrary right
 	fdFlags := uint32(0)
 	resultOpenedFd := uint32(8)
 	initialMemory := []byte{
 		'?',                          // `path` is after this
 		'w', 'a', 'z', 'e', 'r', 'o', // path
-		'?', // `resultOpenedFd` is after this
 	}
+	expectedFD := uint32(5) // arbitrary expected FD
 	expectedMemory := append(
 		initialMemory,
-		4, 0, 0, 0, // resultOpenedFd
+		'?',        // `resultOpenedFd` is after this
+		5, 0, 0, 0, // = epxectedFD (5)
 		'?',
 	)
-	expectedFD := uint32(4) // arbitrary expected FD
+
+	// MemFS for testing
+	// Create a memFS for testing that has "./wazero" file.
+	memFS := &MemFS{
+		Files: map[string][]byte{
+			"wazero": {},
+		},
+	}
 
 	ctx := context.Background()
 	config := NewConfig()
@@ -1556,7 +1657,7 @@ func TestSnapshotPreview1_PathOpen(t *testing.T) {
 	type pathOpenFn func(ctx publicwasm.Module, fd, dirflags, path, pathLen, oflags uint32,
 		fsRightsBase, fsRightsInheriting uint64,
 		fdFlags, resultOpenedFd uint32) wasi.Errno
-	tests := []struct {
+	pathOpenFns := []struct {
 		name     string
 		pathOpen func() pathOpenFn
 	}{
@@ -1574,38 +1675,50 @@ func TestSnapshotPreview1_PathOpen(t *testing.T) {
 		}},
 	}
 
-	for _, tt := range tests {
-		tc := tt
-		t.Run(tc.name, func(t *testing.T) {
-			// Create a memFS for testing that has "./wazero" file.
-			memFS := &MemFS{
-				Files: map[string][]byte{
-					"wazero": []byte(""),
-				},
-			}
-			config.opened = map[uint32]fileEntry{
-				fd: {
-					path:    ".",
-					fileSys: memFS,
-				},
-			}
+	tests := []struct {
+		name         string
+		fd           uint32
+		expectedPath string
+	}{
+		{
+			name:         "simple file open",
+			fd:           workdirFD,
+			expectedPath: "wazero",
+		},
+	}
 
-			maskMemory(t, mod, len(expectedMemory))
-			ok := mod.Memory().Write(0, initialMemory)
-			require.True(t, ok)
+	for _, pathOpenFn := range pathOpenFns {
+		pf := pathOpenFn
+		t.Run(pf.name, func(t *testing.T) {
+			for _, tt := range tests {
+				tc := tt
+				t.Run(tc.name, func(t *testing.T) {
+					// Set up a fresh opened FD table
+					config.opened = map[uint32]fileEntry{
+						workdirFD: {
+							path:    ".",
+							fileSys: memFS,
+						},
+					}
+
+					maskMemory(t, mod, len(expectedMemory))
+					ok := mod.Memory().Write(0, initialMemory)
+					require.True(t, ok)
 
-			errno := tc.pathOpen()(mod, fd, dirflags, path, pathLen, oflags, fsRightsBase, fsRightsInheriting, fdFlags, resultOpenedFd)
-			require.Equal(t, wasi.ErrnoSuccess, errno)
+					errno := pf.pathOpen()(mod, tc.fd, dirflags, path, pathLen, oflags, fsRightsBase, fsRightsInheriting, fdFlags, resultOpenedFd)
+					require.Equal(t, wasi.ErrnoSuccess, errno)
 
-			actual, ok := mod.Memory().Read(0, uint32(len(expectedMemory)))
-			require.True(t, ok)
-			require.Equal(t, expectedMemory, actual)
-			require.Equal(t, "wazero", config.opened[expectedFD].path) // verify the file was actually opened
+					actual, ok := mod.Memory().Read(0, uint32(len(expectedMemory)))
+					require.True(t, ok)
+					require.Equal(t, expectedMemory, actual)
+					require.Equal(t, tc.expectedPath, config.opened[expectedFD].path) // verify the file was actually opened
+				})
+			}
 		})
 	}
 }
 
-func TestSnapshotPreview1_PathOpen_Erros(t *testing.T) {
+func TestSnapshotPreview1_PathOpen_Errors(t *testing.T) {
 	validFD := uint64(3) // arbitrary valid fd after 0, 1, and 2, that are stdin/out/err
 	// Create a memFS for testing that has "./wazero" file.
 	memFS := &MemFS{
@@ -1631,9 +1744,9 @@ func TestSnapshotPreview1_PathOpen_Erros(t *testing.T) {
 	}) // wazero is the path to the file in the memFS
 
 	tests := []struct {
-		name                              string
-		fd, path, pathLen, resultOpenedFd uint64
-		expectedErrno                     wasi.Errno
+		name                                      string
+		fd, path, pathLen, oflags, resultOpenedFd uint64
+		expectedErrno                             wasi.Errno
 	}{
 		{
 			name:          "invalid fd",
@@ -1674,7 +1787,7 @@ func TestSnapshotPreview1_PathOpen_Erros(t *testing.T) {
 	for _, tt := range tests {
 		tc := tt
 		t.Run(tc.name, func(t *testing.T) {
-			results, err := fn.Call(mod, tc.fd, 0, tc.path, tc.pathLen, 0, 0, 0, 0, tc.resultOpenedFd)
+			results, err := fn.Call(mod, tc.fd, 0, tc.path, tc.pathLen, tc.oflags, 0, 0, 0, tc.resultOpenedFd)
 			require.NoError(t, err)
 			require.Equal(t, tc.expectedErrno, wasi.Errno(results[0])) // results[0] is the errno
 		})