converting unsafe.Pointers to uintptr

741 views
Skip to first unread message

Dave Cheney

unread,
Jan 22, 2014, 3:31:29 PM1/22/14
to golang-dev
Hello,

I have been watching the flurry of CLs related to removing any 'smuggling' of unsafe.Pointers as uintptrs.

As this practice was very common in the std library and is now being removed because it causes the precise GC to fault, should the compiler be altered to reject this conversion if it can no longer be done safely ?

Cheers

Dave

Robert Griesemer

unread,
Jan 22, 2014, 3:37:35 PM1/22/14
to Dave Cheney, golang-dev
It's probably worthwhile determining exactly what operations will continue to be ok given a precise GC. And possibly update the language accordingly. Feel free to file an issue and assign it to me.
- gri


--
 
---
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.

Brad Fitzpatrick

unread,
Jan 22, 2014, 3:44:06 PM1/22/14
to Robert Griesemer, Dave Cheney, golan...@googlegroups.com

Has anything actually changed?

I thought we were just being lazy/lucky before.

Russ Cox

unread,
Jan 22, 2014, 3:46:01 PM1/22/14
to Brad Fitzpatrick, Robert Griesemer, Dave Cheney, golang-dev
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

Robert Griesemer

unread,
Jan 22, 2014, 3:50:24 PM1/22/14
to Russ Cox, Brad Fitzpatrick, Dave Cheney, golang-dev
We still may want to prohibit assigning pointers to uintptrs.

Dave Cheney

unread,
Jan 22, 2014, 3:59:13 PM1/22/14
to Russ Cox, Brad Fitzpatrick, Robert Griesemer, golang-dev
Thanks for your comments Russ and Brad.

On Thu, Jan 23, 2014 at 7:46 AM, Russ Cox <r...@golang.org> wrote:
I don't believe much has actually changed. Pointers should have been kept in unsafe.Pointers - not uintptrs - always.

This is the bit that worries me, we've gotten away with this for more than 4 years and a very large body of existing code has used this pattern up to this week without incident. 

It may be that we were always lucky, and some very infrequent crashes could have been attributed to this underlying issue but were not understood at the time.

Now, in a Go 1.3 world, this pattern is known to cause the GC to loose track of pointers leading to memory corruption and I fear a long period of adjustment is necessary for authors of low level Go code. 

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 think this must, at a minimum, be mentioned in the 1.3 release notes. 

It would be much better if the compiler could identify these cases and reject the code.

I'd argue if this pattern was always an error then there is scope with the 'bugfixes' clause of the Go 1 contract to reject code which previously compiled under 1.2 and earlier, just as we did with nil change in 1.2.

Keith Randall

unread,
Jan 22, 2014, 4:05:10 PM1/22/14
to Dave Cheney, Russ Cox, Brad Fitzpatrick, Robert Griesemer, golang-dev
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.



--

Gustavo Niemeyer

unread,
Jan 22, 2014, 4:14:45 PM1/22/14
to Keith Randall, Dave Cheney, Russ Cox, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 7: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.

Add and Sub, with semantics similar to time.Time, would cover other
cases in use.

> 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.

ptr.Sub(zero) % alignment


gustavo @ http://niemeyer.net

Dave Cheney

unread,
Jan 22, 2014, 4:15:45 PM1/22/14
to Russ Cox, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Thu, Jan 23, 2014 at 7:46 AM, Russ Cox <r...@golang.org> wrote:
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.

Sorry to bang the drum here, but if I understand the advice here, this fix


May not be correct.
 
Russ

Russ Cox

unread,
Jan 22, 2014, 4:23:58 PM1/22/14
to Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
We have not yet added methods to any built-in type. I am reluctant to break that precedent for such a corner case. I'd really rather not add methods to unsafe.Pointer.

Russ

Gustavo Niemeyer

unread,
Jan 22, 2014, 4:25:07 PM1/22/14
to Dave Cheney, Russ Cox, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 7:15 PM, Dave Cheney <da...@cheney.net> wrote:
> Sorry to bang the drum here, but if I understand the advice here, this fix
>
> https://codereview.appspot.com/55520043/diff/30001/src/pkg/syscall/exec_linux.go
>
> May not be correct.

This looks fine to me. The address being referenced is being held in
memory by the unsafe.Pointer and by the actual Go value for the
duration of the system call.


