Skip to content

Commit f9ca3b5

Browse files
committed
runtime: scheduler, cgo reorganization
* Change use of m->g0 stack (aka scheduler stack). * Provide runtime.mcall(f) to invoke f() on m->g0 stack. * Replace scheduler loop entry with runtime.mcall(schedule). Runtime.mcall eliminates the need for fake scheduler states that exist just to run a bit of code on the m->g0 stack (Grecovery, Gstackalloc). The elimination of the scheduler as a loop that stops and starts using gosave and gogo fixes a bad interaction with the way cgo uses the m->g0 stack. Cgo runs external (gcc-compiled) C functions on that stack, and then when calling back into Go, it sets m->g0->sched.sp below the added call frames, so that other uses of m->g0's stack will not interfere with those frames. Unfortunately, gogo (longjmp) back to the scheduler loop at this point would end up running scheduler with the lower sp, which no longer points at a valid stack frame for a call to scheduler. If scheduler then wrote any function call arguments or local variables to where it expected the stack frame to be, it would overwrite other data on the stack. I realized this possibility while debugging a problem with calling complex Go code in a Go -> C -> Go cgo callback. This wasn't the bug I was looking for, it turns out, but I believe it is a real bug nonetheless. Switching to runtime.mcall, which only adds new frames to the stack and never jumps into functions running in existing ones, fixes this bug. * Move cgo-related code out of proc.c into cgocall.c. * Add very large comment describing cgo call sequences. * Simpilify, regularize cgo function implementations and names. * Add test suite as misc/cgo/test. Now the Go -> C path calls cgocall, which calls asmcgocall, and the C -> Go path calls cgocallback, which calls cgocallbackg. The shuffling, which affects mainly the callback case, moves most of the callback implementation to cgocallback running on the m->curg stack (not the m->g0 scheduler stack) and only while accounted for with $GOMAXPROCS (between calls to exitsyscall and entersyscall). The previous callback code did not block in startcgocallback's approximation to exitsyscall, so if, say, the garbage collector were running, it would still barge in and start doing things like call malloc. Similarly endcgocallback's approximation of entersyscall did not call matchmg to kick off new OS threads when necessary, which caused the bug in issue 1560. Fixes #1560. R=iant CC=golang-dev https://golang.org/cl/4253054
1 parent 6d6f338 commit f9ca3b5

21 files changed

+907
-419
lines changed

misc/cgo/stdio/Makefile

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@ include ../../../src/Make.inc
66

77
TARG=stdio
88
CGOFILES=\
9-
align.go\
109
file.go\
11-
test.go\
12-
test1.go\
1310

1411
CLEANFILES+=hello fib chain run.out
1512

misc/cgo/stdio/hello.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,8 @@
44

55
package main
66

7-
import (
8-
"os"
9-
"stdio"
10-
)
7+
import "stdio"
118

129
func main() {
1310
stdio.Stdout.WriteString(stdio.Greeting + "\n")
14-
15-
l := stdio.Atol("123")
16-
if l != 123 {
17-
println("Atol 123: ", l)
18-
panic("bad atol")
19-
}
20-
21-
n, err := stdio.Strtol("asdf", 123)
22-
if n != 0 || err != os.EINVAL {
23-
println("Strtol: ", n, err)
24-
panic("bad atoi2")
25-
}
26-
27-
stdio.TestAlign()
28-
stdio.TestEnum()
2911
}

misc/cgo/test/Makefile

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Copyright 2011 The Go Authors. All rights reserved.
2+
# Use of this source code is governed by a BSD-style
3+
# license that can be found in the LICENSE file.
4+
5+
include ../../../src/Make.inc
6+
7+
TARG=runtime/cgotest
8+
9+
CGOFILES=\
10+
align.go\
11+
basic.go\
12+
callback.go\
13+
issue1222.go\
14+
issue1328.go\
15+
issue1560.go\
16+
17+
CGO_OFILES=\
18+
callback_c.o\
19+
20+
OFILES=\
21+
runtime.$O\
22+
23+
include ../../../src/Make.pkg

misc/cgo/stdio/align.go renamed to misc/cgo/test/align.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package stdio
1+
package cgotest
22

