]> Cypherpunks repositories - gostls13.git/commitdiff
cgo: adjust return value location to account for stack copies.
authorKeith Randall <khr@golang.org>
Thu, 25 Sep 2014 14:59:01 +0000 (07:59 -0700)
committerKeith Randall <khr@golang.org>
Thu, 25 Sep 2014 14:59:01 +0000 (07:59 -0700)
During a cgo call, the stack can be copied.  This copy invalidates
the pointer that cgo has into the return value area.  To fix this
problem, pass the address of the location containing the stack
top value (which is in the G struct).  For cgo functions which
return values, read the stktop before and after the cgo call to
compute the adjustment necessary to write the return value.

Fixes #8771

LGTM=iant, rsc
R=iant, rsc, khr
CC=golang-codereviews
https://golang.org/cl/144130043

misc/cgo/test/callback.go
misc/cgo/test/callback_c.c
misc/cgo/test/cgo_test.go
src/cmd/cgo/out.go
src/runtime/asm_386.s
src/runtime/asm_amd64.s
src/runtime/asm_arm.s
src/runtime/cgo/callbacks.c
src/runtime/stack.c

index a7f1a3ecd67a8a35ecf8927f2ff3e03d0f8b8bb9..44167e6e9ef0f8955e6a92dd36d9f6061566a60a 100644 (file)
@@ -10,6 +10,9 @@ void callGoFoo(void);
 void callGoStackCheck(void);
 void callPanic(void);
 void callCgoAllocate(void);
+int callGoReturnVal(void);
+int returnAfterGrow(void);
+int returnAfterGrowFromGo(void);
 */
 import "C"
 
@@ -212,6 +215,48 @@ func testAllocateFromC(t *testing.T) {
        C.callCgoAllocate() // crashes or exits on failure
 }
 
+// Test that C code can return a value if it calls a Go function that
+// causes a stack copy.
+func testReturnAfterGrow(t *testing.T) {
+       // Use a new goroutine so that we get a small stack.
+       c := make(chan int)
+       go func() {
+               c <- int(C.returnAfterGrow())
+       }()
+       if got, want := <-c, 123456; got != want {
+               t.Errorf("got %d want %d", got, want)
+       }
+}
+
+// Test that we can return a value from Go->C->Go if the Go code
+// causes a stack copy.
+func testReturnAfterGrowFromGo(t *testing.T) {
+       // Use a new goroutine so that we get a small stack.
+       c := make(chan int)
+       go func() {
+               c <- int(C.returnAfterGrowFromGo())
+       }()
+       if got, want := <-c, 129*128/2; got != want {
+               t.Errorf("got %d want %d", got, want)
+       }
+}
+
+//export goReturnVal
+func goReturnVal() (r C.int) {
+       // Force a stack copy.
+       var f func(int) int
+       f = func(i int) int {
+               var buf [256]byte
+               use(buf[:])
+               if i == 0 {
+                       return 0
+               }
+               return i + f(i-1)
+       }
+       r = C.int(f(128))
+       return
+}
+
 func testCallbackStack(t *testing.T) {
        // Make cgo call and callback with different amount of stack stack available.
        // We do not do any explicit checks, just ensure that it does not crash.
index dcd4ddd4ee3b1062df92e3dee3db354b8db0e939..5bb642534019ad545a87ee6d97a96d0796085dcc 100644 (file)
@@ -64,3 +64,19 @@ callGoStackCheck(void)
        extern void goStackCheck(void);
        goStackCheck();
 }
+
+int
+returnAfterGrow(void)
+{
+       extern int goReturnVal(void);
+       goReturnVal();
+       return 123456;
+}
+
+int
+returnAfterGrowFromGo(void)
+{
+       extern int goReturnVal(void);
+       return goReturnVal();
+}
+
index 1d1abf72914bc2dc4ca5f5b4d1213c12c2b3e6e4..fcfad8304946b476149fad0997aed1d009b229f8 100644 (file)
@@ -10,53 +10,55 @@ import "testing"
 // so that they can use cgo (import "C").
 // These wrappers are here for gotest to find.
 
