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: cgo pointer checking issue with struct fields #25941

Closed
egonelbre opened this issue Jun 18, 2018 · 7 comments

Comments

@egonelbre
Copy link
Contributor

commented Jun 18, 2018

Issue: go 1.10.2 and current go tip (b7d9e6e).
Machine: darwin/amd64

When you use a pointer inside to another struct with pointers when calling C then CGO check fails with "panic: runtime error: cgo argument has Go pointer to Go pointer".

Possibly related to #14210

Code to reproduce:

package main

// void exampleCall(void *p) {}
import "C"
import "unsafe"

type Data struct {
	Map          map[int32]int32
	InternalData InternalData
}
type InternalData struct{ X, Y, Z float32 }

func main() {
	data := &Data{}
	data.Map = make(map[int32]int32)
	C.exampleCall(unsafe.Pointer(&data.InternalData))
}
@andybons

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@andybons andybons added this to the Unplanned milestone Jun 18, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

This is a bug that should be fixed some day. The problem is that the conversion to unsafe.Pointer breaks the cgo pointer check. It no longer knows the type of the value, so it examines the entire memory area.

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2018

I was examining / thinking how to fix it. What if it were to unwrap unsafe.Pointer part for the cgoCheckPointer?

So, instead of generating:

func (_cgo0 unsafe.Pointer) {
    _cgoCheckPointer(_cgo0)
    C.f(_cgo0)
}(unsafe.Pointer(p))

It would generate:

func (_cgo0 ptype) {
    _cgoCheckPointer(_cgo0)
    C.f(unsafe.Pointer(_cgo0))
}(p)

The same goes for the needsPointerCheck - i.e. ignore the outermost cast.

Thoughts?

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2018

Probably only in cases when ptype itself is a pointer. Because it still would need to check unsafe.Pointer(uintptr(p)) and similar.

@gopherbot

This comment has been minimized.

Copy link

commented Jun 20, 2018

Change https://golang.org/cl/119976 mentions this issue: cmd/cgo: avoid losing some typeinfo with cgoCheckPointer

@gopherbot

This comment has been minimized.

Copy link

commented Jun 23, 2018

Change https://golang.org/cl/120615 mentions this issue: cmd/cgo: rewrite pointer checking

@gopherbot

This comment has been minimized.

Copy link

commented Oct 17, 2018

Change https://golang.org/cl/142884 mentions this issue: cmd/cgo: rewrite pointer checking to use more function literals

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