runtime: change read and write to return negative errno value

The internal read and write functions used to return -1 on error; change them to return a negative errno value instead. This will be used by later CLs in this series. For most targets this is a simplification, although for ones that call into libc it is a complication. Updates #27707 Change-Id: Id02bf9487f03e7e88e4f2b85e899e986738697ad Reviewed-on: https://go-review.googlesource.com/c/go/+/171823 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
golang · Oct 21, 2019 · b653c87 · b653c87
1 parent 6917b3c
commit b653c87
Show file tree

Hide file tree

Showing 30 changed files with 141 additions and 93 deletions.
diff --git a/src/runtime/crash_unix_test.go b/src/runtime/crash_unix_test.go
@@ -18,6 +18,7 @@ import (
 	"strings"
 	"syscall"
 	"testing"
+	"unsafe"
 )
 
 // sigquit is the signal to send to kill a hanging testdata program.
@@ -33,6 +34,29 @@ func init() {
 	}
 }
 
+func TestBadOpen(t *testing.T) {
+	// make sure we get the correct error code if open fails. Same for
+	// read/write/close on the resulting -1 fd. See issue 10052.
+	nonfile := []byte("/notreallyafile")
+	fd := runtime.Open(&nonfile[0], 0, 0)
+	if fd != -1 {
+		t.Errorf("open(%q)=%d, want -1", nonfile, fd)
+	}
+	var buf [32]byte
+	r := runtime.Read(-1, unsafe.Pointer(&buf[0]), int32(len(buf)))
+	if got, want := r, -int32(syscall.EBADF); got != want {
+		t.Errorf("read()=%d, want %d", got, want)
+	}
+	w := runtime.Write(^uintptr(0), unsafe.Pointer(&buf[0]), int32(len(buf)))
+	if got, want := w, -int32(syscall.EBADF); got != want {
+		t.Errorf("write()=%d, want %d", got, want)
+	}
+	c := runtime.Close(-1)
+	if c != -1 {
+		t.Errorf("close()=%d, want -1", c)
+	}
+}
+
 func TestCrashDumpsAllThreads(t *testing.T) {
 	if *flagQuick {
 		t.Skip("-quick")

diff --git a/src/runtime/os2_aix.go b/src/runtime/os2_aix.go
@@ -399,16 +399,23 @@ func write1(fd uintptr, p unsafe.Pointer, n int32) int32 {
 	// Check the validity of g because without a g during
 	// newosproc0.
 	if _g_ != nil {
-		r, _ := syscall3(&libc_write, uintptr(fd), uintptr(p), uintptr(n))
+		r, errno := syscall3(&libc_write, uintptr(fd), uintptr(p), uintptr(n))
+		if int32(r) < 0 {
+			return -int32(errno)
+		}
 		return int32(r)
 	}
+	// Note that in this case we can't return a valid errno value.
 	return write2(fd, uintptr(p), n)
 
 }
 
 //go:nosplit
 func read(fd int32, p unsafe.Pointer, n int32) int32 {
-	r, _ := syscall3(&libc_read, uintptr(fd), uintptr(p), uintptr(n))
+	r, errno := syscall3(&libc_read, uintptr(fd), uintptr(p), uintptr(n))
+	if int32(r) < 0 {
+		return -int32(errno)
+	}
 	return int32(r)
 }
 

diff --git a/src/runtime/os3_solaris.go b/src/runtime/os3_solaris.go
@@ -447,7 +447,11 @@ func raiseproc(sig uint32) /* int32 */ {
 
 //go:nosplit
 func read(fd int32, buf unsafe.Pointer, nbyte int32) int32 {
-	return int32(sysvicall3(&libc_read, uintptr(fd), uintptr(buf), uintptr(nbyte)))
+	r1, err := sysvicall3Err(&libc_read, uintptr(fd), uintptr(buf), uintptr(nbyte))
+	if c := int32(r1); c >= 0 {
+		return c
+	}
+	return -int32(err)
 }
 
 //go:nosplit
@@ -511,7 +515,11 @@ func walltime1() (sec int64, nsec int32) {
 
 //go:nosplit
 func write1(fd uintptr, buf unsafe.Pointer, nbyte int32) int32 {
-	return int32(sysvicall3(&libc_write, uintptr(fd), uintptr(buf), uintptr(nbyte)))
+	r1, err := sysvicall3Err(&libc_write, fd, uintptr(buf), uintptr(nbyte))
+	if c := int32(r1); c >= 0 {
+		return c
+	}
+	return -int32(err)
 }
 
 func osyield1()

diff --git a/src/runtime/os_solaris.go b/src/runtime/os_solaris.go
@@ -122,6 +122,16 @@ func sysvicall2(fn *libcFunc, a1, a2 uintptr) uintptr {
 
 //go:nosplit
 func sysvicall3(fn *libcFunc, a1, a2, a3 uintptr) uintptr {
+	r1, _ := sysvicall3Err(fn, a1, a2, a3)
+	return r1
+}
+
+//go:nosplit
+//go:cgo_unsafe_args
+
+// sysvicall3Err returns both the system call result and the errno value.
+// This is used by sysicall3 and write1.
+func sysvicall3Err(fn *libcFunc, a1, a2, a3 uintptr) (r1, err uintptr) {
 	// Leave caller's PC/SP around for traceback.
 	gp := getg()
 	var mp *m
@@ -146,7 +156,7 @@ func sysvicall3(fn *libcFunc, a1, a2, a3 uintptr) uintptr {
 	if mp != nil {
 		mp.libcallsp = 0
 	}
-	return libcall.r1
+	return libcall.r1, libcall.err
 }
 
 //go:nosplit

diff --git a/src/runtime/runtime_test.go b/src/runtime/runtime_test.go
@@ -290,32 +290,6 @@ func TestTrailingZero(t *testing.T) {
 	}
 }
 
-func TestBadOpen(t *testing.T) {
-	if GOOS == "windows" || GOOS == "js" {
-		t.Skip("skipping OS that doesn't have open/read/write/close")
-	}
-	// make sure we get the correct error code if open fails. Same for
-	// read/write/close on the resulting -1 fd. See issue 10052.
-	nonfile := []byte("/notreallyafile")
-	fd := Open(&nonfile[0], 0, 0)
-	if fd != -1 {
-		t.Errorf("open(\"%s\")=%d, want -1", string(nonfile), fd)
-	}
-	var buf [32]byte
-	r := Read(-1, unsafe.Pointer(&buf[0]), int32(len(buf)))
-	if r != -1 {
-		t.Errorf("read()=%d, want -1", r)
-	}
-	w := Write(^uintptr(0), unsafe.Pointer(&buf[0]), int32(len(buf)))
-	if w != -1 {
-		t.Errorf("write()=%d, want -1", w)
-	}
-	c := Close(-1)
-	if c != -1 {
-		t.Errorf("close()=%d, want -1", c)
-	}
-}
-
 func TestAppendGrowth(t *testing.T) {
 	var x []int64
 	check := func(want int) {

diff --git a/src/runtime/stubs2.go b/src/runtime/stubs2.go
@@ -13,12 +13,17 @@ package runtime
 
 import "unsafe"
 
+// read calls the read system call.
+// It returns a non-negative number of bytes written or a negative errno value.
 func read(fd int32, p unsafe.Pointer, n int32) int32
+
 func closefd(fd int32) int32
 
 func exit(code int32)
 func usleep(usec uint32)
 
+// write calls the write system call.
+// It returns a non-negative number of bytes written or a negative errno value.
 //go:noescape
 func write1(fd uintptr, p unsafe.Pointer, n int32) int32
 

diff --git a/src/runtime/sys_darwin_386.s b/src/runtime/sys_darwin_386.s
@@ -64,6 +64,12 @@ TEXT runtime·read_trampoline(SB),NOSPLIT,$0
 	MOVL	8(CX), AX		// arg 3 count
 	MOVL	AX, 8(SP)
 	CALL	libc_read(SB)
+	TESTL	AX, AX
+	JGE	noerr
+	CALL	libc_error(SB)
+	MOVL	(AX), AX
+	NEGL	AX			// caller expects negative errno value
+noerr:
 	MOVL	BP, SP
 	POPL	BP
 	RET
@@ -80,6 +86,12 @@ TEXT runtime·write_trampoline(SB),NOSPLIT,$0
 	MOVL	8(CX), AX		// arg 3 count
 	MOVL	AX, 8(SP)
 	CALL	libc_write(SB)
+	TESTL	AX, AX
+	JGE	noerr
+	CALL	libc_error(SB)
+	MOVL	(AX), AX
+	NEGL	AX			// caller expects negative errno value
+noerr:
 	MOVL	BP, SP
 	POPL	BP
 	RET

diff --git a/src/runtime/sys_darwin_amd64.s b/src/runtime/sys_darwin_amd64.s
@@ -46,6 +46,12 @@ TEXT runtime·read_trampoline(SB),NOSPLIT,$0
 	MOVL	16(DI), DX		// arg 3 count
 	MOVL	0(DI), DI		// arg 1 fd
 	CALL	libc_read(SB)
+	TESTL	AX, AX
+	JGE	noerr
+	CALL	libc_error(SB)
+	MOVL	(AX), AX
+	NEGL	AX			// caller expects negative errno value
+noerr:
 	POPQ	BP
 	RET
 
@@ -56,6 +62,12 @@ TEXT runtime·write_trampoline(SB),NOSPLIT,$0
 	MOVL	16(DI), DX		// arg 3 count
 	MOVQ	0(DI), DI		// arg 1 fd
 	CALL	libc_write(SB)
+	TESTL	AX, AX
+	JGE	noerr
+	CALL	libc_error(SB)
+	MOVL	(AX), AX
+	NEGL	AX			// caller expects negative errno value
+noerr:
 	POPQ	BP
 	RET
 

diff --git a/src/runtime/sys_darwin_arm.s b/src/runtime/sys_darwin_arm.s
@@ -32,13 +32,27 @@ TEXT runtime·write_trampoline(SB),NOSPLIT,$0
 	MOVW	8(R0), R2	// arg 3 count
 	MOVW	0(R0), R0	// arg 1 fd
 	BL	libc_write(SB)
+	MOVW	$-1, R1
+	CMP	R0, R1
+	BNE	noerr
+	BL	libc_error(SB)
+	MOVW	(R0), R0
+	RSB	$0, R0, R0	// caller expects negative errno value
+noerr:
 	RET
 
 TEXT runtime·read_trampoline(SB),NOSPLIT,$0
 	MOVW	4(R0), R1	// arg 2 buf
 	MOVW	8(R0), R2	// arg 3 count
 	MOVW	0(R0), R0	// arg 1 fd
 	BL	libc_read(SB)
+	MOVW	$-1, R1
+	CMP	R0, R1
+	BNE	noerr
+	BL	libc_error(SB)
+	MOVW	(R0), R0
+	RSB	$0, R0, R0	// caller expects negative errno value
+noerr:
 	RET
 
 TEXT runtime·pipe_trampoline(SB),NOSPLIT,$0

diff --git a/src/runtime/sys_darwin_arm64.s b/src/runtime/sys_darwin_arm64.s
@@ -35,13 +35,27 @@ TEXT runtime·write_trampoline(SB),NOSPLIT,$0
 	MOVW	16(R0), R2	// arg 3 count
 	MOVW	0(R0), R0	// arg 1 fd
 	BL	libc_write(SB)
+	MOVD	$-1, R1
+	CMP	R0, R1
+	BNE	noerr
+	BL	libc_error(SB)
+	MOVW	(R0), R0
+	NEG	R0, R0		// caller expects negative errno value
+noerr:
 	RET
 
 TEXT runtime·read_trampoline(SB),NOSPLIT,$0
 	MOVD	8(R0), R1	// arg 2 buf
 	MOVW	16(R0), R2	// arg 3 count
 	MOVW	0(R0), R0	// arg 1 fd
 	BL	libc_read(SB)
+	MOVD	$-1, R1
+	CMP	R0, R1
+	BNE	noerr
+	BL	libc_error(SB)
+	MOVW	(R0), R0
+	NEG	R0, R0		// caller expects negative errno value
+noerr:
 	RET
 
 TEXT runtime·pipe_trampoline(SB),NOSPLIT,$0

diff --git a/src/runtime/sys_dragonfly_amd64.s b/src/runtime/sys_dragonfly_amd64.s
@@ -104,7 +104,7 @@ TEXT runtime·read(SB),NOSPLIT,$-8
 	MOVL	$3, AX
 	SYSCALL
 	JCC	2(PC)
-	MOVL	$-1, AX
+	NEGL	AX			// caller expects negative errno
 	MOVL	AX, ret+24(FP)
 	RET
 
@@ -130,7 +130,7 @@ TEXT runtime·write1(SB),NOSPLIT,$-8
 	MOVL	$4, AX
 	SYSCALL
 	JCC	2(PC)
-	MOVL	$-1, AX
+	NEGL	AX			// caller expects negative errno
 	MOVL	AX, ret+24(FP)
 	RET
 

diff --git a/src/runtime/sys_freebsd_386.s b/src/runtime/sys_freebsd_386.s
@@ -93,7 +93,7 @@ TEXT runtime·read(SB),NOSPLIT,$-4
 	MOVL	$3, AX
 	INT	$0x80
 	JAE	2(PC)
-	MOVL	$-1, AX
+	NEGL	AX			// caller expects negative errno
 	MOVL	AX, ret+12(FP)
 	RET
 
@@ -127,7 +127,7 @@ TEXT runtime·write1(SB),NOSPLIT,$-4
 	MOVL	$4, AX
 	INT	$0x80
 	JAE	2(PC)
-	MOVL	$-1, AX
+	NEGL	AX			// caller expects negative errno
 	MOVL	AX, ret+12(FP)
 	RET
 

diff --git a/src/runtime/sys_freebsd_amd64.s b/src/runtime/sys_freebsd_amd64.s
@@ -93,7 +93,7 @@ TEXT runtime·read(SB),NOSPLIT,$-8
 	MOVL	$3, AX
 	SYSCALL
 	JCC	2(PC)
-	MOVL	$-1, AX
+	NEGQ	AX			// caller expects negative errno
 	MOVL	AX, ret+24(FP)
 	RET
 
@@ -128,7 +128,7 @@ TEXT runtime·write1(SB),NOSPLIT,$-8
 	MOVL	$4, AX
 	SYSCALL
 	JCC	2(PC)
-	MOVL	$-1, AX
+	NEGQ	AX			// caller expects negative errno
 	MOVL	AX, ret+24(FP)
 	RET
 

diff --git a/src/runtime/sys_freebsd_arm.s b/src/runtime/sys_freebsd_arm.s
@@ -117,7 +117,7 @@ TEXT runtime·read(SB),NOSPLIT|NOFRAME,$0
 	MOVW n+8(FP), R2	// arg 3 count
 	MOVW $SYS_read, R7
 	SWI $0
-	MOVW.CS	$-1, R0
+	SUB.CS	$0, R0, R0	// caller expects negative errno
 	MOVW	R0, ret+12(FP)
 	RET
 
@@ -153,7 +153,7 @@ TEXT runtime·write1(SB),NOSPLIT|NOFRAME,$0
 	MOVW n+8(FP), R2	// arg 3 count
 	MOVW $SYS_write, R7
 	SWI $0
-	MOVW.CS	$-1, R0
+	SUB.CS	$0, R0, R0	// caller expects negative errno
 	MOVW	R0, ret+12(FP)
 	RET
 

diff --git a/src/runtime/sys_linux_386.s b/src/runtime/sys_linux_386.s
@@ -115,9 +115,6 @@ TEXT runtime·write1(SB),NOSPLIT,$0
 	MOVL	p+4(FP), CX
 	MOVL	n+8(FP), DX
 	INVOKE_SYSCALL
-	CMPL	AX, $0xfffff001
-	JLS	2(PC)
-	MOVL	$-1, AX
 	MOVL	AX, ret+12(FP)
 	RET
 
@@ -127,9 +124,6 @@ TEXT runtime·read(SB),NOSPLIT,$0
 	MOVL	p+4(FP), CX
 	MOVL	n+8(FP), DX
 	INVOKE_SYSCALL
-	CMPL	AX, $0xfffff001
-	JLS	2(PC)
-	MOVL	$-1, AX
 	MOVL	AX, ret+12(FP)
 	RET
 

diff --git a/src/runtime/sys_linux_amd64.s b/src/runtime/sys_linux_amd64.s
@@ -97,9 +97,6 @@ TEXT runtime·write1(SB),NOSPLIT,$0-28
 	MOVL	n+16(FP), DX
 	MOVL	$SYS_write, AX
 	SYSCALL
-	CMPQ	AX, $0xfffffffffffff001
-	JLS	2(PC)
-	MOVL	$-1, AX
 	MOVL	AX, ret+24(FP)
 	RET
 
@@ -109,9 +106,6 @@ TEXT runtime·read(SB),NOSPLIT,$0-28
 	MOVL	n+16(FP), DX
 	MOVL	$SYS_read, AX
 	SYSCALL
-	CMPQ	AX, $0xfffffffffffff001
-	JLS	2(PC)
-	MOVL	$-1, AX
 	MOVL	AX, ret+24(FP)
 	RET