33
/*
44
#include <stdio.h>
@@ -55,24 +55,18 @@ void cTest(SDL_KeyboardEvent *event) {
5555
import "C"
5656

5757
import (
58-
"fmt"
59-
"syscall"
58+
"testing"
6059
)
6160

62-
func TestAlign() {
63-
if syscall.ARCH == "amd64" {
64-
// alignment is known to be broken on amd64.
65-
// http://code.google.com/p/go/issues/detail?id=609
66-
return
67-
}
61+
func TestAlign(t *testing.T) {
6862
var evt C.SDL_KeyboardEvent
6963
C.makeEvent(&evt)
7064
if C.same(&evt, evt.typ, evt.which, evt.state, evt.keysym.scancode, evt.keysym.sym, evt.keysym.mod, evt.keysym.unicode) == 0 {
71-
fmt.Println("*** bad alignment")
65+
t.Error("*** bad alignment")
7266
C.cTest(&evt)
73-
fmt.Printf("Go: %#x %#x %#x %#x %#x %#x %#x\n",
67+
t.Errorf("Go: %#x %#x %#x %#x %#x %#x %#x\n",
7468
evt.typ, evt.which, evt.state, evt.keysym.scancode,
7569
evt.keysym.sym, evt.keysym.mod, evt.keysym.unicode)
76-
fmt.Println(evt)
70+
t.Error(evt)
7771
}
7872
}

misc/cgo/stdio/test.go renamed to misc/cgo/test/basic.go

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
// This file contains test cases for cgo.
5+
// Basic test cases for cgo.
66

7-
package stdio
7+
package cgotest
88

99
/*
1010
#include <stdio.h>
@@ -52,6 +52,7 @@ struct ibv_context {
5252
import "C"
5353
import (
5454
"os"
55+
"testing"
5556
"unsafe"
5657
)
5758

@@ -89,38 +90,35 @@ func Atol(s string) int {
8990
return int(n)
9091
}
9192

92-
func TestConst() {
93+
func TestConst(t *testing.T) {
9394
C.myConstFunc(nil, 0, nil)
9495
}
9596

96-
func TestEnum() {
97+
func TestEnum(t *testing.T) {
9798
if C.Enum1 != 1 || C.Enum2 != 2 {
98-
println("bad enum", C.Enum1, C.Enum2)
99+
t.Error("bad enum", C.Enum1, C.Enum2)
99100
}
100101
}
101102

102-
func TestAtol() {
103+
func TestAtol(t *testing.T) {
103104
l := Atol("123")
104105
if l != 123 {
105-
println("Atol 123: ", l)
106-
panic("bad atol")
106+
t.Error("Atol 123: ", l)
107107
}
108108
}
109109

110-
func TestErrno() {
110+
func TestErrno(t *testing.T) {
111111
n, err := Strtol("asdf", 123)
112112
if n != 0 || err != os.EINVAL {
113-
println("Strtol: ", n, err)
114-
panic("bad strtol")
113+
t.Error("Strtol: ", n, err)
115114
}
116115
}
117116

118-
func TestMultipleAssign() {
119-
p := C.CString("123")
117+
func TestMultipleAssign(t *testing.T) {
118+
p := C.CString("234")
120119
n, m := C.strtol(p, nil, 345), C.strtol(p, nil, 10)
121120
if n != 0 || m != 234 {
122-
println("Strtol x2: ", n, m)
123-
panic("bad strtol x2")
121+
t.Fatal("Strtol x2: ", n, m)
124122
}
125123
C.free(unsafe.Pointer(p))
126124
}
@@ -134,11 +132,3 @@ var (
134132
type Context struct {
135133
ctx *C.struct_ibv_context
136134
}
137-
138-
func Test() {
139-
TestAlign()
140-
TestAtol()
141-
TestEnum()
142-
TestErrno()
143-
TestConst()
144-
}

misc/cgo/test/callback.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
package cgotest
2+
3+
/*
4+
void callback(void *f);
5+
void callGoFoo(void) {
6+
extern void goFoo(void);
7+
goFoo();
8+
}
9+
*/
10+
import "C"
11+
12+
import (
13+
"runtime"
14+
"testing"
15+
"unsafe"
16+
)
17+
18+
// nestedCall calls into C, back into Go, and finally to f.
19+
func nestedCall(f func()) {
20+
// NOTE: Depends on representation of f.
21+
// callback(x) calls goCallback(x)
22+
C.callback(*(*unsafe.Pointer)(unsafe.Pointer(&f)))
23+
}
24+
25+
//export goCallback
26+
func goCallback(p unsafe.Pointer) {
27+
(*(*func())(unsafe.Pointer(&p)))()
28+
}
29+
30+
func TestCallback(t *testing.T) {
31+
var x = false
32+
nestedCall(func(){x = true})
33+
if !x {
34+
t.Fatal("nestedCall did not call func")
35+
}
36+
}
37+
38+
func TestCallbackGC(t *testing.T) {
39+
nestedCall(runtime.GC)
40+
}
41+
42+
func lockedOSThread() bool // in runtime.c
43+
44+
func TestCallbackPanic(t *testing.T) {
45+
// Make sure panic during callback unwinds properly.
46+
if lockedOSThread() {
47+
t.Fatal("locked OS thread on entry to TestCallbackPanic")
48+
}
49+
defer func() {
50+
s := recover()
51+
if s == nil {
52+
t.Fatal("did not panic")
53+
}
54+
if s.(string) != "callback panic" {
55+
t.Fatal("wrong panic:", s)
56+
}
57+
if lockedOSThread() {
58+
t.Fatal("locked OS thread on exit from TestCallbackPanic")
59+
}
60+
}()
61+
nestedCall(func(){panic("callback panic")})
62+
panic("nestedCall returned")
63+
}
64+
65+
func TestCallbackPanicLoop(t *testing.T) {
66+
// Make sure we don't blow out m->g0 stack.
67+
for i := 0; i < 100000; i++ {
68+
TestCallbackPanic(t)
69+
}
70+
}
71+
72+
func TestCallbackPanicLocked(t *testing.T) {
73+
runtime.LockOSThread()
74+
defer runtime.UnlockOSThread()
75+
76+
if !lockedOSThread() {
77+
t.Fatal("runtime.LockOSThread didn't")
78+
}
79+
defer func() {
80+
s := recover()
81+
if s == nil {
82+
t.Fatal("did not panic")
83+
}
84+
if s.(string) != "callback panic" {
85+
t.Fatal("wrong panic:", s)
86+
}
87+
if !lockedOSThread() {
88+
t.Fatal("lost lock on OS thread after panic")
89+
}
90+
}()
91+
nestedCall(func(){panic("callback panic")})
92+
panic("nestedCall returned")
93+
}
94+
95+
// Callback with zero arguments used to make the stack misaligned,
96+
// which broke the garbage collector and other things.
97+
func TestZeroArgCallback(t *testing.T) {
98+
defer func() {
99+
s := recover()
100+
if s != nil {
101+
t.Fatal("panic during callback:", s)
102+
}
103+
}()
104+
C.callGoFoo()
105+
}
106+
107+
//export goFoo
108+
func goFoo() {
109+
x := 1
110+
for i := 0; i < 10000; i++ {
111+
// variadic call mallocs + writes to
112+
variadic(x, x, x)
113+
if x != 1 {
114+
panic("bad x")
115+
}
116+
}
117+
}
118+
119+
func variadic(x ...interface{}) {}
120+
121+
func TestBlocking(t *testing.T) {
122+
c := make(chan int)
123+
go func() {
124+
for i := 0; i < 10; i++ {
125+
c <- <-c
126+
}
127+
}()
128+
nestedCall(func(){
129+
for i := 0; i < 10; i++ {
130+
c <- i
131+
if j := <-c; j != i {
132+
t.Errorf("out of sync %d != %d", j, i)
133+
}
134+
}
135+
})
136+
}

