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.

Sign up for GitHub

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

Jump to bottom

cmd/cgo: cgo pointer checking issue with struct fields #25941

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

cmd/cgo: cgo pointer checking issue with struct fields #25941

egonelbre opened this issue Jun 18, 2018 · 7 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@egonelbre
Copy link
Contributor

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
Copy link
Member

/cc @ianlancetaylor

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 18, 2018
@andybons andybons added this to the Unplanned milestone Jun 18, 2018
@ianlancetaylor
Copy link
Contributor

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.

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 18, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 18, 2018
@egonelbre
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@gopherbot
Copy link

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

@dmitshur dmitshur mentioned this issue Nov 2, 2018
Closed
@bobrik bobrik mentioned this issue Nov 15, 2018
Closed
@golang golang locked and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@egonelbre @andybons @ianlancetaylor @gopherbot