Skip to content

Commit bc135f6

Browse files
committed
runtime: fix crash in runtime.GoroutineProfile
This is a possible Go 1.2.1 candidate. Fixes #6946. R=iant, r CC=golang-dev https://golang.org/cl/41640043
1 parent c134ce2 commit bc135f6

File tree

6 files changed

+83
-25
lines changed

6 files changed

+83
-25
lines changed

src/pkg/runtime/mgc0.c

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,27 +1515,7 @@ scanframe(Stkframe *frame, void *arg)
15151515
static void
15161516
scanstack(G* gp, void *scanbuf)
15171517
{
1518-
uintptr pc;
1519-
uintptr sp;
1520-
uintptr lr;
1521-
1522-
if(gp->syscallstack != (uintptr)nil) {
1523-
// Scanning another goroutine that is about to enter or might
1524-
// have just exited a system call. It may be executing code such
1525-
// as schedlock and may have needed to start a new stack segment.
1526-
// Use the stack segment and stack pointer at the time of
1527-
// the system call instead, since that won't change underfoot.
1528-
sp = gp->syscallsp;
1529-
pc = gp->syscallpc;
1530-
lr = 0;
1531-
} else {
1532-
// Scanning another goroutine's stack.
1533-
// The goroutine is usually asleep (the world is stopped).
1534-
sp = gp->sched.sp;
1535-
pc = gp->sched.pc;
1536-
lr = gp->sched.lr;
1537-
}
1538-
runtime·gentraceback(pc, sp, lr, gp, 0, nil, 0x7fffffff, scanframe, scanbuf, false);
1518+
runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, scanframe, scanbuf, false);
15391519
}
15401520

15411521
static void

src/pkg/runtime/mprof.goc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ saveg(uintptr pc, uintptr sp, G *gp, TRecord *r)
528528
{
529529
int32 n;
530530

531-
n = runtime·gentraceback((uintptr)pc, (uintptr)sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil, false);
531+
n = runtime·gentraceback(pc, sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil, false);
532532
if(n < nelem(r->stk))
533533
r->stk[n] = 0;
534534
}
@@ -556,7 +556,7 @@ func GoroutineProfile(b Slice) (n int, ok bool) {
556556
for(gp = runtime·allg; gp != nil; gp = gp->alllink) {
557557
if(gp == g || gp->status == Gdead)
558558
continue;
559-
saveg(gp->sched.pc, gp->sched.sp, gp, r++);
559+
saveg(~(uintptr)0, ~(uintptr)0, gp, r++);
560560
}
561561
}
562562

src/pkg/runtime/proc.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ runtime·tracebackothers(G *me)
276276
if((gp = m->curg) != nil && gp != me) {
277277
runtime·printf("\n");
278278
runtime·goroutineheader(gp);
279-
runtime·traceback(gp->sched.pc, gp->sched.sp, gp->sched.lr, gp);
279+
runtime·traceback(~(uintptr)0, ~(uintptr)0, 0, gp);
280280
}
281281

282282
for(gp = runtime·allg; gp != nil; gp = gp->alllink) {
@@ -290,7 +290,7 @@ runtime·tracebackothers(G *me)
290290
runtime·printf("\tgoroutine running on other thread; stack unavailable\n");
291291
runtime·printcreatedby(gp);
292292
} else
293-
runtime·traceback(gp->sched.pc, gp->sched.sp, gp->sched.lr, gp);
293+
runtime·traceback(~(uintptr)0, ~(uintptr)0, 0, gp);
294294
}
295295
}
296296

src/pkg/runtime/runtime_unix_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright 2013 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+
// Only works on systems with syscall.Close.
6+
// We need a fast system call to provoke the race,
7+
// and Close(-1) is nearly universally fast.
8+
9+
// +build darwin dragonfly freebsd linux netbsd openbsd plan9
10+
11+
package runtime_test
12+
13+
import (
14+
"runtime"
15+
"sync"
16+
"sync/atomic"
17+
"syscall"
18+
"testing"
19+
)
20+
21+
func TestGoroutineProfile(t *testing.T) {
22+
// GoroutineProfile used to use the wrong starting sp for
23+
// goroutines coming out of system calls, causing possible
24+
// crashes.
25+
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(100))
26+
27+
var stop uint32
28+
defer atomic.StoreUint32(&stop, 1) // in case of panic
29+
30+
var wg sync.WaitGroup
31+
for i := 0; i < 4; i++ {
32+
wg.Add(1)
33+
go func() {
34+
for atomic.LoadUint32(&stop) == 0 {
35+
syscall.Close(-1)
36+
}
37+
wg.Done()
38+
}()
39+
}
40+
41+
max := 10000
42+
if testing.Short() {
43+
max = 100
44+
}
45+
stk := make([]runtime.StackRecord, 100)
46+
for n := 0; n < max; n++ {
47+
_, ok := runtime.GoroutineProfile(stk)
48+
if !ok {
49+
t.Fatalf("GoroutineProfile failed")
50+
}
51+
}
52+
53+
// If the program didn't crash, we passed.
54+
atomic.StoreUint32(&stop, 1)
55+
wg.Wait()
56+
}

src/pkg/runtime/traceback_arm.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,18 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
2020
Stktop *stk;
2121
String file;
2222

23+
if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
24+
if(gp->syscallstack != (uintptr)nil) {
25+
pc0 = gp->syscallpc;
26+
sp0 = gp->syscallsp;
27+
lr0 = 0;
28+
} else {
29+
pc0 = gp->sched.pc;
30+
sp0 = gp->sched.sp;
31+
lr0 = gp->sched.lr;
32+
}
33+
}
34+
2335
nprint = 0;
2436
runtime·memclr((byte*)&frame, sizeof frame);
2537
frame.pc = pc0;

src/pkg/runtime/traceback_x86.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
3030
String file;
3131

3232
USED(lr0);
33+
34+
if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
35+
if(gp->syscallstack != (uintptr)nil) {
36+
pc0 = gp->syscallpc;
37+
sp0 = gp->syscallsp;
38+
} else {
39+
pc0 = gp->sched.pc;
40+
sp0 = gp->sched.sp;
41+
}
42+
}
3343

3444
nprint = 0;
3545
runtime·memclr((byte*)&frame, sizeof frame);

0 commit comments

Comments
 (0)