runtime: keep g->syscallsp consistent after cgo->Go callbacks

Normally, the caller to runtime.entersyscall() must not return before
calling runtime.exitsyscall(), lest g->syscallsp become a dangling
pointer. runtime.cgocallbackg() violates this constraint. To work around
this, save g->syscallsp and g->syscallpc around cgo->Go callbacks, then
restore them after calling runtime.entersyscall(), which restores the
syscall stack frame pointer saved by cgocall. This allows the GC to
correctly trace a goroutine that is currently returning from a
Go->cgo->Go chain.

This also adds a check to proc.c that panics if g->syscallsp is clearly
invalid. It is not 100% foolproof, as it will not catch a case where the
stack was popped then pushed back beyond g->syscallsp, but it does catch
the present cgo issue and makes existing tests fail without the bugfix.

Fixes issue 7978.

LGTM=dvyukov, rsc
R=golang-codereviews, dvyukov, minux, bradfitz, iant, gobot, rsc
CC=golang-codereviews, rsc
https://codereview.appspot.com/131910043

Committer: Russ Cox <rsc@golang.org>

2 files changed, 100 insertions, 0 deletions

diff --git a/misc/cgo/test/cgo_test.go b/misc/cgo/test/cgo_test.go
index 3783af061..1899d4605 100644
--- a/misc/cgo/test/cgo_test.go
+++ b/misc/cgo/test/cgo_test.go
@@ -56,5 +56,6 @@ func TestNaming(t *testing.T)              { testNaming(t) }
 func Test7560(t *testing.T)                { test7560(t) }
 func Test5242(t *testing.T)                { test5242(t) }
 func Test8092(t *testing.T)                { test8092(t) }
+func Test7978(t *testing.T)                { test7978(t) }
 
 func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }
diff --git a/misc/cgo/test/issue7978.go b/misc/cgo/test/issue7978.go
new file mode 100644
index 000000000..39864476c
--- /dev/null
+++ b/misc/cgo/test/issue7978.go
@@ -0,0 +1,99 @@
+// Copyright 2014 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Issue 7978.  Stack tracing didn't work during cgo code after calling a Go
+// callback.  Make sure GC works and the stack trace is correct.
+
+package cgotest
+
+/*
+#include <stdint.h>
+
+void issue7978cb(void);
+
+// use ugly atomic variable sync since that doesn't require calling back into
+// Go code or OS dependencies
+static void issue7978c(uint32_t *sync) {
+	while(__sync_fetch_and_add(sync, 0) != 0)
+		;
+	__sync_fetch_and_add(sync, 1);
+	while(__sync_fetch_and_add(sync, 0) != 2)
+		;
+	issue7978cb();
+	__sync_fetch_and_add(sync, 1);
+	while(__sync_fetch_and_add(sync, 0) != 6)
+		;
+}
+*/
+import "C"
+
+import (
+	"runtime"
+	"strings"
+	"sync/atomic"
+	"testing"
+)
+
+var issue7978sync uint32
+
+func issue7978check(t *testing.T, wantFunc string, badFunc string, depth int) {
+	runtime.GC()
+	buf := make([]byte, 65536)
+	trace := string(buf[:runtime.Stack(buf, true)])
+	for _, goroutine := range strings.Split(trace, "\n\n") {
+		if strings.Contains(goroutine, "test.issue7978go") {
+			trace := strings.Split(goroutine, "\n")
+			// look for the expected function in the stack
+			for i := 0; i < depth; i++ {
+				if badFunc != "" && strings.Contains(trace[1+2*i], badFunc) {
+					t.Errorf("bad stack: found %s in the stack:\n%s", badFunc, goroutine)
+					return
+				}
+				if strings.Contains(trace[1+2*i], wantFunc) {
+					return
+				}
+			}
+			t.Errorf("bad stack: didn't find %s in the stack:\n%s", wantFunc, goroutine)
+			return
+		}
+	}
+	t.Errorf("bad stack: goroutine not found. Full stack dump:\n%s", trace)
+}
+
+func issue7978wait(store uint32, wait uint32) {
+	if store != 0 {
+		atomic.StoreUint32(&issue7978sync, store)
+	}
+	for atomic.LoadUint32(&issue7978sync) != wait {
+		runtime.Gosched()
+	}
+}
+
+//export issue7978cb
+func issue7978cb() {
+	issue7978wait(3, 4)
+}
+
+func issue7978go() {
+	C.issue7978c((*C.uint32_t)(&issue7978sync))
+	issue7978wait(7, 8)
+}
+
+func test7978(t *testing.T) {
+	issue7978sync = 0
+	go issue7978go()
+	// test in c code, before callback
+	issue7978wait(0, 1)
+	issue7978check(t, "runtime.cgocall_errno(", "", 1)
+	// test in go code, during callback
+	issue7978wait(2, 3)
+	issue7978check(t, "test.issue7978cb(", "test.issue7978go", 3)
+	// test in c code, after callback
+	issue7978wait(4, 5)
+	issue7978check(t, "runtime.cgocall_errno(", "runtime.cgocallback", 1)
+	// test in go code, after return from cgo
+	issue7978wait(6, 7)
+	issue7978check(t, "test.issue7978go(", "", 3)
+	atomic.StoreUint32(&issue7978sync, 8)
+}