internal/poll: don't call SetFilePointerEx in Seek for overlapped handles

Overlapped handles don't have the file pointer updated when performing
I/O operations, so there is no need to call syscall.SetFilePointerEx in
FD.Seek. Updating the in-memory offset is sufficient.

Updates #74951 (provides a more complete fix)

Change-Id: Ibede6625cdbd501fc92cfdf8ce2782ec291af2b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/698035
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go
index 23500d3e390f4aa601227495e628d93c5dbec7f7..4d466e5b64c9d07e58bc81d12e62a3e5b7b7b2c7 100644 (file)
--- a/src/internal/poll/fd_windows.go
+++ b/src/internal/poll/fd_windows.go
@@ -1165,11 +1165,29 @@ func (fd *FD) Seek(offset int64, whence int) (int64, error) {
        }
        defer fd.readWriteUnlock()
 
-       if !fd.isBlocking && whence == io.SeekCurrent {
-               // Windows doesn't keep the file pointer for overlapped file handles.
-               // We do it ourselves in case to account for any read or write
-               // operations that may have occurred.
-               offset += fd.offset
+       if !fd.isBlocking {
+               // Windows doesn't use the file pointer for overlapped file handles,
+               // there is no point on calling syscall.Seek.
+               var newOffset int64
+               switch whence {
+               case io.SeekStart:
+                       newOffset = offset
+               case io.SeekCurrent:
+                       newOffset = fd.offset + offset
+               case io.SeekEnd:
+                       var size int64
+                       if err := windows.GetFileSizeEx(fd.Sysfd, &size); err != nil {
+                               return 0, err
+                       }
+                       newOffset = size + offset
+               default:
+                       return 0, windows.ERROR_INVALID_PARAMETER
+               }
+               if newOffset < 0 {
+                       return 0, windows.ERROR_NEGATIVE_SEEK
+               }
+               fd.setOffset(newOffset)
+               return newOffset, nil
        }
        n, err := syscall.Seek(fd.Sysfd, offset, whence)
        fd.setOffset(n)

diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go
index 905cabc81e479873fab9f7437bcd4ca5d8a4106a..b60648ea299126e3bec95c924c9bebefaf6b0d34 100644 (file)
--- a/src/internal/syscall/windows/syscall_windows.go
+++ b/src/internal/syscall/windows/syscall_windows.go
@@ -39,6 +39,7 @@ const (
        ERROR_NOT_SUPPORTED          syscall.Errno = 50
        ERROR_CALL_NOT_IMPLEMENTED   syscall.Errno = 120
        ERROR_INVALID_NAME           syscall.Errno = 123
+       ERROR_NEGATIVE_SEEK          syscall.Errno = 131
        ERROR_LOCK_FAILED            syscall.Errno = 167
        ERROR_IO_INCOMPLETE          syscall.Errno = 996
        ERROR_NO_TOKEN               syscall.Errno = 1008
@@ -195,6 +196,7 @@ const (
 //sys  SetFileInformationByHandle(handle syscall.Handle, fileInformationClass uint32, buf unsafe.Pointer, bufsize uint32) (err error) = kernel32.SetFileInformationByHandle
 //sys  VirtualQuery(address uintptr, buffer *MemoryBasicInformation, length uintptr) (err error) = kernel32.VirtualQuery
 //sys  GetTempPath2(buflen uint32, buf *uint16) (n uint32, err error) = GetTempPath2W
+//sys  GetFileSizeEx(handle syscall.Handle, size *int64) (err error) = kernel32.GetFileSizeEx
 
 const (
        // flags for CreateToolhelp32Snapshot

diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go
index 53f456c5cfa4e77131e205af375a6cf2d0016701..fb8bc80629a626972d532d7fa1548abbb3ef27ef 100644 (file)
--- a/src/internal/syscall/windows/zsyscall_windows.go
+++ b/src/internal/syscall/windows/zsyscall_windows.go
@@ -73,6 +73,7 @@ var (
        procGetConsoleCP                      = modkernel32.NewProc("GetConsoleCP")
        procGetCurrentThread                  = modkernel32.NewProc("GetCurrentThread")
        procGetFileInformationByHandleEx      = modkernel32.NewProc("GetFileInformationByHandleEx")
+       procGetFileSizeEx                     = modkernel32.NewProc("GetFileSizeEx")
        procGetFinalPathNameByHandleW         = modkernel32.NewProc("GetFinalPathNameByHandleW")
        procGetModuleFileNameW                = modkernel32.NewProc("GetModuleFileNameW")
        procGetModuleHandleW                  = modkernel32.NewProc("GetModuleHandleW")
@@ -326,6 +327,14 @@ func GetFileInformationByHandleEx(handle syscall.Handle, class uint32, info *byt
        return
 }
 
+func GetFileSizeEx(handle syscall.Handle, size *int64) (err error) {
+       r1, _, e1 := syscall.Syscall(procGetFileSizeEx.Addr(), 2, uintptr(handle), uintptr(unsafe.Pointer(size)), 0)
+       if r1 == 0 {
+               err = errnoErr(e1)
+       }
+       return
+}
+
 func GetFinalPathNameByHandle(file syscall.Handle, filePath *uint16, filePathSize uint32, flags uint32) (n uint32, err error) {
        r0, _, e1 := syscall.SyscallN(procGetFinalPathNameByHandleW.Addr(), uintptr(file), uintptr(unsafe.Pointer(filePath)), uintptr(filePathSize), uintptr(flags))
        n = uint32(r0)

diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go
index 8258c4ff4bc8d11581ee124bb2225eecd82876a8..cd2413d26d4a534357d4ae31d4621e159d38d75b 100644 (file)
--- a/src/os/os_windows_test.go
+++ b/src/os/os_windows_test.go
@@ -1868,10 +1868,26 @@ func TestFileOverlappedSeek(t *testing.T) {
        if n != int64(len(buf)) {
                t.Errorf("expected file pointer to be at offset %d, got %d", len(buf), n)
        }
+       if n, err = f.Seek(1, io.SeekStart); err != nil {
+               t.Fatal(err)
+       } else if n != 1 {
+               t.Errorf("expected file pointer to be at offset %d, got %d", 1, n)
+       }
+       if n, err = f.Seek(-1, io.SeekEnd); err != nil {
+               t.Fatal(err)
+       } else if n != int64(len(content)-1) {
+               t.Errorf("expected file pointer to be at offset %d, got %d", len(content)-1, n)
+       }
+       if _, err := f.Seek(-1, io.SeekStart); !errors.Is(err, windows.ERROR_NEGATIVE_SEEK) {
+               t.Errorf("expected ERROR_NEGATIVE_SEEK, got %v", err)
+       }
+       if _, err := f.Seek(0, -1); !errors.Is(err, windows.ERROR_INVALID_PARAMETER) {
+               t.Errorf("expected ERROR_INVALID_PARAMETER, got %v", err)
+       }
 }
 
-func TestFileOverlappedReadAtVolume(t *testing.T) {
-       // Test that we can use File.ReadAt with an overlapped volume handle.
+func TestFileOverlappedReadAtSeekVolume(t *testing.T) {
+       // Test that we can use File.ReadAt and File.Seek with an overlapped volume handle.
        // See https://go.dev/issues/74951.
        t.Parallel()
        name := `\\.\` + filepath.VolumeName(t.TempDir())
@@ -1888,6 +1904,9 @@ func TestFileOverlappedReadAtVolume(t *testing.T) {
        if _, err := f.ReadAt(buf[:], 0); err != nil {
                t.Fatal(err)
        }
+       if _, err := f.Seek(0, io.SeekCurrent); err != nil {
+               t.Fatal(err)
+       }
 }
 
 func TestPipe(t *testing.T) {