Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/sys/unix: Connectx is broken on darwin/amd64 #71302

Open
database64128 opened this issue Jan 17, 2025 · 4 comments
Open

x/sys/unix: Connectx is broken on darwin/amd64 #71302

database64128 opened this issue Jan 17, 2025 · 4 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Milestone

Comments

@database64128
Copy link
Contributor

database64128 commented Jan 17, 2025

Go version

go version go1.23.5 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/database64128/Library/Caches/go-build'
GOENV='/Users/database64128/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/database64128/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/database64128/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.5/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.5/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.5'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/database64128/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/xd/v25clzgj08d6_cbbp7lwszfr0000gn/T/go-build880517508=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

@wwqgtxx reports that x/sys/unix.Connectx is broken on darwin/amd64. The call succeeds, but does not return the correct number of bytes sent, and may cause memory corruption (fatal error: stack not a power of 2).

They were able to work around the issue by making the following minimal change:

diff --git a/sys/unix/zsyscall_darwin_amd64.go b/sys/unix/zsyscall_darwin_amd64.go
index 24b346e..f8cc117 100644
--- a/sys/unix/zsyscall_darwin_amd64.go
+++ b/sys/unix/zsyscall_darwin_amd64.go
@@ -848,7 +848,7 @@ func connectx(fd int, endpoints *SaEndpoints, associd SaeAssocID, flags uint32,
 	} else {
 		_p0 = unsafe.Pointer(&_zero)
 	}
-	_, _, e1 := syscall_syscall9(libc_connectx_trampoline_addr, uintptr(fd), uintptr(unsafe.Pointer(endpoints)), uintptr(associd), uintptr(flags), uintptr(_p0), uintptr(len(iov)), uintptr(unsafe.Pointer(n)), uintptr(unsafe.Pointer(connid)), 0)
+	_, _, e1 := Syscall9(SYS_CONNECTX, uintptr(fd), uintptr(unsafe.Pointer(endpoints)), uintptr(associd), uintptr(flags), uintptr(_p0), uintptr(len(iov)), uintptr(unsafe.Pointer(n)), uintptr(unsafe.Pointer(connid)), 0)
 	if e1 != 0 {
 		err = errnoErr(e1)
 	}

Connectx was added by me in golang/sys@59665e5 (CL 606155). It does not have any issues on darwin/arm64. We spent hours on this and were not able to pinpoint the exact cause.

What did you see happen?

Connectx is broken on darwin/amd64.

What did you expect to see?

Connectx works on darwin/amd64.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 17, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 17, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 17, 2025
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 17, 2025
@mknyszek
Copy link
Contributor

CC @golang/runtime

@mknyszek
Copy link
Contributor

Switching to the underlying syscall instead of libc suggests we're probably holding Darwin's libc wrong? Michael P notes maybe stack corruption from a mistake in how it's called? macOS requires us to call through libc for everything, so I'm not sure we should switch to making the underlying syscall directly.

@randall77
Copy link
Contributor

I think syscall9 is incorrect. The return values might not be plumbed back correctly. Try this patch:

diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go
index 5c769a71ea..af10c53659 100644
--- a/src/runtime/sys_darwin.go
+++ b/src/runtime/sys_darwin.go
@@ -72,10 +72,11 @@ func syscall6()
 //go:nosplit
 //go:cgo_unsafe_args
 func syscall_syscall9(fn, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2, err uintptr) {
+       args := struct{ fn, a1, a2, a3, a4, a5, a6, a7, a8, a9, r1, r2, err uintptr }{fn, a1, a2, a3, a4, a5, a6, a7, a8, a9, 0, 0, 0}
        entersyscall()
-       libcCall(unsafe.Pointer(abi.FuncPCABI0(syscall9)), unsafe.Pointer(&fn))
+       libcCall(unsafe.Pointer(abi.FuncPCABI0(syscall9)), unsafe.Pointer(&args))
        exitsyscall()
-       return
+       return args.r1, args.r2, args.err
 }
 func syscall9()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Projects
Development

No branches or pull requests

6 participants