-func TestAlign(t *testing.T)               { testAlign(t) }
-func TestConst(t *testing.T)               { testConst(t) }
-func TestEnum(t *testing.T)                { testEnum(t) }
-func TestAtol(t *testing.T)                { testAtol(t) }
-func TestErrno(t *testing.T)               { testErrno(t) }
-func TestMultipleAssign(t *testing.T)      { testMultipleAssign(t) }
-func TestUnsignedInt(t *testing.T)         { testUnsignedInt(t) }
-func TestCallback(t *testing.T)            { testCallback(t) }
-func TestCallbackGC(t *testing.T)          { testCallbackGC(t) }
-func TestCallbackPanic(t *testing.T)       { testCallbackPanic(t) }
-func TestCallbackPanicLoop(t *testing.T)   { testCallbackPanicLoop(t) }
-func TestCallbackPanicLocked(t *testing.T) { testCallbackPanicLocked(t) }
-func TestPanicFromC(t *testing.T)          { testPanicFromC(t) }
-func TestAllocateFromC(t *testing.T)       { testAllocateFromC(t) }
-func TestZeroArgCallback(t *testing.T)     { testZeroArgCallback(t) }
-func TestBlocking(t *testing.T)            { testBlocking(t) }
-func Test1328(t *testing.T)                { test1328(t) }
-func TestParallelSleep(t *testing.T)       { testParallelSleep(t) }
-func TestSetEnv(t *testing.T)              { testSetEnv(t) }
-func TestHelpers(t *testing.T)             { testHelpers(t) }
-func TestLibgcc(t *testing.T)              { testLibgcc(t) }
-func Test1635(t *testing.T)                { test1635(t) }
-func TestPrintf(t *testing.T)              { testPrintf(t) }
-func Test4029(t *testing.T)                { test4029(t) }
-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 Test5227(t *testing.T)                { test5227(t) }
-func TestCflags(t *testing.T)              { testCflags(t) }
-func Test5337(t *testing.T)                { test5337(t) }
-func Test5548(t *testing.T)                { test5548(t) }
-func Test5603(t *testing.T)                { test5603(t) }
-func Test6833(t *testing.T)                { test6833(t) }
-func Test3250(t *testing.T)                { test3250(t) }
-func TestCallbackStack(t *testing.T)       { testCallbackStack(t) }
-func TestFpVar(t *testing.T)               { testFpVar(t) }
-func Test4339(t *testing.T)                { test4339(t) }
-func Test6390(t *testing.T)                { test6390(t) }
-func Test5986(t *testing.T)                { test5986(t) }
-func Test7665(t *testing.T)                { test7665(t) }
-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 Test8694(t *testing.T)                { test8694(t) }
+func TestAlign(t *testing.T)                 { testAlign(t) }
+func TestConst(t *testing.T)                 { testConst(t) }
+func TestEnum(t *testing.T)                  { testEnum(t) }
+func TestAtol(t *testing.T)                  { testAtol(t) }
+func TestErrno(t *testing.T)                 { testErrno(t) }
+func TestMultipleAssign(t *testing.T)        { testMultipleAssign(t) }
+func TestUnsignedInt(t *testing.T)           { testUnsignedInt(t) }
+func TestCallback(t *testing.T)              { testCallback(t) }
+func TestCallbackGC(t *testing.T)            { testCallbackGC(t) }
+func TestCallbackPanic(t *testing.T)         { testCallbackPanic(t) }
+func TestCallbackPanicLoop(t *testing.T)     { testCallbackPanicLoop(t) }
+func TestCallbackPanicLocked(t *testing.T)   { testCallbackPanicLocked(t) }
+func TestPanicFromC(t *testing.T)            { testPanicFromC(t) }
+func TestAllocateFromC(t *testing.T)         { testAllocateFromC(t) }
+func TestZeroArgCallback(t *testing.T)       { testZeroArgCallback(t) }
+func TestBlocking(t *testing.T)              { testBlocking(t) }
+func Test1328(t *testing.T)                  { test1328(t) }
+func TestParallelSleep(t *testing.T)         { testParallelSleep(t) }
+func TestSetEnv(t *testing.T)                { testSetEnv(t) }
+func TestHelpers(t *testing.T)               { testHelpers(t) }
+func TestLibgcc(t *testing.T)                { testLibgcc(t) }
+func Test1635(t *testing.T)                  { test1635(t) }
+func TestPrintf(t *testing.T)                { testPrintf(t) }
+func Test4029(t *testing.T)                  { test4029(t) }
+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 Test5227(t *testing.T)                  { test5227(t) }
+func TestCflags(t *testing.T)                { testCflags(t) }
+func Test5337(t *testing.T)                  { test5337(t) }
+func Test5548(t *testing.T)                  { test5548(t) }
+func Test5603(t *testing.T)                  { test5603(t) }
+func Test6833(t *testing.T)                  { test6833(t) }
+func Test3250(t *testing.T)                  { test3250(t) }
+func TestCallbackStack(t *testing.T)         { testCallbackStack(t) }
+func TestFpVar(t *testing.T)                 { testFpVar(t) }
+func Test4339(t *testing.T)                  { test4339(t) }
+func Test6390(t *testing.T)                  { test6390(t) }
+func Test5986(t *testing.T)                  { test5986(t) }
+func Test7665(t *testing.T)                  { test7665(t) }
+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 Test8694(t *testing.T)                  { test8694(t) }
+func TestReturnAfterGrow(t *testing.T)       { testReturnAfterGrow(t) }
+func TestReturnAfterGrowFromGo(t *testing.T) { testReturnAfterGrowFromGo(t) }
 
 func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }
index 2d14f766fc4dae1ec5bad258022cb2a8f6cdc474..4e5b3a24541ae11b1d05213a89a7371579fa767b 100644 (file)
@@ -44,6 +44,7 @@ func (p *Package) writeDefs() {
        fmt.Fprintf(fm, "int main() { return 0; }\n")
        if *importRuntimeCgo {
                fmt.Fprintf(fm, "void crosscall2(void(*fn)(void*, int), void *a, int c) { }\n")
+               fmt.Fprintf(fm, "char* cgo_topofstack(void) { return (char*)0; }\n")
        } else {
                // If we're not importing runtime/cgo, we *are* runtime/cgo,
                // which provides crosscall2.  We just need a prototype.
@@ -519,9 +520,13 @@ func (p *Package) writeOutputFunc(fgcc *os.File, n *Name) {
        // Use packed attribute to force no padding in this struct in case
        // gcc has different packing requirements.
        fmt.Fprintf(fgcc, "\t%s %v *a = v;\n", ctype, p.packedAttribute())
+       if n.FuncType.Result != nil {
+               // Save the stack top for use below.
+               fmt.Fprintf(fgcc, "\tchar *stktop = cgo_topofstack();\n")
+       }
        fmt.Fprintf(fgcc, "\t")
        if t := n.FuncType.Result; t != nil {
-               fmt.Fprintf(fgcc, "a->r = ")
+               fmt.Fprintf(fgcc, "__typeof__(a->r) r = ")
                if c := t.C.String(); c[len(c)-1] == '*' {
                        fmt.Fprint(fgcc, "(__typeof__(a->r)) ")
                }
@@ -544,6 +549,13 @@ func (p *Package) writeOutputFunc(fgcc *os.File, n *Name) {
                fmt.Fprintf(fgcc, "a->p%d", i)
        }
        fmt.Fprintf(fgcc, ");\n")
+       if n.FuncType.Result != nil {
+               // The cgo call may have caused a stack copy (via a callback).
+               // Adjust the return value pointer appropriately.
+               fmt.Fprintf(fgcc, "\ta = (void*)((char*)a + (cgo_topofstack() - stktop));\n")
+               // Save the return value.
+               fmt.Fprintf(fgcc, "\ta->r = r;\n")
+       }
        if n.AddError {
                fmt.Fprintf(fgcc, "\treturn errno;\n")
        }
@@ -1131,6 +1143,8 @@ __cgo_size_assert(__cgo_long_long, 8)
 __cgo_size_assert(float, 4)
 __cgo_size_assert(double, 8)
 
+extern char* cgo_topofstack(void);
+
 #include <errno.h>
 #include <string.h>
 `
index 846a214d555696dd4224ae7a72edb556bb2d5d98..f1b3346e83341745268199e2040dbd35e08cfc79 100644 (file)
@@ -2275,3 +2275,13 @@ TEXT runtime·fastrand1(SB), NOSPLIT, $0-4
 TEXT runtime·return0(SB), NOSPLIT, $0
        MOVL    $0, AX
        RET
+
+// Called from cgo wrappers, this function returns g->m->curg.stack.hi.
+// Must obey the gcc calling convention.
+TEXT cgo_topofstack(SB),NOSPLIT,$0
+       get_tls(CX)
+       MOVL    g(CX), AX
+       MOVL    g_m(AX), AX
+       MOVL    m_curg(AX), AX
+       MOVL    (g_stack+stack_hi)(AX), AX
+       RET
index 7304d79a2fe35e3692d9ebca3ad0111ab7d84246..b4c6c6bdca99a7acfa1adb2298e8fef4119dae87 100644 (file)
@@ -2220,3 +2220,14 @@ TEXT runtime·fastrand1(SB), NOSPLIT, $0-4
 TEXT runtime·return0(SB), NOSPLIT, $0
        MOVL    $0, AX
        RET
+
+
+// Called from cgo wrappers, this function returns g->m->curg.stack.hi.
+// Must obey the gcc calling convention.
+TEXT cgo_topofstack(SB),NOSPLIT,$0
+       get_tls(CX)
+       MOVQ    g(CX), AX
+       MOVQ    g_m(AX), AX
+       MOVQ    m_curg(AX), AX
+       MOVQ    (g_stack+stack_hi)(AX), AX
+       RET
index 38d97b78f3afb815e152480be63d18a04d3559f7..2c5de8afb19d2b42a3de63d8f2de327b33512fe6 100644 (file)
@@ -1300,3 +1300,11 @@ yieldloop:
        RET
        SUB     $1, R1
        B yieldloop
+
+// Called from cgo wrappers, this function returns g->m->curg.stack.hi.
+// Must obey the gcc calling convention.
+TEXT cgo_topofstack(SB),NOSPLIT,$0
+       MOVW    g_m(g), R0
+       MOVW    m_curg(R0), R0
+       MOVW    (g_stack+stack_hi)(R0), R0
+       RET
index 16614d03dbc7f64c82051c45a686cd19dd6083fa..cea9b1667ff78d1e183f1c2766600d42bc9c98ee 100644 (file)
@@ -78,3 +78,6 @@ void (*_cgo_free)(void*) = x_cgo_free;
 #pragma cgo_import_static x_cgo_thread_start
 extern void x_cgo_thread_start(void*);
 void (*_cgo_thread_start)(void*) = x_cgo_thread_start;
+
+#pragma cgo_export_static cgo_topofstack
+#pragma cgo_export_dynamic cgo_topofstack
index 0d8814731c5d6b202255b69a0a13935c0d04526d..2d23c717bd75cb467de710cd8a566f9ccad515a6 100644 (file)
@@ -827,7 +827,9 @@ runtime·shrinkstack(G *gp)
        if(used >= oldsize / 4)
                return; // still using at least 1/4 of the segment.
 
-       if(gp->syscallsp != 0) // TODO: can we handle this case?
+       // We can't copy the stack if we're in a syscall.
+       // The syscall might have pointers into the stack.
+       if(gp->syscallsp != 0)
                return;
 
 #ifdef GOOS_windows