syscall: fix a few Linux system calls

These functions claimed to return error (an interface)
and be implemented entirely in assembly, but it's not
possible to create an interface from assembly
(at least not easily).

In reality the functions were written to return an errno uintptr
despite the Go prototype saying error.
When the errno was 0, they coincidentally filled out a nil error
by writing the 0 to the type word of the interface.
If the errno was ever non-zero, the functions would
create a non-nil error that would crash when trying to
call err.Error().

Luckily these functions (Seek, Time, Gettimeofday) pretty
much never fail, so it was all kind of working.

Found by go vet.

LGTM=bradfitz, r
R=golang-codereviews, bradfitz, r
CC=golang-codereviews
https://golang.org/cl/99320043

diff --git a/src/pkg/syscall/asm_linux_386.s b/src/pkg/syscall/asm_linux_386.s
index 668fc2bee5d08bcbeeac116eeb2ef58fecd3399a..30b22073df3db35c542b4d57af4bc779c46ff598 100644 (file)
--- a/src/pkg/syscall/asm_linux_386.s
+++ b/src/pkg/syscall/asm_linux_386.s
@@ -162,7 +162,7 @@ oksock1:
 // taking the address of the return value newoffset.
 // Underlying system call is
 //     llseek(int fd, int offhi, int offlo, int64 *result, int whence)
-TEXT ·Seek(SB),NOSPLIT,$0-32
+TEXT ·seek(SB),NOSPLIT,$0-28
        CALL    runtime·entersyscall(SB)
        MOVL    $SYS__LLSEEK, AX        // syscall entry
        MOVL    4(SP), BX       // fd

diff --git a/src/pkg/syscall/asm_linux_amd64.s b/src/pkg/syscall/asm_linux_amd64.s
index d75096dec01c178fbabef2dae2a705a49a179dac..995b60ecd0bd9a23a4c064739021505c8073ecec 100644 (file)
--- a/src/pkg/syscall/asm_linux_amd64.s
+++ b/src/pkg/syscall/asm_linux_amd64.s
@@ -110,7 +110,7 @@ ok2:
        MOVQ    $0, 80(SP)      // errno
        RET
 
-TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
+TEXT ·gettimeofday(SB),NOSPLIT,$0-16
        MOVQ    8(SP), DI
        MOVQ    $0, SI
        MOVQ    runtime·__vdso_gettimeofday_sym(SB), AX
@@ -124,11 +124,3 @@ TEXT ·Gettimeofday(SB),NOSPLIT,$0-24
 ok7:
        MOVQ    $0, 16(SP)  // errno
        RET
-
-TEXT ·Time(SB),NOSPLIT,$0-32
-       MOVQ    8(SP), DI
-       MOVQ    runtime·__vdso_time_sym(SB), AX
-       CALL    AX
-       MOVQ    AX, 16(SP)  // tt
-       MOVQ    $0, 24(SP)  // errno
-       RET

diff --git a/src/pkg/syscall/asm_linux_arm.s b/src/pkg/syscall/asm_linux_arm.s
index bf54b4fe6dc8da4aaf44b1094159c3be45365a82..8499a737041f3de53fdfc66999d756f454e94c66 100644 (file)
--- a/src/pkg/syscall/asm_linux_arm.s
+++ b/src/pkg/syscall/asm_linux_arm.s
@@ -98,7 +98,7 @@ ok2:
        RET
 
 #define SYS__LLSEEK 140  /* from zsysnum_linux_arm.go */
-// func Seek(fd int, offset int64, whence int) (newoffset int64, errno int)
+// func seek(fd int, offset int64, whence int) (newoffset int64, errno int)
 // Implemented in assembly to avoid allocation when
 // taking the address of the return value newoffset.
 // Underlying system call is

