Repository navigation
fix: sync fakecgo fixes from purego, lint and test every target - #92
Merged
Merged
Conversation
kolkov
added
priority: high
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
fakecgo_test.go (arm64 && !cgo) imports internal/fakecgo, which has no files on Windows, so go test ./internal/arch/arm64 failed with "[setup failed]" on Windows ARM64. Windows needs no fakecgo: runtime.cgocall works there without it. Exclude windows from the constraint.
TestExecuteCaptureRegistersSimple failed at random on linux/arm64 (GPR mismatch: 0 0 0 0) depending on test flags and Go version. It passed &out to the capture shim as a uintptr. A uintptr is invisible to escape analysis, so out stayed on the goroutine stack; when the stack grew before the call, the shim wrote to the old stack and the test read zeros. Forcing a stack copy right after taking the address made it fail 15/15. Keep the argument an unsafe.Pointer, as ffi.CallFunction documents: avalue leaks, so out moves to the heap. TestCallback_PointerArg had the same bug: it stored &original as a raw word in a fake callback frame, and a forced stack copy made it fail 5/5. The frame plays the C caller, so pin the target with runtime.Pinner, as for any Go pointer stored in memory C reads. Both tests now pass with a forced stack copy.
A uintptr hides the target from escape analysis: a local it points to can stay on the goroutine stack and move before the C function runs, so C gets a dangling pointer. Two goffi tests failed at random on linux/arm64 for exactly this reason. - Package docs and CallFunctionContext Safety notes: keep pointer arguments in unsafe.Pointer variables, and pin Go pointers stored in memory that C reads with runtime.Pinner. - README strlen example: strPtr is an unsafe.Pointer. The old example was harmless (string literal data is not on the stack) but taught the pattern. Checked on Windows and Linux: strlen(...) = 13.
…'s FP threadentry_trampoline is not NOFRAME, so the assembler's prologue saves R30 and stores the caller's R29 at -8(RSP). The manual SUB of 8*24 moves that slot to 8*23(RSP), and the hand-written STP (R29, R30) at 8*22 wrote R30 over it. The epilogue then returned to C with R29 = R30. When an OS thread created by the runtime exits, for example because a goroutine returns while locked with runtime.LockOSThread, the thread returns from threadentry_trampoline into glibc's start_thread, which reads [x29, #56] and faults. TestThreadEntryReturn (added later in this series with the amd64 frame-pointer fix) crashed with SIGSEGV 3/3 on linux/arm64 without this change (QEMU, Go 1.25.3 to tip, and the native ubuntu-24.04-arm runner) and passes 3/3 with it; darwin/arm64 did not crash because its libpthread does not use x29 there. This fix comes before the set*id and frame-pointer commits because their tests exit locked threads. Drop the manual R29/R30 save and restore, as upstream did in ebitengine/purego#426. crosscall2, the other function saving R29/R30 by hand, is NOFRAME and needs it.
With fakecgo, runtime.iscgo is true but the syscall.cgo_libc_set*id hooks that runtime/cgo provides were missing. syscall.Setuid, Setgid, Setgroups and the other set*id calls then fell back to runtime.doAllThreadsSyscall, which panics with "doAllThreadsSyscall not supported with cgo enabled". Every CGO_ENABLED=0 Linux program importing goffi crashed on these calls. Provide the hooks like runtime/cgo does (linux.go + linux_syscall.c): they call the glibc functions, which apply the change to every thread of the process. syscall.AllThreadsSyscall now returns ENOTSUP, as with real cgo. gen.go can now generate wrappers for OS-specific symbols; the existing generated files are unchanged. Port of ebitengine/purego#426. TestSetuidEtc comes from Go's cmd/cgo/internal/test; as root it checks the credentials of every thread. Android is not covered: Bionic needs __errno instead of __errno_location.
stack_t had ss_flags before ss_size (the Linux order). On NetBSD (sys/sys/sigtypes.h) ss_size comes first, so the sigaltstack call in threadentry passed ss_size=4, ss_flags=0 and never disabled the signal stack inherited from the creating thread (ebitengine/purego#433, #304). pthread_mutex_t and pthread_cond_t were 8-byte integers initialised to 0. NetBSD defines them as structs (48/40 bytes on amd64, 40/40 on aarch64) whose magic field every libpthread call checks, so locking the runtime init mutex returned EINVAL without locking, or aborted with PTHREAD_DIAGASSERT=a. Mirror struct __pthread_mutex_st and __pthread_cond_st and use the magic values of the static initializers. Also build trampolines_{amd64,arm64}.s on NetBSD: without them x_cgo_init_trampoline and threadentry_trampoline were not code. The expected layouts were computed by gcc from the NetBSD headers and are checked at compile time, so cross-compiling the tests for netbsd catches a drift. NetBSD is not a supported platform yet; nothing was run on NetBSD.
When an M created by fakecgo exits (e.g. a goroutine returns while locked to its OS thread), mexit returns from runtime.mstart into threadentry with BP set to 0. Real cgo calls mstart through crosscall, which saves and restores the callee-saved registers; fakecgo called it directly. Go releases up to 1.27 restore SP arithmetically in threadentry's epilogue, so the zero BP went unnoticed. With the amd64 LEAVE epilogue (golang/go CL 548317, July 2026) it faults: TestThreadEntryReturn crashed with SIGSEGV 3 of 3 runs with go1.28-devel 3d45c388. Current tip skips LEAVE in functions that call ABI0 code (golang/go#80710), which only incidentally covers threadentry today. Call mstart through callThreadEntryFn, an amd64 assembly shim that saves and restores the callee-saved registers like crosscall does. Other architectures keep the indirect call. Port of ebitengine/purego#486.
FreeBSD binaries built with CGO_ENABLED=0 had DT_NEEDED libpthread.so. That name is only a development symlink to libthr.so: lib/libthr/Makefile installs it through SYMLINKS, which bsd.lib.mk tags as dev, so a system installed from base packages (pkgbase) has it only with FreeBSD-clibs-dev. ebitengine/purego#480 reports the resulting start-up failure on FreeBSD 15.1-STABLE. Import the pthread symbols from libthr.so.3, the name the library is installed under (SHLIB_MAJOR=3, /lib). ffi/callback_cthread_test.go already loads libthr.so.3. Port of ebitengine/purego#481.
- crosscall2 (arm64): drop the runtime·load_g call. runtime·cgocallback loads g itself, or takes it from frame when fn is nil, so the call was redundant and only kept one more runtime-internal linkname in use (ebitengine/purego#456). The callback dispatcher comments no longer claim crosscall2 calls load_g (amd64 never did). - x_cgo_init: keep pthread_attr_t on the stack, as _cgo_sys_thread_start already does, instead of a malloc that only worked around clang < 3.8 msan (ebitengine/purego#432, golang/go CL 745662). - pthread_attr_t is now [8]uint64 instead of [64]byte, so it has the 8-byte alignment of the C type on the Go stack; the size still covers glibc (56/64), Darwin (64), FreeBSD (8) and NetBSD (16). - structs.HostLayout on Darwin pthread_mutex_t and pthread_cond_t (ebitengine/purego#502).
On Windows, GetSymbol used the deprecated syscall.StringBytePtr, which panics on a NUL byte, so GetSymbol(h, "a\x00b") crashed the process. On Linux, macOS, FreeBSD and Android the name was passed to dlopen/dlsym as a C string and silently cut at the NUL: LoadLibrary and GetSymbol then looked up a different library or symbol. LoadLibrary and GetSymbol now return a *LibraryError wrapping syscall.EINVAL on every OS, as Windows LoadLibrary already did through syscall.UTF16PtrFromString. GetSymbol on Windows uses syscall.BytePtrFromString.
- Darwin ObjC test: objcCallCPtr lets the FFI call write the UTF8String result straight into an unsafe.Pointer, so cString no longer converts a uintptr (go vet unsafeptr). - Remove objcArgInt64, objcArgRect, objcArgSize and objcCallBool from the Darwin ObjC tests: added in b7dcd3d (#9) and never called. - reflect.Ptr is the old name of reflect.Pointer; callback.go already uses the new one, callback_arm64.go and callback_windows.go now do too. - classification.go: elementKind's type is inferred from the typed constant (staticcheck QF1011). No behaviour change.
.golangci.yml had run.build-tags: [linux] since the initial commit. A GOOS-named tag adds *_linux.go to the host OS files instead of switching the target, so off Linux typecheck failed and golangci-lint analysed nothing. CI linted only linux/amd64, so arm64, darwin, windows, freebsd and android code was never linted. - Remove the build tag. - New lint-targets job (part of CI Success): linux/arm64, darwin and windows (amd64, arm64), freebsd (amd64, arm64) with GOOS/GOARCH and CGO_ENABLED=0, plus linux/amd64 with CGO_ENABLED=1 for the cgo-only files. The existing Lint job (a required check) is unchanged. - android/arm64 is linted in the informational android-cross job. - Exclude SA4006 in ffi/struct_e2e_test.go: on Android NewCallback always panics by design, so staticcheck sees the code after it as unreachable. - Drop the Git Flow header and the develop branch triggers; the branch no longer exists. All targets report 0 issues locally with golangci-lint v2.14.0, the version CI installs.
Bugs fixed on this branch reached releases because CI never ran tests on the target or toolchain where they live. - Test matrix: add ubuntu-24.04-arm, windows-11-arm and macos-15-intel. linux/arm64, windows/arm64 and darwin/amd64 were only cross-compiled; go test ./internal/arch/arm64 did not even build on Windows ARM64 since v0.5.1. The comment claiming there are no Intel macOS runners was wrong (macos-15-intel is available to public repositories). - test-go-stable (part of CI Success): the suite ran only on Go 1.25, the go.mod minimum; fakecgo depends on runtime and compiler internals that change between releases. - test-go-tip (informational): the amd64 LEAVE epilogue that crashed fakecgo's threadentry was visible on tip before any release. - Run the fakecgo set*id tests as root on Linux (CGO_ENABLED=0): only as root does TestSetuidEtc check that every thread got the new credentials. The test binary is built as the runner user so root never writes to the Go caches. - Cross-compile: every FreeBSD test package must compile and link (go test -exec=/bin/true), FreeBSD binaries must have DT_NEEDED libthr.so.3 and not libpthread.so, and vetting internal/fakecgo for netbsd evaluates its compile-time ABI layout asserts. - Weekly scheduled run and workflow_dispatch, so toolchain changes are noticed without a commit.
kolkov
force-pushed
the
fix/fakecgo-upstream-sync
branch
from
October 15, 2026 22:05
9cce03a to
3eceb5a
Compare
Contributor
Author
|
@lkmavi thanks for checking #90 on your Mac, Could you run this PR's branch on macOS as well? It touches code that only runs for real on Apple hardware:
git fetch origin fix/fakecgo-upstream-sync && git checkout FETCH_HEAD
CGO_ENABLED=0 go test -count=1 ./...
CGO_ENABLED=1 go test -count=1 ./...If you have a gogpu app at hand, a quick run of one of its examples on top of this branch would also help. No rush. |
Contributor
|
@kolkov Passed |
This was referenced Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related pieces of work:
internal/fakecgochange in purego since our snapshot (23e4f41→d456b1b, v0.12.0-alpha.1) was checked against our code and against primary sources (Go runtime, glibc/FreeBSD/NetBSD headers). Each applicable fix is ported in its own commit, with a test or a check that fails before the change.Every commit builds for linux, darwin, windows, freebsd (amd64/arm64) and android/arm64 and passes the full test suite on linux/amd64 (
CGO_ENABLED=0and1) on its own. Fixes for bugs already onmaincome before the commits whose tests expose them, so on linux/arm64 (QEMU) every commit from the second on passes too; the first one still carries the flaky-test bug frommainthat the second commit fixes.Bugs fixed
mainsyscall.Setuid/Setgid/Setgroups/… panic withdoAllThreadsSyscall not supported with cgo enabledin everyCGO_ENABLED=0Linux program that imports goffiGetSymbolpanics on a NUL (deprecatedsyscall.StringBytePtr); elsewhere the name is cut at the NUL and a different library/symbol can be usedgo test ./internal/arch/arm64fails with[setup failed]on Windows ARM64 (since v0.5.1)LEAVEepilogue of the Go 1.28 dev treeDT_NEEDED libpthread.sois a dev-only symlink on FreeBSDstack_torder (purego#433); mutex/cond were zeroed 8-byte integers instead of NetBSD's structs with a magic numberCleanups: purego#456 (
load_gin arm64crosscall2), #432 (x_cgo_initattr on the stack), #502 (HostLayout), 8-byte alignedpthread_attr_t,reflect.Pointer, a darwin test that no longer convertsuintptrtounsafe.Pointer, and four Darwin test helpers that were never called (added in #9).Not ported: purego#464 (our FreeBSD files are already per-arch) and style-only or other-architecture commits.
Found by the first native linux/arm64 run of this PR
mainthreadentry_trampolinesaved R29/R30 by hand inside the frame the assembler already sets up and overwrote the caller's saved FP; when a runtime thread exits (e.g. a goroutine returns while locked to its OS thread), glibc'sstart_threadfaults. SIGSEGV 3/3 on linux/arm64. Upstream removed the same lines in purego#426; my first audit wrongly marked that part as not applicableCGO_ENABLED=0)TestExecuteCaptureRegistersSimpleandTestCallback_PointerArgpassed Go pointers asuintptr, so the target stayed on the goroutine stack and a stack copy left the callee writing to the old stack; random failures (forced stack copy: 15/15 and 5/5 failures before, 0 after)uintptrpattern; package docs now state the rule and point toruntime.PinnerDetails
setuid. With fakecgo,
runtime.iscgois true butsyscall.cgo_libc_setuidand friends were nil, sosyscall.SetuidcalledAllThreadsSyscall→runtime.doAllThreadsSyscall, which panics wheniscgois set (runtime/os_linux.go). The hooks now call glibc'sset*id, which changes the credentials of every thread.gen.gocan generate OS-specific wrappers; the existing generated files are byte-for-byte unchanged. Android (Bionic uses__errno, not__errno_location) is left for a follow-up.Frame pointer.
mstartreturns intothreadentrywithBP = 0when an M exits. Go up to 1.27 restoresSPwithADDQ/POPQ BPthere, so nothing happens (checked withobjdumpon 1.25.3 and 1.27.1). golang/go CL 548317 (July 2026) made the compiler useLEAVE, which then faults. Current tip skipsLEAVEin functions that call ABI0 code (golang/go#80710), which coversthreadentryonly by accident. With Go built at golang/go3d45c388,TestThreadEntryReturncrashed with SIGSEGV 3/3 before the change and passes 3/3 after it.FreeBSD.
lib/libthr/Makefileinstallslibpthread.sothroughSYMLINKS, whichshare/mk/bsd.lib.mktagsdev, so on pkgbase systems it only ships withFreeBSD-clibs-dev.readelf -d:libpthread.sobefore,libthr.so.3after (amd64 and arm64).NetBSD.
stack_thad the Linux field order (sys/sys/sigtypes.h:ss_sp, ss_size, ss_flags), sothreadentrynever disabled the inherited signal stack.pthread_mutex_t/pthread_cond_twere 8-byte integers set to 0; NetBSD's are structs with a magic number (48/40 bytes on amd64, 40/40 on aarch64), sopthread_mutex_lockreturnedEINVALwithout locking. Layouts were computed by gcc from the NetBSD headers and are asserted at compile time.trampolines_{amd64,arm64}.sare now also built on NetBSD. Nothing was run on NetBSD.Lint
.golangci.ymlhadbuild-tags: [linux]: a GOOS-named tag adds*_linux.goto the host files instead of switching the target, so linting off Linux type-checked nothing, and CI linted only linux/amd64. The tag is gone; a newlint-targetsjob (in CI Success) lints linux/arm64, darwin, windows, freebsd and linux/amd64 with cgo; android is linted in its informational job. One documented exclusion: SA4006 inffi/struct_e2e_test.go, because on AndroidNewCallbackalways panics by design.CI
windows-11-armin the test matrixmacos-15-intelin the test matrixubuntu-24.04-armin the test matrixtest-go-stable(gates) andtest-go-tip(informational), weekly scheduled runTestSetuidEtcas root on LinuxDT_NEEDEDcheckedNot covered yet: running anything on FreeBSD/NetBSD (VM job, planned).
Testing done locally
go test -count=1 ./...withCGO_ENABLED=0and1on Go 1.27.1, tipd61b240band3d45c388; fakecgo tests also with a Go 1.25.3 build.TestSetuidEtcas root: all 21 cases change the credentials of every thread.go test -count=1 ./...withCGO_ENABLED=0and1.golangci-lintv2.14.0 (the version CI installs): 0 issues on all linted targets.actionlintv1.7.12 onci.yml: clean.qemu-aarch64, glibc 2.35 arm64):internal/arch/arm645/5,TestThreadEntryReturn3/3; both bugs were reproduced there first and checked under gdb.TestSetuidEtchangs under this QEMU (purego documents the QEMU ≤ 8.2tgkillbug for the same test), so it is left to the native arm64 runner.Note for #88: the NetBSD
trampolines_*.sbuild tags and layouts are now in this PR, so #88 will need a rebase.