gustavo @ http://niemeyer.net

Russ Cox

unread,
Jan 22, 2014, 4:26:42 PM1/22/14
to Gustavo Niemeyer, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
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

Keith Randall

unread,
Jan 22, 2014, 4:28:21 PM1/22/14
to Russ Cox, Gustavo Niemeyer, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
If you don't like Offset you could define + to do the right thing for unsafe.Pointer/uintptr args.  unsafe.Pointer already has special semantics for casts.




On Wed, Jan 22, 2014 at 1:26 PM, Russ Cox <r...@golang.org> wrote:
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

Dave Cheney

unread,
Jan 22, 2014, 4:32:24 PM1/22/14
to Russ Cox, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer, golang-dev
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. Doing the conversion like

   v := uintptr(unsafe.Pointer(&someslice[0])) 

is not safe because of the way that all those conversions collapse down to v := uintptr(&someslice[0]) (sorry for the terrible description)

This is really subtle stuff and the reward for getting this wrong is memory corruption.

Gustavo Niemeyer

unread,
Jan 22, 2014, 4:35:00 PM1/22/14
to Keith Randall, Russ Cox, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 7:28 PM, Keith Randall <k...@google.com> wrote:
> If you don't like Offset you could define + to do the right thing for
> unsafe.Pointer/uintptr args. unsafe.Pointer already has special semantics
> for casts.

That's a nice idea. Would make dealing with simple pointer arithmetic
slightly less painful in terms of syntax, when integrating with C code.
If we do that, though, we may as well support ints rather than uintptr,
and properly handle minus and negative offsets.


gustavo @ http://niemeyer.net

Russ Cox

unread,
Jan 22, 2014, 4:36:46 PM1/22/14
to Dave Cheney, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer, golang-dev
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.

Russ


Gustavo Niemeyer

unread,
Jan 22, 2014, 4:43:24 PM1/22/14
to Russ Cox, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 7:36 PM, Russ Cox <r...@golang.org> 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.

Oh, I see. Does that apply to cgo function calls as well? If not, any
C code that references GC-allocated values for the duration of the cgo
call will need to be fixed.


gustavo @ http://niemeyer.net

Russ Cox

unread,
Jan 22, 2014, 4:57:15 PM1/22/14
to Gustavo Niemeyer, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
If you are writing a cgo function you should arrange that it takes a real pointer (even an unsafe.Pointer would be fine) and not a uintptr, and then everything is okay. If you declare var x int and pass &x to cgo, x will end up on the heap, not on the stack.

Russ

Brad Fitzpatrick

unread,
Jan 22, 2014, 5:04:24 PM1/22/14
to Russ Cox, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
Russ, Dmitry,


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.Pointer
        if 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?


Russ Cox

unread,
Jan 22, 2014, 5:13:00 PM1/22/14
to Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 5:04 PM, Brad Fitzpatrick <brad...@golang.org> wrote:
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.

Yes, the NOSPLIT is what makes RawSyscall and Syscall special. It applies to socketcall too.
 
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.Pointer
        if 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.

Russ

Brad Fitzpatrick

unread,
Jan 22, 2014, 5:14:31 PM1/22/14
to Russ Cox, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 2:13 PM, Russ Cox <r...@golang.org> wrote:

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.

Perfect.

minux

unread,
Jan 22, 2014, 5:30:05 PM1/22/14
to Keith Randall, Brad Fitzpatrick, Russ Cox, golang-dev, Robert Griesemer, Dave Cheney


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.

brainman

unread,
Jan 22, 2014, 8:00:01 PM1/22/14
to golan...@googlegroups.com, Dave Cheney, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer
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.

Alex

Gustavo Niemeyer

unread,
Jan 22, 2014, 9:46:08 PM1/22/14
to Russ Cox, Dave Cheney, Brad Fitzpatrick, Robert Griesemer, golang-dev
On Wed, Jan 22, 2014 at 7:57 PM, Russ Cox <r...@golang.org> wrote:
> If you are writing a cgo function you should arrange that it takes a real
> pointer (even an unsafe.Pointer would be fine) and not a uintptr, and then
> everything is okay. If you declare var x int and pass &x to cgo, x will end
> up on the heap, not on the stack.

Understood. Parameters are unsafe.Pointers already, so it's all good.

Thanks,


gustavo @ http://niemeyer.net

minux

unread,
Jan 23, 2014, 1:10:20 AM1/23/14
to brainman, golang-dev, Dave Cheney, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer
On Wed, Jan 22, 2014 at 8:00 PM, brainman <alex.b...@gmail.com> wrote:
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 {
addr is the address of static code (i.e. not heap allocated), so it won't be collected, this one is safe.
the caller must have p on stack, so p won't be collected also.
func (p *Proc) Call(a ...uintptr) (r1, r2 uintptr, lastErr error) {
this one and (*LazyProc).Call something we need to worry about.
func (p *LazyProc) Addr() uintptr {
similar to Addr() 
func (p *LazyProc) Call(a ...uintptr) (r1, r2 uintptr, lastErr error) {

in windows syscall will be broken.
 
Not necessarily. If the caller still retains at least one reference to the object whose address
has been passed to these will avoid the object being garbage collected.

Unless the user do this:
r1, r2, err := p.Call(uintptr(unsafe.Pointer(&SomeStruct{})))

And this might only happen when &SomeStruct{} is an input to the syscall. How often does
this kind of situation arise in practice? I think the compiler (or golint) could definitely do something
here (that line of code is dangerous because &SomeStruct might be deallocated even before
entering the p.Call, so the compiler can reject such code, and in general, it could also reject code
that stores a freshly allocated pointer only as a uintptr into a variable)

brainman

unread,
Jan 23, 2014, 1:17:41 AM1/23/14
to golan...@googlegroups.com, brainman, Dave Cheney, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer
On Thursday, 23 January 2014 17:10:20 UTC+11, minux wrote:
 
Not necessarily. ...

I will believe you. :-) Still, it is bit unsettling.

Alex

minux

unread,
Jan 23, 2014, 1:28:22 AM1/23/14
to brainman, golang-dev, Dave Cheney, Gustavo Niemeyer, Brad Fitzpatrick, Robert Griesemer
I also have that feeling. That's why I proposed that we do something
to encourage the usage of unsafe.Pointer over uintptr.

I think the way to do that could be to either:
1. allow some limited kind of arithmetic operations on unsafe.Pointer
(this will hurt orthogonality and introduce special case to the type
system, but IMO this is the way to persuade people not to use uintptr
to hold a pointer),
2. provide helper functions in unsafe package and somehow convince
people to use them extensively when trying to do pointer arithmetic.

Dmitry Vyukov

unread,
Jan 23, 2014, 5:41:34 AM1/23/14
to Russ Cox, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
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().
It seems to work because of the way we generate temps for such statements.

Russ, are all such variations 100% safe and under control?
E.g
syscall.Syscall(SYS_FOO, uintptr(unsafe.Pointer(foo())),
uintptr(unsafe.Pointer(<-c)), uintptr(unsafe.Pointer(bar())))
?

Russ Cox

unread,
Jan 23, 2014, 3:15:16 PM1/23/14
to Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
On Thu, Jan 23, 2014 at 5:41 AM, Dmitry Vyukov <dvy...@google.com> wrote:
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.

Russ 

roger peppe

unread,
Jan 24, 2014, 5:10:25 AM1/24/14
to Russ Cox, Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer, golang-dev
FWIW I'm not keen on changing the type checking semantics
to allow (unsafe.Pointer + uintptr). It seems like we'd be
breaking a nicely universal type-system invariant (named
types must be identical for +, -, /, *, etc) for the sake of a
niche convenience.

I personally wouldn't mind methods on unsafe.Pointer (it might be built
in to the language, but because it's imported normally from
a package, rather than being part of the universe scope, it doesn't
"feel" like it is).

Assuming that's not acceptable, we could add some functions
instead:

package unsafe

func Add(Pointer, uintptr) Pointer
func Sub(Pointer, Pointer) uintptr

alternatively:

func Add(Pointer, int) Pointer

so you can more easily get negative offsets,
but that breaks the symmetry with Sub.

Andy Balholm

unread,
Jan 24, 2014, 11:03:37 AM1/24/14
to golan...@googlegroups.com, Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
On Thursday, January 23, 2014 12:15:16 PM UTC-8, rsc wrote:
Yes. Don't do that.

What about this syscall function that I wrote (in code.google.com/p/com-and-go)? It doesn't include any explicit conversions from unsafe.Pointer to uintptr, but I suspect that it would break if there were a compacting garbage collector. If I used a slice of unsafe.Pointer for the args instead of a slice of uintptr, then pointers would get relocated correctly. But what about ints that look like pointers? Would they end up getting relocated then?

syscall.go:

package com

import (
"unsafe"
)

// A winCall structure represents a call to a Windows DLL function.
type winCall struct {
fn          uintptr
n           uintptr
args        *uintptr
r1, r2, err uintptr
}

// An iface structure is the same as the memory representation of an
// interface{} value.
type iface struct {
typ *struct {
size uintptr
// other fields, defined in reflect.rtype but not needed here
}
data uintptr
}

// Syscall calls a Windows DLL function.
func Syscall(fn uintptr, args ...interface{}) (r1, r2, err uintptr) {
// Copy all the args as uintptr values.
u := make([]uintptr, 0, len(args))
for _, v := range args {
ifHeader := *(*iface)(unsafe.Pointer(&v))
s := int(ifHeader.typ.size)
if s <= int(unsafe.Sizeof(uintptr(0))) {
// The data is stored directly in the interface header.
u = append(u, ifHeader.data)
} else {
// The interface header holds a pointer to the data.
p := ifHeader.data
for s > 0 {
u = append(u, *(*uintptr)(unsafe.Pointer(p)))
p += unsafe.Sizeof(uintptr(0))
s -= int(unsafe.Sizeof(uintptr(0)))
}
}
}

c := winCall{
fn: fn,
n:  uintptr(len(u)),
}
if len(u) > 0 {
c.args = &u[0]
}
cSyscall(&c)
return c.r1, c.r2, c.err
}

func cSyscall(c *winCall)

-----------------------------------------------------------------
syscall.c:

#include <runtime.h>
#include <cgocall.h>
void runtime·asmstdcall(void *c);

void ·cSyscall(WinCall *c) {
runtime·cgocall(runtime·asmstdcall, c);
}

Keith Randall

unread,
Jan 24, 2014, 4:04:38 PM1/24/14
to Andy Balholm, golang-dev, Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
You never do unsafe.Pointer -> uintptr explicitly, but you do it implicitly here
    u = append(u, ifHeader.data)
and here
  u = append(u, *(*uintptr)(unsafe.Pointer(p)))

If any of the args you are passing are pointers (or contain pointers), you've just converted those pointers to uintptrs.

Right now, we have a one-sided error, where storing a pointer in a uintptr is bad (the referent may get garbage collected) but storing an integer in an unsafe.Pointer is not too bad (GC gets imprecise: some objects which should be collected are not).

With a copying collector, both directions are bad.  Integers stored in an unsafe.Pointer may now randomly change values.

With stack copying we're sticking a toe in that second world, but we have the escape valve that anything really tough can be punted (by not letting it live on copyable stacks - pushing it to the heap or to the M stack).


Andy Balholm

unread,
Jan 24, 2014, 5:15:36 PM1/24/14
to Keith Randall, golang-dev, Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer

On Jan 24, 2014, at 1:04 PM, Keith Randall <k...@google.com> wrote:

> You never do unsafe.Pointer -> uintptr explicitly, but you do it implicitly here
> u = append(u, ifHeader.data)
> and here
> u = append(u, *(*uintptr)(unsafe.Pointer(p)))
>
> If any of the args you are passing are pointers (or contain pointers), you've just converted those pointers to uintptrs.

I know.

> Right now, we have a one-sided error, where storing a pointer in a uintptr is bad (the referent may get garbage collected) but storing an integer in an unsafe.Pointer is not too bad (GC gets imprecise: some objects which should be collected are not).
>
> With a copying collector, both directions are bad. Integers stored in an unsafe.Pointer may now randomly change values.
>
> With stack copying we're sticking a toe in that second world, but we have the escape valve that anything really tough can be punted (by not letting it live on copyable stacks - pushing it to the heap or to the M stack).

So how should something like this be handled?

Keith Randall

unread,
Jan 24, 2014, 5:33:11 PM1/24/14
to Andy Balholm, golang-dev, Dmitry Vyukov, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
You could do something like https://codereview.appspot.com/52110044, where I build a separate reflect.Type for each function signature and use that type to hold the args.

If there aren't too many variants, you could hardcode the different possible signatures.

Brad Fitzpatrick

unread,
Jan 24, 2014, 5:49:15 PM1/24/14
to Keith Randall, Andy Balholm, golang-dev, Dmitry Vyukov, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
On Fri, Jan 24, 2014 at 2:33 PM, Keith Randall <k...@google.com> wrote:
You could do something like https://codereview.appspot.com/52110044,

Incidentally, that CL introduced reflect.funcLayout which I see in recent builder crashes, like:



brainman

unread,
Feb 7, 2014, 12:39:41 AM2/7/14
to golan...@googlegroups.com, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
What about this call to WSARecv


o.buf parameter holds buffer (allocated by Go). Once WSARecv returns, Windows expects that buffer to remain unmoved until IO is completed. At this moment all we have to do is keep all objects live until we are done. Wouldn't gc changes break that?

Alex

Dmitry Vyukov

unread,
Feb 7, 2014, 12:54:47 AM2/7/14
to brainman, golang-dev, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
What changes do you mean?
Moving GC will break this. Just as any similar cgo code, when you
expect that an object will be at the same location after cgo call
returns.
Moving GC will require some form of pinning of objects, or a special
persistent allocator for such objects.
I use persistent allocator in netpoll_epoll/kqueue, because once you
pass a pointer into these functions, it must be alive basically till
process termination.

brainman

unread,
Feb 7, 2014, 1:02:39 AM2/7/14
to golan...@googlegroups.com, brainman, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
On Friday, 7 February 2014 16:54:47 UTC+11, Dmitry Vyukov wrote:
Moving GC will break this. ...

That is what I meant. Also stacks that are copied.

Moving GC will require some form of pinning of objects, or a special
persistent allocator for such objects.

Do you have any picture of how that might work?

Alex

Dmitry Vyukov

unread,
Feb 7, 2014, 2:46:04 AM2/7/14
to brainman, golang-dev, Brad Fitzpatrick, Dave Cheney, Gustavo Niemeyer, Robert Griesemer
On Fri, Feb 7, 2014 at 10:02 AM, brainman <alex.b...@gmail.com> wrote:
> On Friday, 7 February 2014 16:54:47 UTC+11, Dmitry Vyukov wrote:
>>
>> Moving GC will break this. ...
>
>
> That is what I meant. Also stacks that are copied.

As for stacks, the descriptor must be forced to be allocated on heap somehow.

>> Moving GC will require some form of pinning of objects, or a special
>> persistent allocator for such objects.
>
> Do you have any picture of how that might work?


I don't have a full picture, and I believe we are still very far away
from moving GC.
But C++/CLI (C++ binding for .NET) allows you do to something along
the lines of:

{
cli::pin_ptr<T> pin(ptr);
// in this scope ptr won't move
}

Also, since you are using syscall package, you can as well use
syscall.VirtualAlloc to get some persistent memory.

Gustavo Niemeyer

unread,
Mar 19, 2014, 12:32:56 PM3/19/14
to Dmitry Vyukov, brainman, golang-dev, Brad Fitzpatrick, Dave Cheney, Robert Griesemer, Russ Cox
As another data point on this conversation, I just used uintptr to
pass a C address in a safe way across different packages, without
exposing an unsafe.Pointer to everybody importing that package. This
should be okay even given a moving GC, since Go won't ever move these
C pointers around, and it's a pattern similar to the one used by the
reflect package, on Value.UnsafeAddr.

Both of these would have to change somehow if Go prevents uintptr =>
unsafe.Pointer at some point.
--

gustavo @ http://niemeyer.net

james4k

unread,
Mar 20, 2014, 4:10:38 PM3/20/14
to golan...@googlegroups.com, Dmitry Vyukov, brainman, Brad Fitzpatrick, Dave Cheney, Robert Griesemer, Russ Cox
Without understanding the problem very deeply, would it be possible to pass those around as *uint?

james4k

unread,
Mar 20, 2014, 4:14:32 PM3/20/14
to golan...@googlegroups.com, Dmitry Vyukov, brainman, Brad Fitzpatrick, Dave Cheney, Robert Griesemer, Russ Cox
Nevermind, my mind lumped that into a typed pointer for some reason. Silly suggestion.
Reply all
Reply to author
Forward
0 new messages