diff --git a/src/pkg/syscall/syscall_linux_386.go b/src/pkg/syscall/syscall_linux_386.go
index 4487848f734c6df486f2fe492467f82964dacd18..c491a286ce8d9622460887052a870632802c3929 100644 (file)
--- a/src/pkg/syscall/syscall_linux_386.go
+++ b/src/pkg/syscall/syscall_linux_386.go
@@ -135,7 +135,15 @@ func Setrlimit(resource int, rlim *Rlimit) (err error) {
 
 // Underlying system call writes to newoffset via pointer.
 // Implemented in assembly to avoid allocation.
-func Seek(fd int, offset int64, whence int) (newoffset int64, err error)
+func seek(fd int, offset int64, whence int) (newoffset int64, err Errno)
+
+func Seek(fd int, offset int64, whence int) (newoffset int64, err error) {
+       newoffset, errno := seek(fd, offset, whence)
+       if errno != 0 {
+               return 0, errno
+       }
+       return newoffset, nil
+}
 
 // Vsyscalls on amd64.
 //sysnb        Gettimeofday(tv *Timeval) (err error)

diff --git a/src/pkg/syscall/syscall_linux_amd64.go b/src/pkg/syscall/syscall_linux_amd64.go
index 9e8663d014a510298583e9532ec9bce0d474f43a..1e330dd99cc76fa0c93feba0fcffea07f1ae04e9 100644 (file)
--- a/src/pkg/syscall/syscall_linux_amd64.go
+++ b/src/pkg/syscall/syscall_linux_amd64.go
@@ -58,8 +58,28 @@ package syscall
 
 func Getpagesize() int { return 4096 }
 
-func Gettimeofday(tv *Timeval) (err error)
-func Time(t *Time_t) (tt Time_t, err error)
+//go:noescape
+func gettimeofday(tv *Timeval) (err Errno)
+
+func Gettimeofday(tv *Timeval) (err error) {
+       errno := gettimeofday(tv)
+       if errno != 0 {
+               return errno
+       }
+       return nil
+}
+
+func Time(t *Time_t) (tt Time_t, err error) {
+       var tv Timeval
+       errno := gettimeofday(&tv)
+       if errno != 0 {
+               return errno
+       }
+       if t != nil {
+               *t = tv.Sec
+       }
+       return Time_t(tv.Sec), nil
+}
 
 func TimespecToNsec(ts Timespec) int64 { return int64(ts.Sec)*1e9 + int64(ts.Nsec) }
 

diff --git a/src/pkg/syscall/syscall_linux_arm.go b/src/pkg/syscall/syscall_linux_arm.go
index 68c796b448ec8c7d12fd8b104d8651266d38faa8..9fe80232a8d02da6d79795b6c6ab64e28371c5c6 100644 (file)
--- a/src/pkg/syscall/syscall_linux_arm.go
+++ b/src/pkg/syscall/syscall_linux_arm.go
@@ -23,9 +23,17 @@ func NsecToTimeval(nsec int64) (tv Timeval) {
        return
 }
 
-// Seek is defined in assembly.
-
-func Seek(fd int, offset int64, whence int) (newoffset int64, err error)
+// Underlying system call writes to newoffset via pointer.
+// Implemented in assembly to avoid allocation.
+func seek(fd int, offset int64, whence int) (newoffset int64, err Errno)
+
+func Seek(fd int, offset int64, whence int) (newoffset int64, err error) {
+       newoffset, errno := seek(fd, offset, whence)
+       if errno != 0 {
+               return 0, errno
+       }
+       return newoffset, nil
+}
 
 //sys  accept(s int, rsa *RawSockaddrAny, addrlen *_Socklen) (fd int, err error)
 //sys  accept4(s int, rsa *RawSockaddrAny, addrlen *_Socklen, flags int) (fd int, err error)

diff --git a/src/pkg/syscall/syscall_unix_test.go b/src/pkg/syscall/syscall_unix_test.go
index 32d7b9f2e4831418f1a3ed6a6cd6288a5c29ce35..a0afb91fcfc8363a6384493ff994cedf233efcc4 100644 (file)
--- a/src/pkg/syscall/syscall_unix_test.go
+++ b/src/pkg/syscall/syscall_unix_test.go
@@ -300,3 +300,15 @@ func TestRlimit(t *testing.T) {
                t.Fatalf("Setrlimit: restore failed: %#v %v", rlimit, err)
        }
 }
+
+func TestSeekFailure(t *testing.T) {
+       _, err := syscall.Seek(-1, 0, 0)
+       if err == nil {
+               t.Fatalf("Seek(-1, 0, 0) did not fail")
+       }
+       str := err.Error() // used to crash on Linux
+       t.Logf("Seek: %v", str)
+       if str == "" {
+               t.Fatalf("Seek(-1, 0, 0) return error with empty message")
+       }
+}