misc/cgo/test/callback_c.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright 2011 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
#include <sys/types.h>
6+
#include "_cgo_export.h"
7+
8+
void
9+
callback(void *f)
10+
{
11+
goCallback(f);
12+
}

misc/cgo/test/cgo_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package cgotest
2+
3+
// dummy file so gotest thinks there are tests.
4+
// the actual tests are in the main go files, next
5+
// to the code they test.
6+

misc/cgo/stdio/test1.go renamed to misc/cgo/test/issue1222.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
// This file contains test cases for cgo.
66

7-
package stdio
7+
package cgotest
88

99
/*
1010
// issue 1222

misc/cgo/test/issue1328.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2011 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package cgotest
6+
7+
import "testing"
8+
9+
// extern void BackIntoGo(void);
10+
// void IntoC() { BackIntoGo(); }
11+
import "C"
12+
13+
//export BackIntoGo
14+
func BackIntoGo() {
15+
x := 1
16+
17+
for i := 0; i < 10000; i++ {
18+
xvariadic(x)
19+
if x != 1 {
20+
panic("x is not 1?")
21+
}
22+
}
23+
}
24+
25+
func xvariadic(x ...interface{}) {
26+
}
27+
28+
func Test1328(t *testing.T) {
29+
C.IntoC()
30+
}

0 commit comments

Comments
 (0)