darwin: replace custom syscall package with Go native syscall package

aykevl · deadprogram · commit 105fe9b25df6 · 2024-09-04T20:04:25.000+02:00
This required a few compiler and runtime tricks to work, but I ran a
bunch of tests and it seems fine. (CI will of course do more exhaustive
testing).

The main benefit here is that we don't need to maintain the darwin
version of the syscall package, and reduce extra risks for bugs (because
we reuse the well-tested syscall package). For example, Go 1.23 needed a
bunch of new constants in the syscall package. That would have been
avoided if we had used the native syscall package on MacOS.
diff --git a/compileopts/target.go b/compileopts/target.go
@@ -389,6 +389,7 @@ func defaultTarget(options *Options) (*TargetSpec, error) {
 			"-platform_version", "macos", platformVersion, platformVersion,
 		)
 		spec.ExtraFiles = append(spec.ExtraFiles,
+			"src/runtime/os_darwin.c",
 			"src/runtime/runtime_unix.c")
 	case "linux":
 		spec.Linker = "ld.lld"
diff --git a/compiler/compiler.go b/compiler/compiler.go
@@ -1847,7 +1847,9 @@ func (b *builder) createFunctionCall(instr *ssa.CallCommon) (llvm.Value, error)
 		case strings.HasPrefix(name, "(device/riscv.CSR)."):
 			return b.emitCSROperation(instr)
 		case strings.HasPrefix(name, "syscall.Syscall") || strings.HasPrefix(name, "syscall.RawSyscall") || strings.HasPrefix(name, "golang.org/x/sys/unix.Syscall") || strings.HasPrefix(name, "golang.org/x/sys/unix.RawSyscall"):
-			return b.createSyscall(instr)
+			if b.GOOS != "darwin" {
+				return b.createSyscall(instr)
+			}
 		case strings.HasPrefix(name, "syscall.rawSyscallNoError") || strings.HasPrefix(name, "golang.org/x/sys/unix.RawSyscallNoError"):
 			return b.createRawSyscallNoError(instr)
 		case name == "runtime.supportsRecover":
@@ -1865,6 +1867,11 @@ func (b *builder) createFunctionCall(instr *ssa.CallCommon) (llvm.Value, error)
 			return llvm.ConstInt(b.ctx.Int8Type(), panicStrategy, false), nil
 		case name == "runtime/interrupt.New":
 			return b.createInterruptGlobal(instr)
+		case name == "internal/abi.FuncPCABI0":
+			retval := b.createDarwinFuncPCABI0Call(instr)
+			if !retval.IsNil() {
+				return retval, nil
+			}
 		}
 
 		calleeType, callee = b.getFunction(fn)
