Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cgo: synthesized _cgoCheckPointer calls weaken function calling type checking #16591

Closed
mdempsky opened this issue Aug 3, 2016 · 3 comments

Comments

@mdempsky
Copy link
Member

commented Aug 3, 2016

Trying to build this code:

package p

// extern void foo(int** p) {}
import "C"

type x *C.int

func bar(**C.int) {
}

func f(p *x) {
    C.foo(p)
    bar(p)
}

fails with:

./foo.go:13: cannot use p (type *x) as type **C.int in argument to bar

I.e., the bar(p) call is rejected, but the C.foo(p) call is not, even though they both logically have a parameter of type **C.int.

The C.foo(p) is being rewritten by cgo into:

    _Cfunc_foo(_cgoCheckPointer((**_Ctype_int)(p)).(**_Ctype_int))

The Go spec says function call arguments should be assignable to their corresponding parameters. But because of the explicit conversion introduced by cgo, this requirement is being relaxed from assignability to convertibility.

Probably not an issue in practice, but I noticed it while addressing mdempsky/unconvert#16.

I think we could address this by generating type-specific _cgoCheckPointer wrappers, but until we can inline calls to non-leaf functions, that might be undesirable.

/cc @ianlancetaylor

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2016

It seems like we should fix this. It was broken in Go 1.7 so it's not terribly urgent, but the longer we wait the harder it is to fix. I don't really mind about having extra calls even without non-leaf inlining. OK to drop for Go 1.8 but if it's easy to fix, please do.

@rsc rsc added NeedsFix and removed NeedsDecision labels Oct 10, 2016

@gopherbot

This comment has been minimized.

Copy link

commented Oct 13, 2016

CL https://golang.org/cl/30973 mentions this issue.

gopherbot pushed a commit that referenced this issue Oct 13, 2016
cmd/cgo: use alias for unsafe rather than separate functions
When we need to generate a call to _cgoCheckPointer, we need to type
assert the result back to the desired type. That is harder when the type
is unsafe.Pointer, as the package can have values of unsafe.Pointer
types without actually importing unsafe, by mixing C void* and :=. We
used to handle this by generating a special function for each needed
type, and defining that function in a separate file where we did import
unsafe.

Simplify the code by not generating those functions, but instead just
import unsafe under the alias _cgo_unsafe. This is a simplification step
toward a fix for issue #16591.

Change-Id: I0edb3e04b6400ca068751709fe063397cf960a54
Reviewed-on: https://go-review.googlesource.com/30973
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Oct 14, 2016

CL https://golang.org/cl/31233 mentions this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.