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: a type alias of unsafe.Pointer is not treated as unsafe.Pointer #67333

Open
dominikh opened this issue May 12, 2024 · 7 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dominikh
Copy link
Member

Go version

go version devel go1.23-07fc59199b Sun May 12 05:36:29 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/dominikh/.cache/go-build'
GOENV='/home/dominikh/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/dominikh/prj/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/dominikh/prj'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/dominikh/prj/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/dominikh/prj/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-07fc59199b Sun May 12 05:36:29 2024 +0000'
GODEBUG=''
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/dominikh/prj/src/example.com/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1241008282=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Run the following code:

package main

// void foo(void *arg) {}
import "C"
import "unsafe"

type T struct {
	self int
	x    *int
}

type up = unsafe.Pointer

func main() {
	x := T{x: new(int)}

	C.foo(unsafe.Pointer(&x))      // reports "cgo argument has Go pointer to unpinned Go pointer", as expected
	C.foo(unsafe.Pointer(&x.self)) // doesn't report anything, as desired
	C.foo(up(&x.self))             // reports "cgo argument has Go pointer to unpinned Go pointer", which is surprising
}

What did you see happen?

The first and third calls panic with runtime error: cgo argument has Go pointer to unpinned Go pointer. The second call doesn't panic.

What did you expect to see?

Only the first call to panic.

Arguably one might also expect all 3 calls to panic, since this also panics, regardless of the use of an alias:

y := unsafe.Pointer(&x.self)
C.foo(unsafe.Pointer(y))

Which makes the use of the 2nd form rather brittle.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 12, 2024
@cuonglm
Copy link
Member

cuonglm commented May 13, 2024

This seems to me a cmd/cgo issue, not the runtime.

cgo does not recorgnize that up(&x.self) is a type conversion, so it emits the cgo check pointer call with up(&x.self) as argument, not &x.self in case of unsafe.Pointer(&x.self).

@ianlancetaylor ianlancetaylor changed the title runtime: cgocheck behaves differently for type alias of unsafe.Pointer cmd/cgo: a type alias of unsafe.Pointer is not treated as unsafe.Pointer May 13, 2024
@ianlancetaylor
Copy link
Contributor

I don't know how to fix this except to do full type checking on the cgo input file, which is difficult as we currently expect the type checker to run on cgo output, not cgo input. Fixing this correctly may require something like #16623, which is a lot of work.

@cuonglm
Copy link
Member

cuonglm commented May 13, 2024

@ianlancetaylor We could probably record type alias only, something like map[string]string{"up": "unsafe.Pointer"}, then the p.isType check can correctly recognize and stripe the type conversion.

@ianlancetaylor
Copy link
Contributor

We can handle simple cases, but what if we import some other package that has type MyUnsafePointer = unsafe.Pointer and then refer to it as otherpkg.MyUnsafePointer(x) ? Is it better to have some type aliases work and some not, or is it better to not try to make type aliases work? It's not clear to me which approach is less confusing.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 13, 2024
@dmitshur dmitshur added this to the Backlog milestone May 13, 2024
@adonovan
Copy link
Member

adonovan commented May 13, 2024

A related previous issue was #57926, in which users exploited the desugaring of cgo to declare methods on types that appear to belong to package C: func (C.T) method() {} The resolution was to use syntactic heuristics to try to reject it, so that we at least make clear that this is not allowed, even if we can't detect it in general (e.g. in the presence of aliasing) without type checking, which is infeasible for the reason @ianlancetaylor noted.

@dmitshur
Copy link
Contributor

@ianlancetaylor Assigned it to you for now, please feel free to update.

@ianlancetaylor
Copy link
Contributor

Thanks, I'm unassigning as I have no plans to work on this.

@ianlancetaylor ianlancetaylor removed their assignment May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: In Progress
Development

No branches or pull requests

6 participants