From afd2d408e4fe1e26f5e091b3af7bcbaaf20f7047 Mon Sep 17 00:00:00 2001 From: unxed Date: Thu, 17 Sep 2026 10:52:14 +0000 Subject: [PATCH] fix: build callback_amd64.s on FreeBSD ffi/callback.go is built for (linux || darwin || freebsd) && amd64, but ffi/callback_amd64.s, which holds the 2000-entry trampoline table and callbackDispatcher, was built only for (linux || darwin) && amd64. The Go side takes the table's address through a placeholder: //go:linkname _callbackTrampoline github.com/go-webgpu/goffi/ffi.callbackTrampoline var _callbackTrampoline byte With the assembly excluded, that one-byte variable becomes the definition of ffi.callbackTrampoline. Nothing fails to compile or link. On a linked freebsd/amd64 consumer, `go tool nm` reports D github.com/go-webgpu/goffi/ffi.callbackTrampoline and no callbackDispatcher or callbackWrap at all, where linux/amd64 and freebsd/arm64 report T for all three. NewCallback therefore returns addresses computed from a data-segment byte, not code. The dispatcher is plain System V AMD64 (the same ABI FreeBSD uses) and enters Go through crosscall2, which internal/fakecgo already provides on FreeBSD, so the fix is adding freebsd to the constraint. The existing cross-compile job could not catch this: `go build ./...` links nothing, and the broken build compiles. scripts/check-callback- trampolines.sh links a small NewCallback consumer for every target whose callbacks come from goffi's own assembly (linux, darwin, freebsd x amd64, arm64) and fails unless the symbol is T. Run against the tree before this change it reports freebsd/amd64 as 'D'; after it, all six targets pass. The Cross-Compile job now runs it. Not verified on a FreeBSD host; the evidence is the symbol table of the cross-linked binaries. Refs: go-webgpu/goffi#76 (review) --- .github/workflows/ci.yml | 7 +++ CHANGELOG.md | 2 + ffi/callback_amd64.s | 2 +- scripts/check-callback-trampolines.sh | 91 +++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100755 scripts/check-callback-trampolines.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31f0c4f..e23a443 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -238,6 +238,13 @@ jobs: echo "✅ netbsd/$arch fakecgo layouts" done + # Targets whose NewCallback uses goffi's own assembly trampolines must + # link ffi.callbackTrampoline as code. A .s file with a narrower build + # constraint than its .go file still compiles and links, so only the + # symbol type of a linked binary shows the breakage. + - name: Check callback trampolines link as code + run: scripts/check-callback-trampolines.sh + # Android arm64/API 29+ source, ABI, cgo-mode, and ELF regression checks. # Keep both supported Go patch lines: runtime/cgo startup and TLS details # are part of this platform contract, so a single floating toolchain is not diff --git a/CHANGELOG.md b/CHANGELOG.md index ec38034..895a990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`LoadLibrary` / `GetSymbol`: names containing a NUL byte.** On Windows `GetSymbol` panicked (deprecated `syscall.StringBytePtr`); on Linux, macOS, FreeBSD and Android the name was cut at the NUL, so a different library or symbol could be loaded. Both functions now return a `*LibraryError` wrapping `syscall.EINVAL` on every OS. - Windows ARM64: `go test ./internal/arch/arm64` failed with `[setup failed]` because a test file imported `internal/fakecgo`, which has no Windows files. - `internal/fakecgo`: smaller cleanups from purego — `crosscall2` on arm64 no longer calls `runtime·load_g` ([purego#456](https://github.com/ebitengine/purego/pull/456)), `x_cgo_init` keeps `pthread_attr_t` on the stack ([purego#432](https://github.com/ebitengine/purego/pull/432)), `pthread_attr_t` has the 8-byte alignment of the C type, and the Darwin mutex/cond types use `structs.HostLayout` ([purego#502](https://github.com/ebitengine/purego/pull/502)). +- **FreeBSD amd64: callbacks pointed into a data page** — `ffi/callback_amd64.s` carried `(linux || darwin) && amd64` while `ffi/callback.go` carried `(linux || darwin || freebsd) && amd64`, so on freebsd/amd64 the assembly trampoline table was never built and the `//go:linkname` placeholder variable became `ffi.callbackTrampoline`. The build succeeded, but `NewCallback` returned addresses in the data segment (`go tool nm`: `D`, not `T`). The assembly now builds on FreeBSD too. +- CI: `scripts/check-callback-trampolines.sh` links a consumer for every asm-trampoline target (linux, darwin, freebsd × amd64, arm64) and fails unless `ffi.callbackTrampoline` is a text symbol. ## [0.6.4] - 2026-09-10 diff --git a/ffi/callback_amd64.s b/ffi/callback_amd64.s index 32d29f4..82bd360 100644 --- a/ffi/callback_amd64.s +++ b/ffi/callback_amd64.s @@ -1,4 +1,4 @@ -//go:build (linux || darwin) && amd64 +//go:build (linux || darwin || freebsd) && amd64 #include "textflag.h" #include "go_asm.h" diff --git a/scripts/check-callback-trampolines.sh b/scripts/check-callback-trampolines.sh new file mode 100755 index 0000000..f5bc63d --- /dev/null +++ b/scripts/check-callback-trampolines.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# check-callback-trampolines.sh — assert that every target whose NewCallback is +# built from goffi's own assembly links ffi.callbackTrampoline as code. +# +# ffi/callback.go and ffi/callback_arm64.go take the address of the trampoline +# table through a //go:linkname *variable*. If the matching .s file is excluded +# by a narrower build constraint than the .go file, that variable itself +# becomes the definition of ffi.callbackTrampoline: the package still compiles +# and links, `go tool nm` reports D (data) instead of T (text), and +# NewCallback hands C code a pointer into a data page. freebsd/amd64 was built +# that way while callback_amd64.s lacked `freebsd` in its constraint, and no +# compile step could notice. +# +# The check links a real consumer binary per target, because building ./... +# links nothing, and inspects the symbol type. +# +# Usage: scripts/check-callback-trampolines.sh + +set -euo pipefail + +MODULE="github.com/go-webgpu/goffi" +SYMBOL="${MODULE}/ffi.callbackTrampoline" +FAKECGO_STD="-gcflags=${MODULE}/internal/fakecgo=-std" + +# goos/goarch|extra go build flags +TARGETS=( + "linux/amd64|" + "linux/arm64|" + "darwin/amd64|" + "darwin/arm64|" + "freebsd/amd64|${FAKECGO_STD}" + "freebsd/arm64|${FAKECGO_STD}" +) + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +GO_VERSION="$(cd "${ROOT}" && go list -m -f '{{.GoVersion}}')" + +PROBE="$(mktemp -d)" +trap 'rm -rf "${PROBE}"' EXIT + +cat >"${PROBE}/go.mod" < ${ROOT} +EOF + +cat >"${PROBE}/main.go" <<'EOF' +package main + +import "github.com/go-webgpu/goffi/ffi" + +func main() { + // A live reference keeps the trampoline table in the binary. Without it, + // dead-code elimination drops the symbol and the check has nothing to see. + _ = ffi.NewCallback(func(a uintptr) uintptr { return a }) +} +EOF + +FAILED=0 +for entry in "${TARGETS[@]}"; do + IFS='|' read -r target flags <<<"${entry}" + goos="${target%%/*}" + goarch="${target##*/}" + bin="${PROBE}/probe-${goos}-${goarch}" + + # shellcheck disable=SC2086 # flags is either empty or a single word + if ! (cd "${PROBE}" && CGO_ENABLED=0 GOOS="${goos}" GOARCH="${goarch}" \ + go build ${flags} -o "${bin}" .); then + echo "❌ ${target}: consumer probe failed to build" + FAILED=1 + continue + fi + + kind="$(go tool nm "${bin}" | awk -v s="${SYMBOL}" '$NF == s { print $(NF-1) }')" + if [ "${kind}" = "T" ]; then + echo "✅ ${target}: ffi.callbackTrampoline is code (T)" + else + echo "❌ ${target}: ffi.callbackTrampoline has type '${kind:-missing}', want T" + echo " Check that the build constraint of the callback .s file covers this target." + FAILED=1 + fi +done + +if [ "${FAILED}" -ne 0 ]; then + exit 1 +fi +echo "✅ Callback trampolines link as code on all asm-trampoline targets"