--
---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Has anything actually changed?
I thought we were just being lazy/lucky before.
I don't believe much has actually changed. Pointers should have been kept in unsafe.Pointers - not uintptrs - always.
There is an outstanding question about how you allow simple pointer arithmetic, which today necessarily requires a conversion to uintptr and back. The answer today is that you have to do it "quickly". Within a single statement is certainly fine and likely to remain so. Splitting the work across multiple statements but no function calls is probably fine but may not remain so. Splitting the work across function calls is not fine and likely to break very soon.
--
I don't believe much has actually changed. Pointers should have been kept in unsafe.Pointers - not uintptrs - always. There is an outstanding question about how you allow simple pointer arithmetic, which today necessarily requires a conversion to uintptr and back. The answer today is that you have to do it "quickly". Within a single statement is certainly fine and likely to remain so. Splitting the work across multiple statements but no function calls is probably fine but may not remain so. Splitting the work across function calls is not fine and likely to break very soon.
Russ
The sketchy part about the exec_linux.go change is that we pass a uintptr to RawSyscall instead of an unsafe.Pointer. But that API is not easily changed at this point, and in practice, since neither Syscall nor RawSyscall will be interrupted for a garbage collection, the uintptr is okay.Russ
Thanks, I think I understand now.It is ok to convert from unsafe.Pointer to uintptr as long as the original unsafe.Pointer value is live in a stack frame.
Russ, Dmitry,On Wed, Jan 22, 2014 at 1:36 PM, Russ Cox <r...@golang.org> wrote:
On Wed, Jan 22, 2014 at 4:32 PM, Dave Cheney <da...@cheney.net> wrote:Thanks, I think I understand now.It is ok to convert from unsafe.Pointer to uintptr as long as the original unsafe.Pointer value is live in a stack frame.That's true today.But if you call an arbitrary function, then it may not be safe in Go 1.3, because copying stacks will need to adjust all the pointers, and it will not find the ones stored in uintptrs. That's why I said that if you kept everything within a single line you were safe but otherwise probably not.The exec_linux.go change is okay because RawSyscall and Syscall are effectively a special case: we know they never grow the stack nor trigger garbage collection.Similarly, calling any assembly function that's NOSPLIT should be okay, right? I was initially worried about syscall_linux_386.go's use of socketcall, but then noticed it's TEXT ·socketcall(SB),NOSPLIT,$0-40.
We currently have to preempt all running goroutines to do a GC, and it can't preempt if there's no preemption point. (currently just function entries with stack checks)But once it's then _in_ a system call, we can still do a GC. (only the return from system call will wait for the GC to be complete)And once we're in a system call, are the arguments to that system call considered roots?Concretely, is this program safe:?Note that the syscall.Read function is only:func Read(fd int, p []byte) (n int, err error) {return read(fd, p)}func read(fd int, p []byte) (n int, err error) {
var _p0 unsafe.Pointerif len(p) > 0 {_p0 = unsafe.Pointer(&p[0])} else {_p0 = unsafe.Pointer(&_zero)}r0, _, e1 := Syscall(SYS_READ, uintptr(fd), uintptr(_p0), uintptr(len(p)))n = int(r0)if e1 != 0 {err = e1}return}Once that uintptr(_p0) conversion happens, _p0 is no longer live, and there's a goroutine with that address stuck in a blocking (Read on a pipe) system call while we do a GC.Can you clarify whether system call args are explicit GC roots?
Arguments to assembly functions are treated conservatively, because they have no type information, so the arguments to Syscall will be treated as roots for the collection. This applies regardless of whether the argument is or is not actually a pointer, so the collector will treat fd, _p0, and len(p) all as potential roots. We have no plans to change this.
On Jan 22, 2014 4:05 PM, "Keith Randall" <k...@google.com> wrote:
>
> We should add an Offset method to unsafe.Pointer. And we should think about deprecating uintptr<->unsafe.Pointer conversions. I don't think we should get rid of them for 1.3, but it would be prudent to deprecate them now so people have some warning if we decide on some curtailment for 1.4 or beyond.
>
> I've seen the use of unsafe.Pointer -> uintptr conversions to check the alignment of the pointer. We could define another method for that, or force people to pass around a base+offset representation.
I also think we shouldn't add method to builtin types, how about make certain arithmetic operations allowed on unsafe.Pointer?
unsafe.Pointer + uintptr = unsafe.Pointer
unsafe.Pointer - unsafe.Pointer = uintptr
if that sounds too magic, I'm also Ok with adding unsafe.Add, Sub function for them.
Using unsafe.Pointer already marks the code unsafe, so disallowing simple and reasonable pointer arithmetic doesn't make much sense.
If we want to do this, we probably need a corresponding gofix module that could fix the common cases and warn the other cases. And golint should also warn about potentially GC-unsafe code.
But if you call an arbitrary function, then it may not be safe in Go 1.3, because copying stacks will need to adjust all the pointers, and it will not find the ones stored in uintptrs. That's why I said that if you kept everything within a single line you were safe but otherwise probably not.
On Thursday, 23 January 2014 08:36:46 UTC+11, rsc wrote:But if you call an arbitrary function, then it may not be safe in Go 1.3, because copying stacks will need to adjust all the pointers, and it will not find the ones stored in uintptrs. That's why I said that if you kept everything within a single line you were safe but otherwise probably not.Then all these:func (p *Proc) Addr() uintptr {
func (p *Proc) Call(a ...uintptr) (r1, r2 uintptr, lastErr error) {
func (p *LazyProc) Addr() uintptr {
func (p *LazyProc) Call(a ...uintptr) (r1, r2 uintptr, lastErr error) {in windows syscall will be broken.
Not necessarily. ...
As far as I understand, there is another tricky aspect in play here.
syscall.Syscall(SYS_FOO, uintptr(unsafe.Pointer(foo())),
uintptr(unsafe.Pointer(bar())))
Potentially the object returned by foo() could have been freed when we
execute bar().
Yes. Don't do that.
You could do something like https://codereview.appspot.com/52110044,
Moving GC will break this. ...
Moving GC will require some form of pinning of objects, or a special
persistent allocator for such objects.