diff --git a/compiler/syscall.go b/compiler/syscall.go
@@ -5,6 +5,7 @@ package compiler
 
 import (
 	"strconv"
+	"strings"
 
 	"golang.org/x/tools/go/ssa"
 	"tinygo.org/x/go-llvm"
@@ -329,3 +330,64 @@ func (b *builder) createRawSyscallNoError(call *ssa.CallCommon) (llvm.Value, err
 	retval = b.CreateInsertValue(retval, llvm.ConstInt(b.uintptrType, 0, false), 1, "")
 	return retval, nil
 }
+
+// Lower a call to internal/abi.FuncPCABI0 on MacOS.
+// This function is called like this:
+//
+//	syscall(abi.FuncPCABI0(libc_mkdir_trampoline), uintptr(unsafe.Pointer(_p0)), uintptr(mode), 0)
+//
+// So we'll want to return a function pointer (as uintptr) that points to the
+// libc function. Specifically, we _don't_ want to point to the trampoline
+// function (which is implemented in Go assembly which we can't read), but
+// rather to the actually intended function. For this we're going to assume that
+// all the functions follow a specific pattern: libc_<functionname>_trampoline.
+//
+// The return value is the function pointer as an uintptr, or a nil value if
+// this isn't possible (and a regular call should be made as fallback).
+func (b *builder) createDarwinFuncPCABI0Call(instr *ssa.CallCommon) llvm.Value {
+	if b.GOOS != "darwin" {
+		// This has only been tested on MacOS (and only seems to be used there).
+		return llvm.Value{}
+	}
+
+	// Check that it uses a function call like syscall.libc_*_trampoline
+	itf := instr.Args[0].(*ssa.MakeInterface)
+	calledFn := itf.X.(*ssa.Function)
+	if pkgName := calledFn.Pkg.Pkg.Path(); pkgName != "syscall" && pkgName != "internal/syscall/unix" {
+		return llvm.Value{}
+	}
+	if !strings.HasPrefix(calledFn.Name(), "libc_") || !strings.HasSuffix(calledFn.Name(), "_trampoline") {
+
+		return llvm.Value{}
+	}
+
+	// Extract the libc function name.
+	name := strings.TrimPrefix(strings.TrimSuffix(calledFn.Name(), "_trampoline"), "libc_")
+	if name == "open" {
+		// Special case: open() is a variadic function and can't be called like
+		// a regular function. Therefore, we need to use a wrapper implemented
+		// in C.
+		name = "syscall_libc_open"
+	}
+	if b.GOARCH == "amd64" {
+		if name == "fdopendir" || name == "readdir_r" {
+			// Hack to support amd64, which needs the $INODE64 suffix.
+			// This is also done in upstream Go:
+			// https://github.com/golang/go/commit/096ab3c21b88ccc7d411379d09fe6274e3159467
+			name += "$INODE64"
+		}
+	}
+
+	// Obtain the C function.
+	// Use a simple function (no parameters or return value) because all we need
+	// is the address of the function.
+	llvmFn := b.mod.NamedFunction(name)
+	if llvmFn.IsNil() {
+		llvmFnType := llvm.FunctionType(b.ctx.VoidType(), nil, false)
+		llvmFn = llvm.AddFunction(b.mod, name, llvmFnType)
+	}
+
+	// Cast the function pointer to a uintptr (because that's what
+	// abi.FuncPCABI0 returns).
+	return b.CreatePtrToInt(llvmFn, b.uintptrType, "")
+}
diff --git a/lib/macos-minimal-sdk b/lib/macos-minimal-sdk
@@ -1 +1 @@
-Subproject commit 91ac2eabd80f10d95cb4255c78999d9d2c45a3be
+Subproject commit 4e4113e3b1244b8fdc5e1486577f25e22d63f36e
diff --git a/loader/goroot.go b/loader/goroot.go
@@ -218,7 +218,7 @@ func listGorootMergeLinks(goroot, tinygoroot string, overrides map[string]bool)
 // with the TinyGo version. This is the case on some targets.
 func needsSyscallPackage(buildTags []string) bool {
 	for _, tag := range buildTags {
-		if tag == "baremetal" || tag == "darwin" || tag == "nintendoswitch" || tag == "tinygo.wasm" {
+		if tag == "baremetal" || tag == "nintendoswitch" || tag == "tinygo.wasm" {
 			return true
 		}
 	}
diff --git a/src/internal/abi/funcpc.go b/src/internal/abi/funcpc.go
@@ -4,11 +4,9 @@ package abi
 // (in particular internal/syscall/unix on MacOS). They do not currently have an
 // implementation, in part because TinyGo doesn't use ABI0 or ABIInternal (it
 // uses a C-like calling convention).
+// Calls to FuncPCABI0 however are treated specially by the compiler when
+// compiling for MacOS.
 
-func FuncPCABI0(f interface{}) uintptr {
-	panic("unimplemented: internal/abi.FuncPCABI0")
-}
+func FuncPCABI0(f interface{}) uintptr
 
-func FuncPCABIInternal(f interface{}) uintptr {
-	panic("unimplemented: internal/abi.FuncPCABIInternal")
-}
+func FuncPCABIInternal(f interface{}) uintptr
diff --git a/src/os/dir_darwin.go b/src/os/dir_darwin.go
@@ -135,7 +135,7 @@ func darwinOpenDir(fd syscallFd) (uintptr, string, error) {
 	}
 	var dir uintptr
 	for {
-		dir, err = syscall.Fdopendir(fd2)
+		dir, err = fdopendir(fd2)
 		if err != syscall.EINTR {
 			break
 		}
@@ -149,6 +149,9 @@ func darwinOpenDir(fd syscallFd) (uintptr, string, error) {
 
 // Implemented in syscall/syscall_libc_darwin_*.go.
 
+//go:linkname fdopendir syscall.fdopendir
+func fdopendir(fd int) (dir uintptr, err error)
+
 //go:linkname closedir syscall.closedir
 func closedir(dir uintptr) (err error)
 
diff --git a/src/os/file_darwin.go b/src/os/file_darwin.go
@@ -0,0 +1,7 @@
+package os
+
+import "syscall"
+
+func pipe(p []int) error {
+	return syscall.Pipe(p)
+}
diff --git a/src/os/file_notdarwin.go b/src/os/file_notdarwin.go
@@ -0,0 +1,9 @@
+//go:build (linux && !baremetal && !wasm_unknown) || wasip1 || wasip2
+
+package os
+
+import "syscall"
+
+func pipe(p []int) error {
+	return syscall.Pipe2(p, syscall.O_CLOEXEC)
+}
diff --git a/src/os/file_unix.go b/src/os/file_unix.go
@@ -70,7 +70,7 @@ func Truncate(name string, size int64) error {
 
 func Pipe() (r *File, w *File, err error) {
 	var p [2]int
-	err = handleSyscallError(syscall.Pipe2(p[:], syscall.O_CLOEXEC))
+	err = handleSyscallError(pipe(p[:]))
 	if err != nil {
 		return
 	}
diff --git a/src/runtime/os_darwin.c b/src/runtime/os_darwin.c
@@ -1,7 +1,32 @@
-// Wrapper function because 'open' is a variadic function and variadic functions
-// use a different (incompatible) calling convention on darwin/arm64.
+//go:build none
+
+// This file is included in the build, despite the //go:build line above.
 
 #include <fcntl.h>
+
+// Wrapper function because 'open' is a variadic function and variadic functions
+// use a different (incompatible) calling convention on darwin/arm64.
+// This function is referenced from the compiler, when it sees a
+// syscall.libc_open_trampoline function.
 int syscall_libc_open(const char *pathname, int flags, mode_t mode) {
     return open(pathname, flags, mode);
 }
+
+// The following functions are called by the runtime because Go can't call
+// function pointers directly.
+
+int tinygo_syscall(int (*fn)(uintptr_t a1, uintptr_t a2, uintptr_t a3), uintptr_t a1, uintptr_t a2, uintptr_t a3) {
+    return fn(a1, a2, a3);
+}
+
+uintptr_t tinygo_syscallX(uintptr_t (*fn)(uintptr_t a1, uintptr_t a2, uintptr_t a3), uintptr_t a1, uintptr_t a2, uintptr_t a3) {
+    return fn(a1, a2, a3);
+}
+
+int tinygo_syscall6(int (*fn)(uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6), uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6) {
+    return fn(a1, a2, a3, a4, a5, a6);
+}
+
+uintptr_t tinygo_syscall6X(uintptr_t (*fn)(uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6), uintptr_t a1, uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6) {
+    return fn(a1, a2, a3, a4, a5, a6);
+}
diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go
@@ -4,8 +4,6 @@ package runtime
 
 import "unsafe"
 
-import "C" // dummy import so that os_darwin.c works
-
 const GOOS = "darwin"
 
 const (
@@ -127,7 +125,107 @@ func hardwareRand() (n uint64, ok bool) {
 	return n, true
 }
 
+//go:linkname syscall_Getpagesize syscall.Getpagesize
+func syscall_Getpagesize() int {
+	return int(libc_getpagesize())
+}
+
+// Call "system calls" (actually: libc functions) in a special way.
+//   - Most calls calls return a C int (which is 32-bits), and -1 on failure.
+//   - syscallX* is for functions that return a 64-bit integer (and also return
+//     -1 on failure).
+//   - syscallPtr is for functions that return a pointer on success or NULL on
+//     failure.
+//   - rawSyscall seems to avoid some stack modifications, which isn't relevant
+//     to TinyGo.
+
+//go:linkname syscall_syscall syscall.syscall
+func syscall_syscall(fn, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
+	// For TinyGo we don't need to do anything special to call C functions.
+	return syscall_rawSyscall(fn, a1, a2, a3)
+}
+
+//go:linkname syscall_rawSyscall syscall.rawSyscall
+func syscall_rawSyscall(fn, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
+	result := call_syscall(fn, a1, a2, a3)
+	r1 = uintptr(result)
+	if result == -1 {
+		// Syscall returns -1 on failure.
+		err = uintptr(*libc___error())
+	}
+	return
+}
+
+//go:linkname syscall_syscallX syscall.syscallX
+func syscall_syscallX(fn, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
+	r1 = call_syscallX(fn, a1, a2, a3)
+	if int64(r1) == -1 {
+		// Syscall returns -1 on failure.
+		err = uintptr(*libc___error())
+	}
+	return
+}
+
+//go:linkname syscall_syscallPtr syscall.syscallPtr
+func syscall_syscallPtr(fn, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
+	r1 = call_syscallX(fn, a1, a2, a3)
+	if r1 == 0 {
+		// Syscall returns a pointer on success, or NULL on failure.
+		err = uintptr(*libc___error())
+	}
+	return
+}
+
+//go:linkname syscall_syscall6 syscall.syscall6
+func syscall_syscall6(fn, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err uintptr) {
+	result := call_syscall6(fn, a1, a2, a3, a4, a5, a6)
+	r1 = uintptr(result)
+	if result == -1 {
+		// Syscall returns -1 on failure.
+		err = uintptr(*libc___error())
+	}
+	return
+}
+
+//go:linkname syscall_syscall6X syscall.syscall6X
+func syscall_syscall6X(fn, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err uintptr) {
+	r1 = call_syscall6X(fn, a1, a2, a3, a4, a5, a6)
+	if int64(r1) == -1 {
+		// Syscall returns -1 on failure.
+		err = uintptr(*libc___error())
+	}
+	return
+}
+
 // uint32_t arc4random(void);
 //
 //export arc4random
 func libc_arc4random() uint32
+
+// int getpagesize(void);
+//
+//export getpagesize
+func libc_getpagesize() int32
+
+// This function returns the error location in the darwin ABI.
+// Discovered by compiling the following code using Clang:
+//
+//	#include <errno.h>
+//	int getErrno() {
+//	    return errno;
+//	}
+//
+//export __error
+func libc___error() *int32
+
+//export tinygo_syscall
+func call_syscall(fn, a1, a2, a3 uintptr) int32
+
+//export tinygo_syscallX
+func call_syscallX(fn, a1, a2, a3 uintptr) uintptr
+
+//export tinygo_syscall6
+func call_syscall6(fn, a1, a2, a3, a4, a5, a6 uintptr) int32
+
+//export tinygo_syscall6X
+func call_syscall6X(fn, a1, a2, a3, a4, a5, a6 uintptr) uintptr
diff --git a/src/syscall/env_libc.go b/src/syscall/env_libc.go
@@ -1,4 +1,4 @@
-//go:build darwin || nintendoswitch || wasip1
+//go:build nintendoswitch || wasip1
 
 package syscall
 
diff --git a/src/syscall/errno_other.go b/src/syscall/errno_other.go
@@ -1,4 +1,4 @@
-//go:build !wasip1 && !wasip2 && !darwin
+//go:build !wasip1 && !wasip2
 
 package syscall
 
diff --git a/src/syscall/mmap_unix_test.go b/src/syscall/mmap_unix_test.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build darwin || linux
+//go:build linux
 
 package syscall_test
 
diff --git a/src/syscall/syscall_libc.go b/src/syscall/syscall_libc.go
@@ -1,4 +1,4 @@
-//go:build darwin || nintendoswitch || wasip1 || wasip2
+//go:build nintendoswitch || wasip1 || wasip2
 
 package syscall
 
diff --git a/src/syscall/syscall_libc_darwin.go b/src/syscall/syscall_libc_darwin.go
diff --git a/src/syscall/syscall_libc_darwin_amd64.go b/src/syscall/syscall_libc_darwin_amd64.go
diff --git a/src/syscall/syscall_libc_darwin_arm64.go b/src/syscall/syscall_libc_darwin_arm64.go

Original file line number	Diff line number	Diff line change
`@@ -389,6 +389,7 @@ func defaultTarget(options Options) (TargetSpec, error) {`
`389`	`389`	`"-platform_version", "macos", platformVersion, platformVersion,`
`390`	`390`	`)`
`391`	`391`	`spec.ExtraFiles = append(spec.ExtraFiles,`
	`392`	`+ "src/runtime/os_darwin.c",`
`392`	`393`	`"src/runtime/runtime_unix.c")`
`393`	`394`	`case "linux":`
`394`	`395`	`spec.Linker = "ld.lld"`
Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ func listGorootMergeLinks(goroot, tinygoroot string, overrides map[string]bool)`
`218`	`218`	`// with the TinyGo version. This is the case on some targets.`
`219`	`219`	`func needsSyscallPackage(buildTags []string) bool {`
`220`	`220`	`for _, tag := range buildTags {`
`221`		`- if tag == "baremetal" \|\| tag == "darwin" \|\| tag == "nintendoswitch" \|\| tag == "tinygo.wasm" {`
	`221`	`+ if tag == "baremetal" \|\| tag == "nintendoswitch" \|\| tag == "tinygo.wasm" {`
`222`	`222`	`return true`
`223`	`223`	`}`
`224`	`224`	`}`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ func Truncate(name string, size int64) error {`
`70`	`70`
`71`	`71`	`func Pipe() (r File, w File, err error) {`
`72`	`72`	`var p [2]int`
`73`		`- err = handleSyscallError(syscall.Pipe2(p[:], syscall.O_CLOEXEC))`
	`73`	`+ err = handleSyscallError(pipe(p[:]))`
`74`	`74`	`if err != nil {`
`75`	`75`	`return`
`76`	`76`	`}`