Repository: tetratelabs/wazero
Pull request #1199 (2023-03-05 14:01:06)

wasi: implements platform.UtimesFile for fd_filestat_set_times
This implements `platform.UtimesFile` which is similar to futimes. Before, we were using path-based functionality even though the call site was for a file descriptor.

Note: right now, there's no obvious code in Go to invoke the `futimens` syscall. This means non-windows isn't implemented at nanos granularity, so ends up falling back to the path based option.

Finally, this removes tests for the seldom supported updates with negative epoch time. There's little impact to this as setting times on files before 1970 isn't a typical use case.

codefromthecrypt: rebased
codefromthecrypt: ok I've reworked this as windows is finicky. I want to land this before the improvement, as basically this shows how dire making our own file type is (particularly in windows you need path to do basically anything directory based). After this, I can start working on our file type.

Patch:
diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml
index 111806db..5e7779dc 100644
--- a/.github/workflows/integration.yaml
+++ b/.github/workflows/integration.yaml
@@ -253,6 +253,7 @@ jobs:
     name: wasi-testsuite
     runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false  # don't fail fast as sometimes failures are arch/OS specific
       matrix:
         os: [ubuntu-22.04, macos-12, windows-2022]
 
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 8f4138e5..b8b94980 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -441,7 +441,7 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) Er
 	// - https://go-review.googlesource.com/c/go/+/219638 (unmerged)
 	//
 	// Here, we emulate the behavior for empty flag (meaning "do not change") by get the current time stamp
-	// by explicitly executing File.Stat() prior to Utimes.
+	// by explicitly executing File.Stat() prior to UtimesNano.
 	var atime, mtime int64
 	var nowAtime, statAtime, nowMtime, statMtime bool
 	if set, now := fstFlags&FileStatAdjustFlagsAtim != 0, fstFlags&FileStatAdjustFlagsAtimNow != 0; set && now {
@@ -476,7 +476,10 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) Er
 
 	// Handle if either parameter should be taken from stat.
 	if statAtime || statMtime {
-		// Get the current timestamp via Stat in order to un-change after calling FS.Utimes().
+		if statAtime && statMtime {
+			return ErrnoSuccess // no change
+		}
+		// Get the current timestamp via Stat in order to un-change after calling FS.UtimesNano().
 		var st platform.Stat_t
 		if err := f.Stat(&st); err != nil {
 			return ToErrno(err)
@@ -489,11 +492,16 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) Er
 		}
 	}
 
-	// TODO: this should work against the file descriptor not its last name!
-	if err := f.FS.Utimes(f.Name, atime, mtime); err != nil {
-		return ToErrno(err)
+	// Try to update the file timestamps by file-descriptor.
+	err := platform.UtimesNanoFile(f.File, atime, mtime)
+
+	// Fall back to path based, despite it being less precise.
+	switch err {
+	case syscall.EPERM, syscall.ENOSYS:
+		err = f.FS.UtimesNano(f.Name, atime, mtime)
 	}
-	return ErrnoSuccess
+
+	return ToErrno(err)
 }
 
 // fdPread is the WASI function named FdPreadName which reads from a file
diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go
index 29117564..3386ecd4 100644
--- a/internal/gojs/fs.go
+++ b/internal/gojs/fs.go
@@ -519,7 +519,7 @@ func (jsfsUtimes) invoke(ctx context.Context, mod api.Module, args ...interface{
 	callback := args[3].(funcWrapper)
 
 	fsc := mod.(*wasm.CallContext).Sys.FS()
-	err := fsc.RootFS().Utimes(path, atimeSec*1e9, mtimeSec*1e9)
+	err := fsc.RootFS().UtimesNano(path, atimeSec*1e9, mtimeSec*1e9)
 
 	return jsfsInvoke(ctx, mod, callback, err)
 }
diff --git a/internal/platform/utimes.go b/internal/platform/utimes.go
new file mode 100644
index 00000000..fac570af
--- /dev/null
+++ b/internal/platform/utimes.go
@@ -0,0 +1,35 @@
+package platform
+
+import (
+	"io/fs"
+	"syscall"
+)
+
+// UtimesNano is like syscall.UtimesNano. This returns syscall.ENOENT if the
+// path doesn't exist.
+//
+// Note: This is like the function `utimensat` with `AT_FDCWD` in POSIX.
+// See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
+func UtimesNano(path string, atimeNsec, mtimeNsec int64) error {
+	err := syscall.UtimesNano(path, []syscall.Timespec{
+		syscall.NsecToTimespec(atimeNsec),
+		syscall.NsecToTimespec(mtimeNsec),
+	})
+	return UnwrapOSError(err)
+}
+
+// UtimesNanoFile is like syscall.Futimes, but for nanosecond precision and
+// fs.File instead of a file descriptor. This returns syscall.EBADF if the file
+// or directory was closed, or syscall.EPERM if the file wasn't opened with
+// permission to update its utimes. On syscall.EPERM, or on syscall.ENOSYS, use
+// UtimesNano with the original path.
+//
+// Note: This is like the function `futimens` in POSIX.
+// See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
+func UtimesNanoFile(f fs.File, atimeNsec, mtimeNsec int64) error {
+	if f, ok := f.(fdFile); ok {
+		err := futimens(f.Fd(), atimeNsec, mtimeNsec)
+		return UnwrapOSError(err)
+	}
+	return syscall.ENOSYS
+}
diff --git a/internal/platform/utimes_unsupported.go b/internal/platform/utimes_unsupported.go
new file mode 100644
index 00000000..9594d4bf
--- /dev/null
+++ b/internal/platform/utimes_unsupported.go
@@ -0,0 +1,12 @@
+//go:build !windows
+
+package platform
+
+import "syscall"
+
+func futimens(fd uintptr, atimeNsec, mtimeNsec int64) error {
+	// Go exports syscall.Futimes, which is microsecond granularity, and
+	// WASI tests expect nanosecond. We don't yet have a way to invoke the
+	// futimens syscall portably.
+	return syscall.ENOSYS
+}
diff --git a/internal/platform/utimes_windows.go b/internal/platform/utimes_windows.go
new file mode 100644
index 00000000..e5d4723c
--- /dev/null
+++ b/internal/platform/utimes_windows.go
@@ -0,0 +1,21 @@
+package platform
+
+import "syscall"
+
+func futimens(fd uintptr, atimeNsec, mtimeNsec int64) error {
+	// Before Go 1.20, ERROR_INVALID_HANDLE was returned for too many reasons.
+	// Kick out so that callers can use path-based operations instead.
+	if !IsGo120 {
+		return syscall.ENOSYS
+	}
+
+	// Attempt to get the stat by handle, which works for normal files
+	h := syscall.Handle(fd)
+
+	// Perform logic similar to what's done in syscall.UtimesNano
+	a := syscall.NsecToFiletime(atimeNsec)
+	w := syscall.NsecToFiletime(mtimeNsec)
+
+	// Note: This returns ERROR_ACCESS_DENIED when the input is a directory.
+	return syscall.SetFileTime(h, nil, &a, &w)
+}
diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go
index cedbbf81..28b1983e 100644
--- a/internal/sysfs/dirfs.go
+++ b/internal/sysfs/dirfs.go
@@ -124,13 +124,9 @@ func (d *dirFS) Symlink(oldName, link string) (err error) {
 	return platform.UnwrapOSError(err)
 }
 
-// Utimes implements FS.Utimes
-func (d *dirFS) Utimes(path string, atimeNsec, mtimeNsec int64) error {
-	err := syscall.UtimesNano(d.join(path), []syscall.Timespec{
-		syscall.NsecToTimespec(atimeNsec),
-		syscall.NsecToTimespec(mtimeNsec),
-	})
-	return platform.UnwrapOSError(err)
+// UtimesNano implements FS.UtimesNano
+func (d *dirFS) UtimesNano(name string, atimeNsec, mtimeNsec int64) error {
+	return platform.UtimesNano(d.join(name), atimeNsec, mtimeNsec)
 }
 
 // Truncate implements FS.Truncate
diff --git a/internal/sysfs/rootfs.go b/internal/sysfs/rootfs.go
index 4864de2f..b9e420fa 100644
--- a/internal/sysfs/rootfs.go
+++ b/internal/sysfs/rootfs.go
@@ -308,10 +308,10 @@ func (c *CompositeFS) Link(oldName, newName string) error {
 	return c.fs[fromFS].Link(oldNamePath, newNamePath)
 }
 
-// Utimes implements FS.Utimes
-func (c *CompositeFS) Utimes(path string, atimeNsec, mtimeNsec int64) error {
+// UtimesNano implements FS.UtimesNano
+func (c *CompositeFS) UtimesNano(path string, atimeNsec, mtimeNsec int64) error {
 	matchIndex, relativePath := c.chooseFS(path)
-	return c.fs[matchIndex].Utimes(relativePath, atimeNsec, mtimeNsec)
+	return c.fs[matchIndex].UtimesNano(relativePath, atimeNsec, mtimeNsec)
 }
 
 // Symlink implements FS.Symlink
diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go
index 235d7270..d330052d 100644
--- a/internal/sysfs/sysfs.go
+++ b/internal/sysfs/sysfs.go
@@ -269,8 +269,8 @@ type FS interface {
 	//   - syscall.EACCES: `path` doesn't have write access.
 	Truncate(path string, size int64) error
 
-	// Utimes is similar to syscall.UtimesNano, except the path is relative to
-	// this file system.
+	// UtimesNano is similar to syscall.UtimesNano, except the path is relative
+	// to this file system.
 	//
 	// # Errors
 	//
@@ -283,7 +283,9 @@ type FS interface {
 	//   - To set wall clock time, retrieve it first from sys.Walltime.
 	//   - syscall.UtimesNano cannot change the ctime. Also, neither WASI nor
 	//     runtime.GOOS=js support changing it. Hence, ctime it is absent here.
-	Utimes(path string, atimeNsec, mtimeNsec int64) error
+	//   - This is like the function `utimensat` with `AT_FDCWD` in POSIX.
+	//     See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
+	UtimesNano(path string, atimeNsec, mtimeNsec int64) error
 }
 
 // ReaderAtOffset gets an io.Reader from a fs.File that reads from an offset,
diff --git a/internal/sysfs/unsupported.go b/internal/sysfs/unsupported.go
index 7cf2899b..c89a551c 100644
--- a/internal/sysfs/unsupported.go
+++ b/internal/sysfs/unsupported.go
@@ -86,8 +86,8 @@ func (UnimplementedFS) Unlink(path string) error {
 	return syscall.ENOSYS
 }
 
-// Utimes implements FS.Utimes
-func (UnimplementedFS) Utimes(path string, atimeNsec, mtimeNsec int64) error {
+// UtimesNano implements FS.UtimesNano
+func (UnimplementedFS) UtimesNano(path string, atimeNsec, mtimeNsec int64) error {
 	return syscall.ENOSYS
 }
 


Test patch:
diff --git a/internal/platform/utimes_test.go b/internal/platform/utimes_test.go
new file mode 100644
index 00000000..a6940f91
--- /dev/null
+++ b/internal/platform/utimes_test.go
@@ -0,0 +1,167 @@
+package platform
+
+import (
+	"io/fs"
+	"os"
+	"path"
+	"runtime"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func TestUtimesNano(t *testing.T) {
+	tmpDir := t.TempDir()
+	file := path.Join(tmpDir, "file")
+	err := os.WriteFile(file, []byte{}, 0o700)
+	require.NoError(t, err)
+
+	dir := path.Join(tmpDir, "dir")
+	err = os.Mkdir(dir, 0o700)
+	require.NoError(t, err)
+
+	t.Run("doesn't exist", func(t *testing.T) {
+		err := UtimesNano("nope",
+			time.Unix(123, 4*1e3).UnixNano(),
+			time.Unix(567, 8*1e3).UnixNano())
+		require.EqualErrno(t, syscall.ENOENT, err)
+	})
+
+	type test struct {
+		name                 string
+		path                 string
+		atimeNsec, mtimeNsec int64
+	}
+
+	// Note: This sets microsecond granularity because Windows doesn't support
+	// nanosecond.
+	//
+	// Negative isn't tested as most platforms don't return consistent results.
+	tests := []test{
+		{
+			name:      "file positive",
+			path:      file,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{
+			name:      "dir positive",
+			path:      dir,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{name: "file zero", path: file},
+		{name: "dir zero", path: dir},
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			err := UtimesNano(tc.path, tc.atimeNsec, tc.mtimeNsec)
+			require.NoError(t, err)
+
+			var stat Stat_t
+			require.NoError(t, Stat(tc.path, &stat))
+			if CompilerSupported() {
+				require.Equal(t, stat.Atim, tc.atimeNsec)
+			} // else only mtimes will return.
+			require.Equal(t, stat.Mtim, tc.mtimeNsec)
+		})
+	}
+}
+
+func TestUtimesNanoFile(t *testing.T) {
+	if !(runtime.GOOS == "windows" && IsGo120) {
+		t.Skip("TODO: implement futimens on darwin, freebsd, linux w/o CGO")
+	}
+
+	tmpDir := t.TempDir()
+
+	file := path.Join(tmpDir, "file")
+	err := os.WriteFile(file, []byte{}, 0o700)
+	require.NoError(t, err)
+	fileF, err := OpenFile(file, syscall.O_RDWR, 0)
+	require.NoError(t, err)
+	defer fileF.Close()
+
+	dir := path.Join(tmpDir, "dir")
+	err = os.Mkdir(dir, 0o700)
+	require.NoError(t, err)
+	dirF, err := OpenFile(dir, syscall.O_RDONLY, 0)
+	require.NoError(t, err)
+	defer fileF.Close()
+
+	type test struct {
+		name                 string
+		file                 fs.File
+		atimeNsec, mtimeNsec int64
+		expectedErr          error
+	}
+
+	// Note: This sets microsecond granularity because Windows doesn't support
+	// nanosecond.
+	//
+	// Negative isn't tested as most platforms don't return consistent results.
+	tests := []*test{
+		{
+			name:      "file positive",
+			file:      fileF,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{name: "file zero", file: fileF},
+		{
+			name:      "dir positive",
+			file:      dirF,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{name: "dir zero", file: dirF},
+	}
+
+	// In windows, trying to update the time of a directory fails, as it is
+	// addressed by path, not by file descriptor.
+	if runtime.GOOS == "windows" {
+		for _, tt := range tests {
+			if tt.file == dirF {
+				tt.expectedErr = syscall.EPERM
+			}
+		}
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			err := UtimesNanoFile(tc.file, tc.atimeNsec, tc.mtimeNsec)
+			if tc.expectedErr != nil {
+				require.EqualErrno(t, tc.expectedErr.(syscall.Errno), err)
+				return
+			}
+
+			var stat Stat_t
+			require.NoError(t, StatFile(tc.file, &stat))
+			if CompilerSupported() {
+				require.Equal(t, stat.Atim, tc.atimeNsec)
+			} // else only mtimes will return.
+			require.Equal(t, stat.Mtim, tc.mtimeNsec)
+		})
+	}
+
+	require.NoError(t, fileF.Close())
+	t.Run("closed file", func(t *testing.T) {
+		err := UtimesNanoFile(fileF,
+			time.Unix(123, 4*1e3).UnixNano(),
+			time.Unix(567, 8*1e3).UnixNano())
+		require.EqualErrno(t, syscall.EBADF, err)
+	})
+
+	require.NoError(t, dirF.Close())
+	t.Run("closed dir", func(t *testing.T) {
+		err := UtimesNanoFile(dirF,
+			time.Unix(123, 4*1e3).UnixNano(),
+			time.Unix(567, 8*1e3).UnixNano())
+		require.EqualErrno(t, syscall.EBADF, err)
+	})
+}
diff --git a/internal/sysfs/adapter_test.go b/internal/sysfs/adapter_test.go
index b877d7d9..25f0d88a 100644
--- a/internal/sysfs/adapter_test.go
+++ b/internal/sysfs/adapter_test.go
@@ -83,7 +83,7 @@ func TestAdapt_Unlink(t *testing.T) {
 	require.EqualErrno(t, syscall.ENOSYS, err)
 }
 
-func TestAdapt_Utimes(t *testing.T) {
+func TestAdapt_UtimesNano(t *testing.T) {
 	tmpDir := t.TempDir()
 	testFS := Adapt(os.DirFS(tmpDir))
 
@@ -91,7 +91,7 @@ func TestAdapt_Utimes(t *testing.T) {
 	realPath := pathutil.Join(tmpDir, path)
 	require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))
 
-	err := testFS.Utimes(path, 1, 1)
+	err := testFS.UtimesNano(path, 1, 1)
 	require.EqualErrno(t, syscall.ENOSYS, err)
 }
 
diff --git a/internal/sysfs/dirfs_test.go b/internal/sysfs/dirfs_test.go
index aed16128..9753b626 100644
--- a/internal/sysfs/dirfs_test.go
+++ b/internal/sysfs/dirfs_test.go
@@ -498,11 +498,11 @@ func TestDirFS_Unlink(t *testing.T) {
 	})
 }
 
-func TestDirFS_Utimes(t *testing.T) {
+func TestDirFS_UtimesNano(t *testing.T) {
 	tmpDir := t.TempDir()
 	testFS := NewDirFS(tmpDir)
 
-	testUtimes(t, tmpDir, testFS)
+	testUtimesNano(t, tmpDir, testFS)
 }
 
 func TestDirFS_OpenFile(t *testing.T) {
diff --git a/internal/sysfs/readfs_test.go b/internal/sysfs/readfs_test.go
index df13b4d8..da7ebc31 100644
--- a/internal/sysfs/readfs_test.go
+++ b/internal/sysfs/readfs_test.go
@@ -113,7 +113,7 @@ func TestReadFS_Unlink(t *testing.T) {
 	require.EqualErrno(t, syscall.ENOSYS, err)
 }
 
-func TestReadFS_Utimes(t *testing.T) {
+func TestReadFS_UtimesNano(t *testing.T) {
 	tmpDir := t.TempDir()
 	writeable := NewDirFS(tmpDir)
 	testFS := NewReadFS(writeable)
@@ -122,7 +122,7 @@ func TestReadFS_Utimes(t *testing.T) {
 	realPath := pathutil.Join(tmpDir, path)
 	require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))
 
-	err := testFS.Utimes(path, 1, 1)
+	err := testFS.UtimesNano(path, 1, 1)
 	require.EqualErrno(t, syscall.ENOSYS, err)
 }
 
diff --git a/internal/sysfs/sysfs_test.go b/internal/sysfs/sysfs_test.go
index cecbc49e..924facb7 100644
--- a/internal/sysfs/sysfs_test.go
+++ b/internal/sysfs/sysfs_test.go
@@ -389,7 +389,7 @@ func testReadlink(t *testing.T, readFS, writeFS FS) {
 	})
 }
 
-func testUtimes(t *testing.T, tmpDir string, testFS FS) {
+func testUtimesNano(t *testing.T, tmpDir string, testFS FS) {
 	file := "file"
 	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
 	require.NoError(t, err)
@@ -399,7 +399,7 @@ func testUtimes(t *testing.T, tmpDir string, testFS FS) {
 	require.NoError(t, err)
 
 	t.Run("doesn't exist", func(t *testing.T) {
-		err := testFS.Utimes("nope",
+		err := testFS.UtimesNano("nope",
 			time.Unix(123, 4*1e3).UnixNano(),
 			time.Unix(567, 8*1e3).UnixNano())
 		require.EqualErrno(t, syscall.ENOENT, err)
@@ -413,6 +413,8 @@ func testUtimes(t *testing.T, tmpDir string, testFS FS) {
 
 	// Note: This sets microsecond granularity because Windows doesn't support
 	// nanosecond.
+	//
+	// Negative isn't tested as most platforms don't return consistent results.
 	tests := []test{
 		{
 			name:      "file positive",
@@ -430,28 +432,10 @@ func testUtimes(t *testing.T, tmpDir string, testFS FS) {
 		{name: "dir zero", path: dir},
 	}
 
-	// linux and freebsd report inaccurate results when the input ts is negative.
-	if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
-		tests = append(tests,
-			test{
-				name:      "file negative",
-				path:      file,
-				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
-				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
-			},
-			test{
-				name:      "dir negative",
-				path:      dir,
-				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
-				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
-			},
-		)
-	}
-
 	for _, tt := range tests {
 		tc := tt
 		t.Run(tc.name, func(t *testing.T) {
-			err := testFS.Utimes(tc.path, tc.atimeNsec, tc.mtimeNsec)
+			err := testFS.UtimesNano(tc.path, tc.atimeNsec, tc.mtimeNsec)
 			require.NoError(t, err)
 
 			var stat platform.Stat_t