-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
spec: document safe use of unsafe.Pointer and uintptr conversions #8994
Labels
Milestone
Comments
I'm interested in what people think about the test program below (similar to what I described in issue #7192, comment 12). "go vet" is happy with it, yet "go run" reliably panics with "not 42". Is "go vet" at fault for not warning about this? Is cmd/gc at fault for not ensuring that spilled (reflect.Value).Pointer() values are tracked as pointers? Or is the developer at fault for writing something tricky that "obviously" isn't supported? This particular use case is fabricated, but I've seen real world code like "if v1.Pointer() == v2.Pointer()" that I suspect could spuriously trigger similar failures if a concurrent goroutine triggers a stop-the-world GC run that interrupts v2.Pointer(). package main import ( "reflect" "runtime" "unsafe" ) type array [1000]int func arrayPointer() reflect.Value { return reflect.ValueOf(&array{42}) } func zero() int { runtime.GC() return 0 } func main() { for i := 0; i < 20; i++ { if (*array)(unsafe.Pointer(arrayPointer().Pointer()))[zero()] != 42 { panic("not 42") } } } |
That's a nice example. The informal rules seem to be something like "you can only convert unsafe.Pointer to uintptr if you convert back in the same expression." That informal rule makes it impossible to convert the result of reflect.Value.Pointer back to unsafe.Pointer, which makes the method somewhat useless. |
Rob: you changed the summary to say "spec", but issue #7192 and https://golang.org/cl/153110044 seemed to argue that any detailed restrictions should not be in the spec. |
CL https://golang.org/cl/18640 mentions this issue. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: