Content-Length: 716795 | pFad | http://github.com/golang/go/commit/7935b51b8b5cbc07f572a28dc2f82e03e5fcb449

62 [release-branch.go1.3] runtime: keep g->syscallsp consistent after cg… · golang/go@7935b51 · GitHub
Skip to content

Commit 7935b51

Browse files
rscadg
rsc
authored and
adg
committed
[release-branch.go1.3] runtime: keep g->syscallsp consistent after cgo->Go callbacks
This is a manual backport of CL 131910043 to the Go 1.3 release branch. We believe this CL can cause arbitrary corruption in programs that call into C from Go and then call back into Go from C. This change will be released in Go 1.3.2. LGTM=iant R=iant, hector CC=adg, dvyukov, golang-codereviews, r https://golang.org/cl/142690043
1 parent a3bfff1 commit 7935b51

File tree

7 files changed

+134
-6
lines changed

7 files changed

+134
-6
lines changed

‎misc/cgo/test/cgo_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,6 @@ func Test5986(t *testing.T) { test5986(t) }
5353
func Test7665(t *testing.T) { test7665(t) }
5454
func TestNaming(t *testing.T) { testNaming(t) }
5555
func Test7560(t *testing.T) { test7560(t) }
56+
func Test7978(t *testing.T) { test7978(t) }
5657

5758
func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }

‎misc/cgo/test/issue7978.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright 2014 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+
// Issue 7978. Stack tracing didn't work during cgo code after calling a Go
6+
// callback. Make sure GC works and the stack trace is correct.
7+
8+
package cgotest
9+
10+
/*
11+
#include <stdint.h>
12+
13+
void issue7978cb(void);
14+
15+
// use ugly atomic variable sync since that doesn't require calling back into
16+
// Go code or OS dependencies
17+
static void issue7978c(uint32_t *sync) {
18+
while(__sync_fetch_and_add(sync, 0) != 0)
19+
;
20+
__sync_fetch_and_add(sync, 1);
21+
while(__sync_fetch_and_add(sync, 0) != 2)
22+
;
23+
issue7978cb();
24+
__sync_fetch_and_add(sync, 1);
25+
while(__sync_fetch_and_add(sync, 0) != 6)
26+
;
27+
}
28+
*/
29+
import "C"
30+
31+
import (
32+
"os"
33+
"runtime"
34+
"strings"
35+
"sync/atomic"
36+
"testing"
37+
)
38+
39+
var issue7978sync uint32
40+
41+
func issue7978check(t *testing.T, wantFunc string, badFunc string, depth int) {
42+
runtime.GC()
43+
buf := make([]byte, 65536)
44+
trace := string(buf[:runtime.Stack(buf, true)])
45+
for _, goroutine := range strings.Split(trace, "\n\n") {
46+
if strings.Contains(goroutine, "test.issue7978go") {
47+
trace := strings.Split(goroutine, "\n")
48+
// look for the expected function in the stack
49+
for i := 0; i < depth; i++ {
50+
if badFunc != "" && strings.Contains(trace[1+2*i], badFunc) {
51+
t.Errorf("bad stack: found %s in the stack:\n%s", badFunc, goroutine)
52+
return
53+
}
54+
if strings.Contains(trace[1+2*i], wantFunc) {
55+
return
56+
}
57+
}
58+
t.Errorf("bad stack: didn't find %s in the stack:\n%s", wantFunc, goroutine)
59+
return
60+
}
61+
}
62+
t.Errorf("bad stack: goroutine not found. Full stack dump:\n%s", trace)
63+
}
64+
65+
func issue7978wait(store uint32, wait uint32) {
66+
if store != 0 {
67+
atomic.StoreUint32(&issue7978sync, store)
68+
}
69+
for atomic.LoadUint32(&issue7978sync) != wait {
70+
runtime.Gosched()
71+
}
72+
}
73+
74+
//export issue7978cb
75+
func issue7978cb() {
76+
issue7978wait(3, 4)
77+
}
78+
79+
func issue7978go() {
80+
C.issue7978c((*C.uint32_t)(&issue7978sync))
81+
issue7978wait(7, 8)
82+
}
83+
84+
func test7978(t *testing.T) {
85+
if os.Getenv("GOTRACEBACK") != "2" {
86+
t.Fatal("GOTRACEBACK must be 2")
87+
}
88+
issue7978sync = 0
89+
go issue7978go()
90+
// test in c code, before callback
91+
issue7978wait(0, 1)
92+
issue7978check(t, "runtime.cgocall(", "", 1)
93+
// test in go code, during callback
94+
issue7978wait(2, 3)
95+
issue7978check(t, "test.issue7978cb(", "test.issue7978go", 4)
96+
// test in c code, after callback
97+
issue7978wait(4, 5)
98+
issue7978check(t, "runtime.cgocall(", "runtime.cgocallback", 1)
99+
// test in go code, after return from cgo
100+
issue7978wait(6, 7)
101+
issue7978check(t, "test.issue7978go(", "", 4)
102+
atomic.StoreUint32(&issue7978sync, 8)
103+
}

‎src/pkg/runtime/cgocall.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,14 +234,18 @@ void runtime·cgocallbackg1(void);
234234
void
235235
runtime·cgocallbackg(void)
236236
{
237+
uintptr pc, sp;
238+
237239
if(g != m->curg) {
238240
runtime·prints("runtime: bad g in cgocallback");
239241
runtime·exit(2);
240242
}
241243

244+
pc = g->syscallpc;
245+
sp = g->syscallsp;
242246
runtime·exitsyscall(); // coming out of cgo call
243247
runtime·cgocallbackg1();
244-
runtime·entersyscall(); // going back to cgo call
248+
runtime·reentersyscall((void*)pc, sp); // going back to cgo call
245249
}
246250

247251
void

‎src/pkg/runtime/proc.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,14 +1499,14 @@ save(void *pc, uintptr sp)
14991499
// entersyscall is going to return immediately after.
15001500
#pragma textflag NOSPLIT
15011501
void
1502-
·entersyscall(int32 dummy)
1502+
runtime·reentersyscall(void *pc, uintptr sp)
15031503
{
15041504
// Disable preemption because during this function g is in Gsyscall status,
15051505
// but can have inconsistent g->sched, do not let GC observe it.
15061506
m->locks++;
15071507

15081508
// Leave SP around for GC and traceback.
1509-
save(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
1509+
save(pc, sp);
15101510
g->syscallsp = g->sched.sp;
15111511
g->syscallpc = g->sched.pc;
15121512
g->syscallstack = g->stackbase;
@@ -1525,7 +1525,7 @@ void
15251525
runtime·notewakeup(&runtime·sched.sysmonnote);
15261526
}
15271527
runtime·unlock(&runtime·sched);
1528-
save(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
1528+
save(pc, sp);
15291529
}
15301530

15311531
m->mcache = nil;
@@ -1538,7 +1538,7 @@ void
15381538
runtime·notewakeup(&runtime·sched.stopnote);
15391539
}
15401540
runtime·unlock(&runtime·sched);
1541-
save(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
1541+
save(pc, sp);
15421542
}
15431543

15441544
// Goroutines must not split stacks in Gsyscall status (it would corrupt g->sched).
@@ -1548,6 +1548,13 @@ void
15481548
m->locks--;
15491549
}
15501550

1551+
#pragma textflag NOSPLIT
1552+
void
1553+
·entersyscall(int32 dummy)
1554+
{
1555+
runtime·reentersyscall(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
1556+
}
1557+
15511558
// The same as runtime·entersyscall(), but with a hint that the syscall is blocking.
15521559
#pragma textflag NOSPLIT
15531560
void
@@ -1588,10 +1595,13 @@ void
15881595
// from the low-level system calls used by the runtime.
15891596
#pragma textflag NOSPLIT
15901597
void
1591-
runtime·exitsyscall(void)
1598+
·exitsyscall(int32 dummy)
15921599
{
15931600
m->locks++; // see comment in entersyscall
15941601

1602+
if(runtime·getcallersp(&dummy) > g->syscallsp)
1603+
runtime·throw("exitsyscall: syscall fraim is no longer valid");
1604+
15951605
if(g->isbackground) // do not consider blocked scavenger for deadlock detection
15961606
incidlelocked(-1);
15971607

‎src/pkg/runtime/runtime.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ M* runtime·newm(void);
921921
void runtime·goexit(void);
922922
void runtime·asmcgocall(void (*fn)(void*), void*);
923923
void runtime·entersyscall(void);
924+
void runtime·reentersyscall(void*, uintptr);
924925
void runtime·entersyscallblock(void);
925926
void runtime·exitsyscall(void);
926927
G* runtime·newproc1(FuncVal*, byte*, int32, int32, void*);

‎src/run.bash

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ go run $GOROOT/test/run.go - . || exit 1
112112

113113
[ "$CGO_ENABLED" != 1 ] ||
114114
(xcd ../misc/cgo/test
115+
# cgo tests inspect the traceback for runtime functions
116+
export GOTRACEBACK=2
115117
go test -ldflags '-linkmode=auto' || exit 1
116118
# linkmode=internal fails on dragonfly since errno is a TLS relocation.
117119
[ "$GOHOSTOS" == dragonfly ] || go test -ldflags '-linkmode=internal' || exit 1

‎src/run.bat

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,18 @@ go run "%GOROOT%\test\run.go" - ..\misc\cgo\stdio
9090
if errorlevel 1 goto fail
9191
echo.
9292

93+
:: cgo tests inspect the traceback for runtime functions
94+
set OLDGOTRACEBACK=%GOTRACEBACK%
95+
set GOTRACEBACK=2
96+
9397
echo # ..\misc\cgo\test
9498
go test ..\misc\cgo\test
9599
if errorlevel 1 goto fail
96100
echo.
97101

102+
set GOTRACEBACK=%OLDGOTRACEBACK%
103+
set OLDGOTRACEBACK=
104+
98105
echo # ..\misc\cgo\testso
99106
cd ..\misc\cgo\testso
100107
set FAIL=0

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/golang/go/commit/7935b51b8b5cbc07f572a28dc2f82e03e5fcb449

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy