search

ebitengine / purego

Apache License 2.0
1.94k stars 63 forks source link

Check `//go:linkname` usage #247

Closed TotallyGamerJet closed 1 month ago

TotallyGamerJet commented 1 month ago

Operating System

What feature would you like to be added?

https://github.com/golang/go/issues/67401

Why is this needed?

We want purego to continue to work

hajimehoshi commented 1 month ago

For our runtime.cgocall, which we pulled, would syscall_cgocaller be available instead?

https://cs.opensource.google/go/go/+/master:src/runtime/cgocall.go;l=110;drc=9fa34d9fa25078423cdb484d39cdd62f067098ac?q=runtime%20cgocall&ss=go%2Fgo

TotallyGamerJet commented 1 month ago

For our runtime.cgocall, which we pulled, would syscall_cgocaller be available instead?

Perhaps for purego.SyscallN but not RegisterFunc since it needs the entire struct since any type of value could be returned

TotallyGamerJet commented 1 month ago

Although this line isn't using //go:linkname perhaps we should make it one so that this function stays around?

https://github.com/ebitengine/purego/blob/6cd12240d3323028bec623dc01c4ec3d40340321/internal/fakecgo/asm_amd64.s#L35

Another possible solution is to just prebuild runtime/cgo for each GOOS,GOARCH, and GOVERSION pair that purego supports into a cgo_GOOS_GOARCH_GOVERSION.syso file. I'm not sure that'll work but it would limit the amount of //go:linkname to just runtime.cgocall and means any changes to the runtime/cgo package would already be included. The hard part is trying to make the builds reproducible. I remember we tried for something else and eventually gave up.

hajimehoshi commented 1 month ago

Of course it is ideal to have reproducible builds, but if there is no other way, we would have to allow such builds...

TotallyGamerJet commented 1 month ago

Of course it is ideal to have reproducible builds, but if there is no other way, we would have to allow such builds...

Are you saying you are in favor of using a prebuilt binary instead of maintaining our own Go port of runtime/cgo?

Of course, I still need to test to make sure this will work

hajimehoshi commented 1 month ago

Are you saying you are in favor of using a prebuilt binary instead of maintaining our own Go port of runtime/cgo?

No, I mean, it is ok-ish to allow non-reproducible binaries if there is no other way. I have no idea which way is better so far.

TotallyGamerJet commented 1 month ago

I think we should move away from using as many linkname comments. It'll hopefully make the code easier to understand and avoid any future breakage. Right now I don't believe purego will break with Go 1.23 but I should try with the go tip as the linkname and required handshake have now made it into tip.

I suggest removing at least the following linkname:

TotallyGamerJet commented 1 month ago

@hajimehoshi I don't think there is anything left to do for this issue. Agree to close it?

hajimehoshi commented 1 month ago

Don't we still use some unexported APIs?

TotallyGamerJet commented 1 month ago

Don't we still use some unexported APIs?

Yes but none of those can be removed without breaking purego. Unless the Go team wants to rewrite runtime/cgo to only use Go we are stuck with those.

hajimehoshi commented 1 month ago

Sure!