bind: make _gopy_clear_go_tls opt-in, off by default#398
Open
satarsa wants to merge 1 commit into
Open
Conversation
The unconditional _gopy_clear_go_tls() call (issue go-python#370) performs a hardcoded TLS store (movq $0, %fs:-8 on linux/amd64). On glibc + CPython 3.12+ that offset overlaps CPython's current-thread-state TLS slot, so the store nulls it and the interpreter segfaults on the first CGo entry in the common single-extension case (issue go-python#395). Add a -clear-go-tls build flag (default false) and gate the single call site on it. The C helper and its pybindgen registration are left in place so opt-in restores the previous behavior exactly. RTLD_GLOBAL-local loading, which is what actually isolates each runtime's goroutine-pointer TLS, is untouched and remains unconditional. Fixes go-python#395
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
The
_gopy_clear_go_tls()call added for issue #370 runs unconditionally before every CGo entry and performs a hardcoded TLS store (movq $0, %fs:-8on linux/amd64,movq $0, %gs:0x30on darwin/amd64). On glibc + CPython 3.12+ that offset overlaps the TLS slot CPython uses for the current thread state (_Py_tss_tstate), so the store nulls it and the interpreter segfaults on the first call: in the common single-extension case, which structurally cannot have the cross-runtime collision the clear was meant to guard against.This PR makes the clear opt-in via a new
-clear-go-tlsbuild flag (defaultfalse): the default build is safe, and the previous behavior stays one flag away.Fixes #395.
The two problems with the unconditional clear
c-sharedunder glibc. In a normal Go binary the runtime lays out its TLS so the g pointer sits at%fs:-8. In ac-sharedlibrary loaded by glibc the loader owns the static TLS block and the g pointer is reached through a linker-assigned TLS variable (runtime.tls_g), not a fixed-8. Somovq $0, %fs:-8does not clear Go's g at all: it zeroes whatever thread-local happens to sit there, which on CPython 3.12+ is_Py_tss_tstate.PR #393 already shipped the mechanism that actually isolates the runtimes: each extension is loaded without
RTLD_GLOBAL, and gopy links each.sowith a version script exporting onlyPyInit_*. Both keep every Go runtime symbol (including the g-pointer TLS) local to its own.so, so there is nothing shared to collide over. The raw-TLS clear is therefore redundant, and with the wrong offset actively harmful.What this PR changes
bind.ClearGoTLSoption and a-clear-go-tlsbuild flag (defaultfalse), mirroring the existing-no-makeflag acrosscmd_build.go,cmd_gen.go,cmd_pkg.go,cmd_exe.go._gopy_clear_go_tls()emission site inbind/gen_func.goon that flag.-clear-go-tlsrestores today's behavior exactly.RTLD_GLOBALhandling untouched and unconditional.6 files, +33/-8.
gofmt,go vet ./..., andgo test ./bind/are clean.Verification
Downstream (the crashing project). Gentoo, glibc, CPython 3.14.6, go 1.26.4,
gcc 16. Rebuilding slidge-whatsapp's extension:
_gopy_clear_go_tls()sites and the first call succeeds (exit 0).gopy build -clear-go-tls: the call reappears (one per CGo entry) and the crash returns (SIGSEGV, exit 139).So the flag is a faithful toggle, and off-by-default removes the crash with no post-build patching.
Multi-extension case, clear off. Two independent gopy extensions (two Go runtimes) loaded in one interpreter, 200000 interleaved cross-calls on the same OS thread with periodic
gc.collect()(the condition #370 describes): 0 wrong results, no crash, no corruption. And each.soexports exactly one global dynamic symbol (PyInit_*) and zeroruntime.*symbols, so the g pointer is per-runtime-private regardless ofRTLD_GLOBAL.I do not have the original #370 reproducer, so I cannot claim to have re-run that exact case; the opt-in keeps the old behavior available for anyone who needs it.
Follow-up (optional)
Happy to add regression tests, here or in a follow-up:
_gopy_clear_go_tls(),-clear-go-tlsproduces one with it._examplescoexistence example, cross-called undergc.collect(), asserting a clean run with the clear off.