code review 139070043: cmd/cgo, runtime: write cgo stub wrappers in Go, not C

cmd/cgo, runtime: write cgo stub wrappers in Go, not C

[rsc, 2014-09-02 23:31:11]

Hello golang-codereviews@googlegroups.com (cc: dvyukov, iant, khr, r),

I'd like you to review this change to
https://code.google.com/p/go/

[brainman, 2014-09-03 00:00:47]

https://codereview.appspot.com/139070043/diff/40001/misc/cgo/test/callback.go
File misc/cgo/test/callback.go (right):

https://codereview.appspot.com/139070043/diff/40001/misc/cgo/test/callback.go...
misc/cgo/test/callback.go:22: "./backdoor"
Some missing files?

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64.s
File src/pkg/runtime/asm_amd64.s (right):

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64...
src/pkg/runtime/asm_amd64.s:769: TEXT runtime·asmcgocall_errno(SB),NOSPLIT,$0-20
s/-20/-24/

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64...
src/pkg/runtime/asm_amd64.s:769: TEXT runtime·asmcgocall_errno(SB),NOSPLIT,$0-20
No changes to asm_386.s and others?

[rsc, 2014-09-03 00:38:06]

PTAL

https://codereview.appspot.com/139070043/diff/40001/misc/cgo/test/callback.go
File misc/cgo/test/callback.go (right):

https://codereview.appspot.com/139070043/diff/40001/misc/cgo/test/callback.go...
misc/cgo/test/callback.go:22: "./backdoor"
On 2014/09/03 00:00:47, brainman wrote:
> Some missing files?

no, looks like gofmt just moved this import.

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64.s
File src/pkg/runtime/asm_amd64.s (right):

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64...
src/pkg/runtime/asm_amd64.s:769: TEXT runtime·asmcgocall_errno(SB),NOSPLIT,$0-20
On 2014/09/03 00:00:47, brainman wrote:
> s/-20/-24/

No, it's 20 = 8+8+4.
The locals above are 24 because stack frames have to be a multiple of 8.

https://codereview.appspot.com/139070043/diff/40001/src/pkg/runtime/asm_amd64...
src/pkg/runtime/asm_amd64.s:769: TEXT runtime·asmcgocall_errno(SB),NOSPLIT,$0-20
On 2014/09/03 00:00:47, brainman wrote:
> No changes to asm_386.s and others?

Thanks, added to CL.

[brainman, 2014-09-03 01:04:57]

LGTM

but you, probably, should wait for someone who knows more about cgo.

Alex

[iant, 2014-09-03 01:08:04]

LGTM

[rsc, 2014-09-03 15:36:17]

*** Submitted as https://code.google.com/p/go/source/detail?r=1bc626afe666 ***

cmd/cgo, runtime: write cgo stub wrappers in Go, not C

LGTM=alex.brainman, iant
R=golang-codereviews, alex.brainman, iant
CC=dvyukov, golang-codereviews, khr, r
https://codereview.appspot.com/139070043

[gobot, 2014-09-03 15:45:24]

This CL appears to have broken the linux-amd64-temp builder.
See http://build.golang.org/log/e8933eb15e41db3fc0690a291193cb947270113e