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

runtime: ignore pointers from Cgo? #8921

Closed
gopherbot opened this issue Oct 11, 2014 · 9 comments
Closed

runtime: ignore pointers from Cgo? #8921

gopherbot opened this issue Oct 11, 2014 · 9 comments
Milestone

Comments

@gopherbot
Copy link

by marcovanotti15:

What does 'go version' print?
go version go1.3.3 windows/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

Source code: https://gist.github.com/mvanotti/7fcbe9d83758e532d197

1. Download source code on Windows 8
2. go build
3. execute compiled binary

What happened?

Program Crashes. This is the output:
http://pastebin.com/Fuku0pWu

What should have happened instead?

Program should not crash.

Please provide any additional information below.

Mailing list discussion: https://groups.google.com/forum/#!topic/golang-nuts/QDVgTGAeOUw

This bug was tested on Windows 8.1 and windows 8.0 with go1.3.3 and go1.3.0 64 bits.

The problem happens when the process_handle_t variable in the process struct is
different than zero. In the example code it uses a hardcoded value, a real value would
be obtained via an API Call to OpenProcess.

process_handle_t is a typedef for the windows HANDLE variable, which is defined as a
typedef to PVOID
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.4, os-windows.

@minux
Copy link
Member

minux commented Oct 11, 2014

Comment 2:

Actually, this is not specific to windows. It just happens that on Windows, HANDLE is
a void* type but it usually stores a small integer.
For example, this program will fail on unix (with tip, not 1.3.3, but I think the
underlying
reason is the same):
http://play.golang.org/p/IAddz4Q3ix
Pointer types from Cgo should be ignored by the GC (or conditionally ignored by GC
if it's invalid), because we forbid passing Go pointers to C (swig might be different,
but that's a different story.)

Labels changed: removed os-windows.

Status changed to Accepted.

@alexbrainman
Copy link
Member

Comment 3:

We had similar problem just a little while ago. https://golang.org/cl/152860043/
Alex

@minux
Copy link
Member

minux commented Oct 12, 2014

Comment 4:

yes, the windows problem is just a specific case of the general issue:
foreign pointers in Go structure.
I don't think we can label than separately as foreign pointers, so it
seems cmd/gc need to ignore them in pointer maps and then we
need a way for cgo to label certain fields as foreign pointers.

@randall77
Copy link
Contributor

Comment 5:

Foreign pointers in Go structures are fine.  As long as they are really pointers.  In
hopefully rare cases where they are ambiguous (sometimes pointers, sometimes not), we
can use uintptr.
 I'm not sure if cgo knows the distinction, though.

@minux
Copy link
Member

minux commented Oct 12, 2014

Comment 6:

yes, real foreign pointers are fine, but unfortunately, cgo doesn't know which type of
pointers might
contain integers.
We disallow cgo to have Go pointers, so perhaps we can just not scan the cgo pointers as
pointers,
as they are guaranteed to not contain pointer to Go heap.
We can add a few blacklist, for example, HANDLE, but that is still not 100% safe.
Or we can use the heuristics that void * are problematic pointers.

@rsc
Copy link
Contributor

rsc commented Oct 28, 2014

Comment 7:

There is no plausible way to start enumerating the 'non-pointer' pointer types.
Don't use fake pointer types. Use uintptr instead of HANDLE, for example.

@rsc
Copy link
Contributor

rsc commented Oct 28, 2014

Comment 8:

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Author

Comment 9 by marcovanotti15:

Hi,
I don't think it is working as intended: as far as I know having fake pointers is valid
in C and it is widely used in windows API. IMO it shouldn't be intended to crash.
I think it should be a Won't Fix + a note in the cgo documentation saying that fake
pointers are not allowed. At least for me it wasn't obvious that the problem was that
HANDLE was a fake pointer and that it wasn't allowed in Go.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants