runtime: store asmcgocall return PC where the ARM unwind expects it

The ARM implementation of runtime.cgocallback_gofunc diverged
from the calling convention by leaving a word of garbage at
the top of the stack and storing the return PC above the
locals.  This change stores the return PC at the top of the
stack and removes the save area above the locals.

Update #5124
This CL fixes first part of the ARM issues and added the unwind test.

R=golang-dev, bradfitz, minux.ma, cshapiro, rsc
CC=golang-dev
https://golang.org/cl/7728045

misc/cgo/test/callback.go

@@ -12,7 +12,9 @@ import "C"
 
 import (
 	"./backdoor"
+	"path"
 	"runtime"
+	"strings"
 	"testing"
 	"unsafe"
 )
@@ -136,3 +138,51 @@ func testBlocking(t *testing.T) {
 		}
 	})
 }
+
+// Test that the stack can be unwound through a call out and call back
+// into Go.
+func testCallbackCallers(t *testing.T) {
+	pc := make([]uintptr, 100)
+	n := 0
+	name := []string{
+		"test.goCallback",
+		"runtime.cgocallbackg",
+		"runtime.cgocallback_gofunc",
+		"return",
+		"runtime.cgocall",
+		"test._Cfunc_callback",
+		"test.nestedCall",
+		"test.testCallbackCallers",
+		"test.TestCallbackCallers",
+		"testing.tRunner",
+		"runtime.goexit",
+	}
+	nestedCall(func() {
+		n = runtime.Callers(2, pc)
+	})
+	// The ARM cannot unwind all the way down to runtime.goexit.
+	// See issue 5124.
+	if n != len(name) && runtime.GOARCH != "arm" {
+		t.Errorf("expected %d frames, got %d", len(name), n)
+	}
+	for i := 0; i < n; i++ {
+		f := runtime.FuncForPC(pc[i])
+		if f == nil {
+			t.Fatalf("expected non-nil Func for pc %p", pc[i])
+		}
+		fname := f.Name()
+		// Remove the prepended pathname from automatically
+		// generated cgo function names.
+		if strings.HasPrefix(fname, "_") {
+			fname = path.Base(f.Name()[1:])
+		}
+		if fname != name[i] {
+			t.Errorf("expected function name %s, got %s", name[i], fname)
+		}
+		// The ARM cannot unwind frames past runtime.cgocall.
+		// See issue 5124.
+		if runtime.GOARCH == "arm" && i == 4 {
+			break
+		}
+	}
+}

misc/cgo/test/cgo_test.go

@@ -36,5 +36,6 @@ func TestBoolAlign(t *testing.T)           { testBoolAlign(t) }
 func Test3729(t *testing.T)                { test3729(t) }
 func Test3775(t *testing.T)                { test3775(t) }
 func TestCthread(t *testing.T)             { testCthread(t) }
+func TestCallbackCallers(t *testing.T)     { testCallbackCallers(t) }
 
 func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }

src/pkg/runtime/asm_arm.s

@@ -326,7 +326,7 @@ TEXT runtime·cgocallback(SB),7,$12
 
 // cgocallback_gofunc(void (*fn)(void*), void *frame, uintptr framesize)
 // See cgocall.c for more details.
-TEXT	runtime·cgocallback_gofunc(SB),7,$16
+TEXT	runtime·cgocallback_gofunc(SB),7,$12
 	// Load m and g from thread-local storage.
 	MOVW	_cgo_load_gm(SB), R0
 	CMP	$0, R0
@@ -337,7 +337,7 @@ TEXT	runtime·cgocallback_gofunc(SB),7,$16
 	// In this case, we're running on the thread stack, so there's
 	// lots of space, but the linker doesn't know. Hide the call from
 	// the linker analysis by using an indirect call.
-	MOVW	m, savedm-16(SP)
+	MOVW	m, savedm-12(SP)
 	CMP	$0, m
 	B.NE havem
 	MOVW	$runtime·needm(SB), R0
@@ -348,10 +348,6 @@ havem:
 	// Save current m->g0->sched.sp on stack and then set it to SP.
 	// Save current sp in m->g0->sched.sp in preparation for
 	// switch back to m->curg stack.
-	MOVW	fn+0(FP), R0
-	MOVW	frame+4(FP), R1
-	MOVW	framesize+8(FP), R2
-
 	MOVW	m_g0(m), R3
 	MOVW	(g_sched+gobuf_sp)(R3), R4
 	MOVW.W	R4, -4(R13)
@@ -368,23 +364,23 @@ havem:
 	// This has the added benefit that it looks to the traceback
 	// routine like cgocallbackg is going to return to that
 	// PC (because we defined cgocallbackg to have
-	// a frame size of 16, the same amount that we use below),
+	// a frame size of 12, the same amount that we use below),
 	// so that the traceback will seamlessly trace back into
 	// the earlier calls.
+	MOVW	fn+4(FP), R0
+	MOVW	frame+8(FP), R1
+	MOVW	framesize+12(FP), R2
 
-	// Save current m->g0->sched.sp on stack and then set it to SP.
 	MOVW	m_curg(m), g
 	MOVW	(g_sched+gobuf_sp)(g), R4 // prepare stack as R4
 
 	// Push gobuf.pc
 	MOVW	(g_sched+gobuf_pc)(g), R5
-	SUB	$4, R4
+	MOVW.W	R5, -16(R4)
-	MOVW	R5, 0(R4)
 
 	// Push arguments to cgocallbackg.
 	// Frame size here must match the frame size above
 	// to trick traceback routines into doing the right thing.
-	SUB	$16, R4
 	MOVW	R0, 4(R4)
 	MOVW	R1, 8(R4)
 	MOVW	R2, 12(R4)
@@ -394,10 +390,10 @@ havem:
 	BL	runtime·cgocallbackg(SB)
 
 	// Restore g->gobuf (== m->curg->gobuf) from saved values.
-	MOVW	16(R13), R5
+	MOVW	0(R13), R5
 	MOVW	R5, (g_sched+gobuf_pc)(g)
-	ADD	$(16+4), R13 // SP clobbered! It is ok!
+	ADD	$(12+4), R13, R4
-	MOVW	R13, (g_sched+gobuf_sp)(g)
+	MOVW	R4, (g_sched+gobuf_sp)(g)
 
 	// Switch back to m->g0's stack and restore m->g0->sched.sp.
 	// (Unlike m->curg, the g0 goroutine never uses sched.pc,
@@ -411,7 +407,7 @@ havem:
 
 	// If the m on entry was nil, we called needm above to borrow an m
 	// for the duration of the call. Since the call is over, return it with dropm.
-	MOVW	savedm-16(SP), R6
+	MOVW	savedm-12(SP), R6
 	CMP	$0, R6
 	B.NE	3(PC)
 	MOVW	$runtime·dropm(SB), R0