cmd/cgo: should make pointers passed to C functions escape

[jtolio]

Assume you have some package foo that uses a C function:

package foo

/*
typedef struct Buf {
} Buf;

void Foo(Buf* buf) {
}
*/
import "C"

func Foo() {
    var buf C.Buf
    C.Foo(&buf)
}

If you compile this package with Go 1.2 (what we use, until we can do some big audit of our cgo usage), we get this:

andrew@andrew-cb:~/escapeme/src/foo$ go version
go version go1.2 linux/amd64
andrew@andrew-cb:~/escapeme/src/foo$ go build -gcflags "-m -l -N"
# foo
./foo.go:14: moved to heap: buf

This is what we'd expect. Since the heap is (currently) a safe place to put Go-allocated memory referenced by C, we'd expect passing &buf to C.Foo to indicate buf should be heap-allocated.

On Go 1.4, we get this:

andrew@andrew-cb:~/escapeme/src/foo$ go version
go version go1.4.2 linux/amd64
andrew@andrew-cb:~/escapeme/src/foo$ go build -gcflags "-m -l -N"
# foo
/tmp/go-build929685257/foo/_obj/_cgo_gotypes.go:12: leaking param: ptr to result ~r1
:10[/tmp/go-build929685257/foo/_obj/_cgo_gotypes.go:27]: _Cfunc_Foo p0 does not escape
:11[/tmp/go-build929685257/foo/_obj/_cgo_gotypes.go:28]: _Cfunc_Foo &p0 does not escape
./foo.go:15: Foo &buf does not escape
andrew@andrew-cb:~/escapeme/src/foo$

Foo &buf does not escape? It totally should! Since it didn't get escaped, we're now passing stack-allocated memory into C. Holy cow that's a bad idea with 1.4, cause stack allocated memory gets moved around.

And here's tip:

andrew@andrew-cb:~/escapeme/src/foo$ go version
go version devel +f8fd550 Tue Mar 31 13:56:18 2015 +0000 linux/amd64
andrew@andrew-cb:~/escapeme/src/foo$ go build -gcflags "-m -l -N"
# foo
/tmp/go-build207622607/foo/_obj/_cgo_gotypes.go:14: leaking param: ptr to result ~r1
:19[/tmp/go-build207622607/foo/_obj/_cgo_gotypes.go:38]: _Cfunc_Foo p0 does not escape
:20[/tmp/go-build207622607/foo/_obj/_cgo_gotypes.go:39]: _Cfunc_Foo &p0 does not escape
./foo.go:15: Foo &buf does not escape
andrew@andrew-cb:~/escapeme/src/foo$ 

[randall77]

@dr2chase

[minux]

well, this is not a escape analysis bug per-se. For example, the example function call C.Foo(&buf) in the issue report gets translated to a call to this func: func _Cfunc_Foo(p0 *_Ctype_struct_Buf) (r1 _Ctype_void) { _cgo_runtime_cgocall_errno(_cgo_0b9ef0be51ea_Cfunc_Foo, uintptr(unsafe.Pointer(&p0))) return } And you can see the p0 really is not escaping. We should add something like: use(unsafe.Pointer(p0)) where use is defined in assembly as a NOP, and have the prototype: func use(unsafe.Pointer) Just like what we did for the syscall package. The reason I think it is not done is that we still are not clear about what kind of guarantee we want to provide for passing Go pointers to C code. In the pass we talked about always making a copy, but apparently that proposal is dismissed after hearing severe opposing voice from the community.

[dr2chase]

Note that what Minux describes is a change from previous behavior, and escape analysis is "tricked" because now there is a "Go function body" that used to be written in C (hence, opaque to escape analysis).

Speaking as someone new to Go and with a pathological imagination about how optimizers can jerk you around, I'd be a little worried that (future) better dead code elimination might disappear all the Go-side pointers and make the underlying object eligible for GC. In addition, if anyone on the C side gets the idea that they can mutate pointers in objects on the Go heap, sadness and woe might ensue (in a crashy and Heisenbuggy way).

[jtolio]

I was actually wondering about that too when a NOP function was mentioned but I know very little about Go internals.

FWIW, I frequently use cgo libraries that mutate Go-allocated memory.

[minux]

The NOP function ("use") I mentioned will be defined in assembly, so Go compiler won't know anything about it except its signature. This suffices to "trick" the compiler into believing the argument for it always escapes. See http://golang.org/src/syscall/zsyscall_linux_amd64.go#L17, http://golang.org/src/syscall/syscall.go#L92 and http://golang.org/src/syscall/asm.s for an example.

[dvyukov]

Go-allocated objects should not be used by C outside of the cgo call (i.e. remembered for future use). So allocating them on stack is OK and beneficial, we do this for syscalls as well.
If a C function remembers an object for future use, then it must be C allocated.
I would say WAI.

[jtolio]

Whether or not I agree with that argument (I'm definitely an "opposing voice in the community" when it comes to the current GC plans wrt heap data, see https://groups.google.com/forum/#!msg/golang-dev/JO99bwdOICs/oV3p-vaBbeEJ or https://groups.google.com/forum/#!msg/golang-dev/qvOqcmAkKnA/CcdW-ogVgrYJ), we are definitely getting corruption due to this issue for cgo code that is not remembering the passed-in pointer somewhere after it returns.

If I recall correctly, certain small syscalls don't have to do as much bookkeeping and setup as cgo?

Either way, for a C function that was purely operating on the passed-in pointer and not remembering it, the pointer became invalid due to stack-moving, in our experience.

[dvyukov]

we are definitely getting corruption due to this issue for cgo code that is not remembering the passed-in pointer somewhere after it returns.

that's kinda an important detail worth noting in the issue description
I guess there is a bug in cgo wrappers, they remember a pointer as uintptr and then grow stack making the pointer invalid. That can be fixed without promoting the objects to heap. Just as we do for syscalls.

[jtolio]

You're right, I apologize for leaving it out. The reason we discovered the unexpected difference in escape analysis is due to infrequent corruption in one of our cgo libraries. We were able to prove that when the corruption happened, the correct pointer address had changed in Go-land.

Also, on further thought, I actually think requiring that C libraries don't save any Go pointers is probably a fine rule, even given the concerns about passing Go-allocated byte-slices to C. Update: on even further thought, I think not being able to remember Go pointers in C makes many callback-based C libraries unusable.

[minux]

On Wed, Apr 1, 2015 at 2:04 AM, Dmitry Vyukov notifications@github.com
wrote:

we are definitely getting corruption due to this issue for cgo code that
is not remembering the passed-in pointer somewhere after it returns.

that's kinda an important detail worth noting in the issue description
I guess there is a bug in cgo wrappers, they remember a pointer as uintptr
and then grow stack making the pointer invalid. That can be fixed without
promoting the objects to heap. Just as we do for syscalls.

I think the way we did for syscall will move pointers argument to heap, no?

[dvyukov]

Syscalls also accept uintptr's, so they should not promote pointers to heap.