purego: new mechanism for calling C functions by TotallyGamerJet · Pull Request #49 · ebitengine/purego

TotallyGamerJet · 2022-10-18T02:11:39Z

This PR adds a new function purego.Func which takes a shared object handle, the name of a symbol in the object and a pointer to a function matching the calling convention. This is the start of a better way of calling C functions. First, it is type safe from the callers perspective. Secondly, it removes repeated code such as strings.CString and runtime.KeepAlive into one place (purego.Func). I ported the objective-c runtime to use this new mechanism to test it out and see how it looks in code. I think it's a big improvement.

The downside is that this is a slower method than calling purego.SyscallN directly but will eventually allow for more complex arguments and returns such as floats, complex, and structs.

We need to decide on if purego.Func should return an error?

I'm kind of leaning towards no since the only error you could potentially recover from is one where the symbol isn't found in the shared object but then what would you do? Stub the function? The other errors (fptr not being a function pointer, too many return arguments) just aren't recoverable in any meaningful way which is why I think it should panic.

Should the objc package use this method or call purego.SyscallN directly?

I think the later since this package gets called a LOT in code ported from Objective-C and we want it to be as fast as possible. The overhead of determining the types seems like a waste for these core functions.

TODO:
[X] Document the conversion from Go types to C types as part of purego.Func.
[X] Keep pointer types alive
[ ] Implement those other types.
[ ] Port oto/ebitengine to use this new method?

hajimehoshi

Seems good.

hajimehoshi

Pretty nice. Are you working on the TODOs now?

TotallyGamerJet · 2022-10-18T13:32:04Z

Yes, I am!

TotallyGamerJet · 2022-10-18T15:12:39Z

I've ported Oto to use purego.FuncHandle. Do you want to wait until floats and struct support is added before merging this? I'm not sure when I'll be able to do that since I'm still working on writing the generator code for the glfw.dylib. It is also a decent amount of additional code since I have to write an architecture specific portion to fill the registers and stack properly. This is fully functional as it for all the types currently supported by SyscallN

hajimehoshi · 2022-10-18T15:24:22Z

Do you want to wait until floats and struct support is added before merging this?
[...]
This is fully functional as it for all the types currently supported by SyscallN

I'm confused. Do you mean the current FuncHandle is strong enough to replace SyscallN even without floats and structs?

TotallyGamerJet · 2022-10-18T15:40:32Z

Yes it can replace all occurrences of SyscallN in Oto and Ebitengine in its current state

hajimehoshi · 2022-10-18T16:09:20Z

So

Do you want to wait until floats and struct support is added before merging this?

I'm fine to merge this with leaving a comment about the TODOs!

hajimehoshi · 2022-10-18T17:27:12Z

+//	func => C function
+//	[]T, unsafe.Pointer, *T => void*
+//
+// There is a special case when the last argument of fptr is a variadic interface (or []interface}


Just out of curiosity, would it be hard to support other types than interface{}?

Do you have an example?

Also note that this does not support calling variadic functions like int printf(const char* format, ...); It's basically a cast to the desired C function. (*(char*, int)())(printf)("%d", 3) is not the same as printf("%d, 3) since the second uses va_args under the hood. The first is what is currently handled

For example: https://pkg.go.dev/reflect#Append (I don't think this can be a C function but this is an example with a non-interface{} type)

I now understand that the purpose of RegisterFunc is to have a Go function that is a counterpart of a C function. So, having another type than interface{} would not make sense.

By the way, in the comment, the order of types is (Go type) => (C type), but is there an order here?

Yes the Go function should have the same signature as the C function being called.

There is no order except for how I thought of the type

OK so I'm fine with =>. Still LGTM

I was about to do the <=> for the types that can be returned. Do you think that's a good idea?

Yes that's still a good idea, but I thought you meant this was one direction. Am I misunderstanding?

I like that since some types like slice cannot be returned from C to Go since there is no way to get its length

Oh my bad. I did not include a period after "I like that." I was stating why I thought it was a good idea to have bidirectional arrows. Unless you think it's better to have another column for C => Go conversions?

IMO I prefer <=> from the whole discussion. Let's go with <=> then :-)

hajimehoshi

LGTM, thanks!

hajimehoshi · 2022-10-18T18:04:36Z

I'm gonna sleep so I might reply tomorrow. Thank you for your hard work!

hajimehoshi

Still LGTM. You can merge if you are ready.

The RegisterFunc type table has func <=> C function in it, but that only holds in the argument direction. Declare a C function as returning a func, and calling it panics: panic: reflect.MakeFunc: value of type *func(int32, int32) int32 is not assignable to type func(int32, int32) int32 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.Func case swapped in a *func, registered into that, and handed the pointer back to MakeFunc, which wont take it. If the C function returns NULL the inner RegisterFunc still panics purego: 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 TestABI plus a return_func_ptr helper in abi_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 a uintptr and register it youself, and nothing in the repo declares a func return. Closes #501

TotallyGamerJet added 2 commits October 17, 2022 21:42

first implementation of Func

8e43dcb

support for reflect.Pointer

afb1f60

TotallyGamerJet requested a review from hajimehoshi October 18, 2022 02:11

TotallyGamerJet added 7 commits October 17, 2022 22:13

use old name for reflect.Pointer (reflect.Ptr)

d621b91

use old name for reflect.Pointer (reflect.Ptr)

ab7557a

support reflect.Func and reflect.UnsafePointer argument

0256b3c

support reflect.Slice argument

a699fdb

support reflect.UnsafePointer return

3b4290f

fix pointer return

bb1b1af

support bool argument

f236fea

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

Comment thread func_darwin.go Resolved

Comment thread objc/objc_runtime_darwin.go Resolved

Comment thread func_darwin.go Resolved

Comment thread func_darwin.go Resolved

Comment thread objc/objc_runtime_darwin.go Resolved

TotallyGamerJet added 4 commits October 17, 2022 23:23

add license comment and reorganize imports

5eb75ac

remove error return

d284cd5

support expanding variadic interfaces

6d61c04

move objc dylib into smaller scope

ea78069

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

TotallyGamerJet added 5 commits October 18, 2022 09:45

add documentation and support Func return

d3f37fe

runtime.keepAlive

6247281

document variadic interface expansion

076985b

support string return

e784c4e

fix possible misuse unsafe.Pointer warning

09b4e99

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

reformat comment

1312bde

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread dlfcn.go Resolved

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

TotallyGamerJet added 4 commits October 18, 2022 12:26

rename FuncHandle to RegisterFuncHandle

718414a

add todo

32bc919

reorder arguments

6bfd775

rename RegisterFuncHandle to RegisterLibFunc

f8d0577

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

swap arguments in registerFunc

586b8f0

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

Comment thread func_darwin.go Resolved

TotallyGamerJet added 3 commits October 18, 2022 13:14

fix return Func

d836435

update comment

ae6c276

update comment 2

39a1230

hajimehoshi reviewed Oct 18, 2022

View reviewed changes

TotallyGamerJet added 2 commits October 18, 2022 13:44

export RegisterFunc

745816a

document WIP

c04074e

hajimehoshi approved these changes Oct 18, 2022

View reviewed changes

TotallyGamerJet added 2 commits October 18, 2022 15:45

describe exception case

75b7353

bidirectional arrows for conversions

6bdd0e9

hajimehoshi approved these changes Oct 19, 2022

View reviewed changes

TotallyGamerJet merged commit 7128aea into ebitengine:main Oct 19, 2022

TotallyGamerJet deleted the func branch October 19, 2022 15:39

This was referenced Aug 23, 2026

purego: fix returning a C function pointer as a Go func #500

Merged

Calling a C function that returns a function pointer panics #501

Closed

Conversation

TotallyGamerJet commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hajimehoshi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hajimehoshi left a comment

Choose a reason for hiding this comment

Uh oh!

TotallyGamerJet commented Oct 18, 2022

Uh oh!

Uh oh!

TotallyGamerJet commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hajimehoshi commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TotallyGamerJet commented Oct 18, 2022

Uh oh!

hajimehoshi commented Oct 18, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TotallyGamerJet Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hajimehoshi left a comment

Choose a reason for hiding this comment

Uh oh!

hajimehoshi commented Oct 18, 2022

Uh oh!

hajimehoshi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TotallyGamerJet commented Oct 18, 2022 •

edited

Loading

TotallyGamerJet commented Oct 18, 2022 •

edited

Loading

hajimehoshi commented Oct 18, 2022 •

edited

Loading

TotallyGamerJet Oct 18, 2022 •

edited

Loading