purego: fix returning a C function pointer as a Go func - #500
Open
NotAFlightRisk wants to merge 2 commits into
Open
purego: fix returning a C function pointer as a Go func#500NotAFlightRisk wants to merge 2 commits into
NotAFlightRisk wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an issue in purego.RegisterFunc where a C function that returns a function pointer would be wrapped as a *func(...)... and then returned from reflect.MakeFunc, causing a panic due to a type mismatch. This aligns the return-path behavior for reflect.Func with the other return-kind cases (i.e., returning the declared Go type, not a pointer to it).
Changes:
- Fix
reflect.Funcreturn handling so the returned value remains a Gofunc(not*func) while still allowingRegisterFuncto populate it. - Add an ABI test case for a C function that returns a function pointer, and validate calling the returned function from Go.
- Add a C test helper that returns a function pointer (
return_func_ptr).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
func.go |
Fixes the reflect.Func return-kind path to register into the address of the func value while returning the correctly-typed func. |
func_test.go |
Adds a regression test that calls a C function returning a function pointer and then invokes the returned function from Go. |
testdata/abitest/abi_test.c |
Adds return_func_ptr and its target function to supply a function pointer return value for the ABI test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
hajimehoshi
reviewed
Aug 23, 2026
Collaborator
Yes please create one |
7 tasks
hajimehoshi
approved these changes
Aug 24, 2026
hajimehoshi
left a comment
Member
There was a problem hiding this comment.
LGTM
@TotallyGamerJet PTAL
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.
What issue is this addressing?
#501
What type of issue is this addressing?
bug
What this PR does | solves
The
RegisterFunctype table hasfunc <=> C functionin it, but that only holds in the argument direction. Declare a C function as returning a func, and calling it panics:Registering is fine, it's the call that goes, and it goes for every signature. Every other case in that return switch leaves the result as the declared type. The
reflect.Funccase swapped in a*func, registered into that, and handed the pointer back toMakeFunc, which wont take it.If the C function returns NULL the inner
RegisterFuncstill panicspurego: cfn is nil. Left that alone, since whether it should hand you a nil Go func instead looked like your call rather than a bug.Test is a block in
TestABIplus areturn_func_ptrhelper inabi_test.c, and it fails on main with teh panic above. Been broken since #49 back in 2022, which is probably why nobody's hit it: you'd normally take the pointer back as auintptrand register it youself, and nothing in the repo declares a func return.