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

diff --git a/misc/cgo/test/callback.go b/misc/cgo/test/callback.go
index 4f5d3f855f81b0f0b795bc2f0726298f424f2dd1..3feec134b70dcefa2111801a8817767682068b87 100644 (file)
--- a/misc/cgo/test/callback.go
+++ b/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
+               }
+       }
+}

diff --git a/misc/cgo/test/cgo_test.go b/misc/cgo/test/cgo_test.go
index 536fa507aee31adac2e7dbbef0a99682290159ce..1901d5d086f78d47c5e171da71016909a223cfa1 100644 (file)
--- a/misc/cgo/test/cgo_test.go
+++ b/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) }

diff --git a/src/pkg/runtime/asm_arm.s b/src/pkg/runtime/asm_arm.s
index 6b2d6afda033c980a0cd8c0c02dbe2c6604b16fa..e5449333266594a2eb81a87bf7a11cdc456ee470 100644 (file)
--- a/src/pkg/runtime/asm_arm.s
+++ 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    R5, 0(R4)
+       MOVW.W  R5, -16(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!
-       MOVW    R13, (g_sched+gobuf_sp)(g)
+       ADD     $(12+4), R13, R4
+       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