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

diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c
index 99d45faa88b3bd994197ef7099df2c9f57670745..8b8a3e52b69aa9064868fb3f71b74aa0c0a07da9 100644 (file)
--- a/src/pkg/runtime/mgc0.c
+++ b/src/pkg/runtime/mgc0.c
@@ -1515,27 +1515,7 @@ scanframe(Stkframe *frame, void *arg)
 static void
 scanstack(G* gp, void *scanbuf)
 {
-       uintptr pc;
-       uintptr sp;
-       uintptr lr;
-
-       if(gp->syscallstack != (uintptr)nil) {
-               // Scanning another goroutine that is about to enter or might
-               // have just exited a system call. It may be executing code such
-               // as schedlock and may have needed to start a new stack segment.
-               // Use the stack segment and stack pointer at the time of
-               // the system call instead, since that won't change underfoot.
-               sp = gp->syscallsp;
-               pc = gp->syscallpc;
-               lr = 0;
-       } else {
-               // Scanning another goroutine's stack.
-               // The goroutine is usually asleep (the world is stopped).
-               sp = gp->sched.sp;
-               pc = gp->sched.pc;
-               lr = gp->sched.lr;
-       }
-       runtime·gentraceback(pc, sp, lr, gp, 0, nil, 0x7fffffff, scanframe, scanbuf, false);
+       runtime·gentraceback(~(uintptr)0, ~(uintptr)0, 0, gp, 0, nil, 0x7fffffff, scanframe, scanbuf, false);
 }
 
 static void

diff --git a/src/pkg/runtime/mprof.goc b/src/pkg/runtime/mprof.goc
index 91bdb42ead51739cbb98cf1d0e415dd4ee6e771f..58d9e1c6130713bd23e3ddabc760235078578465 100644 (file)
--- a/src/pkg/runtime/mprof.goc
+++ b/src/pkg/runtime/mprof.goc
@@ -528,7 +528,7 @@ saveg(uintptr pc, uintptr sp, G *gp, TRecord *r)
 {
        int32 n;
        
-       n = runtime·gentraceback((uintptr)pc, (uintptr)sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil, false);
+       n = runtime·gentraceback(pc, sp, 0, gp, 0, r->stk, nelem(r->stk), nil, nil, false);
        if(n < nelem(r->stk))
                r->stk[n] = 0;
 }
@@ -556,7 +556,7 @@ func GoroutineProfile(b Slice) (n int, ok bool) {
                        for(gp = runtime·allg; gp != nil; gp = gp->alllink) {
                                if(gp == g || gp->status == Gdead)
                                        continue;
-                               saveg(gp->sched.pc, gp->sched.sp, gp, r++);
+                               saveg(~(uintptr)0, ~(uintptr)0, gp, r++);
                        }
                }
        

diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c
index de26c72d3d7553243dd79c7e6ffcef95fb5ef8a4..ed3e1e73eebdd1ba7a5556cce834b74b56632647 100644 (file)
--- a/src/pkg/runtime/proc.c
+++ b/src/pkg/runtime/proc.c
@@ -276,7 +276,7 @@ runtime·tracebackothers(G *me)
        if((gp = m->curg) != nil && gp != me) {
                runtime·printf("\n");
                runtime·goroutineheader(gp);
-               runtime·traceback(gp->sched.pc, gp->sched.sp, gp->sched.lr, gp);
+               runtime·traceback(~(uintptr)0, ~(uintptr)0, 0, gp);
        }
 
        for(gp = runtime·allg; gp != nil; gp = gp->alllink) {
@@ -290,7 +290,7 @@ runtime·tracebackothers(G *me)
                        runtime·printf("\tgoroutine running on other thread; stack unavailable\n");
                        runtime·printcreatedby(gp);
                } else
-                       runtime·traceback(gp->sched.pc, gp->sched.sp, gp->sched.lr, gp);
+                       runtime·traceback(~(uintptr)0, ~(uintptr)0, 0, gp);
        }
 }
 

diff --git a/src/pkg/runtime/runtime_unix_test.go b/src/pkg/runtime/runtime_unix_test.go
new file mode 100644 (file)
index 0000000..963de8c
--- /dev/null
+++ b/src/pkg/runtime/runtime_unix_test.go
@@ -0,0 +1,56 @@
+// Copyright 2013 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Only works on systems with syscall.Close.
+// We need a fast system call to provoke the race,
+// and Close(-1) is nearly universally fast.
+
+// +build darwin dragonfly freebsd linux netbsd openbsd plan9
+
+package runtime_test
+
+import (
+       "runtime"
+       "sync"
+       "sync/atomic"
+       "syscall"
+       "testing"
+)
+
+func TestGoroutineProfile(t *testing.T) {
+       // GoroutineProfile used to use the wrong starting sp for
+       // goroutines coming out of system calls, causing possible
+       // crashes.
+       defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(100))
+
+       var stop uint32
+       defer atomic.StoreUint32(&stop, 1) // in case of panic
+
+       var wg sync.WaitGroup
+       for i := 0; i < 4; i++ {
+               wg.Add(1)
+               go func() {
+                       for atomic.LoadUint32(&stop) == 0 {
+                               syscall.Close(-1)
+                       }
+                       wg.Done()
+               }()
+       }
+
+       max := 10000
+       if testing.Short() {
+               max = 100
+       }
+       stk := make([]runtime.StackRecord, 100)
+       for n := 0; n < max; n++ {
+               _, ok := runtime.GoroutineProfile(stk)
+               if !ok {
+                       t.Fatalf("GoroutineProfile failed")
+               }
+       }
+
+       // If the program didn't crash, we passed.
+       atomic.StoreUint32(&stop, 1)
+       wg.Wait()
+}

diff --git a/src/pkg/runtime/traceback_arm.c b/src/pkg/runtime/traceback_arm.c
index 341aa20588010d7a3a0574dce69732a431084e7b..8a3685e76c508ee3d0ffa251bfdba5a88aaf3eee 100644 (file)
--- a/src/pkg/runtime/traceback_arm.c
+++ b/src/pkg/runtime/traceback_arm.c
@@ -20,6 +20,18 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
        Stktop *stk;
        String file;
 
+       if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
+               if(gp->syscallstack != (uintptr)nil) {
+                       pc0 = gp->syscallpc;
+                       sp0 = gp->syscallsp;
+                       lr0 = 0;
+               } else {
+                       pc0 = gp->sched.pc;
+                       sp0 = gp->sched.sp;
+                       lr0 = gp->sched.lr;
+               }
+       }
+
        nprint = 0;
        runtime·memclr((byte*)&frame, sizeof frame);
        frame.pc = pc0;

diff --git a/src/pkg/runtime/traceback_x86.c b/src/pkg/runtime/traceback_x86.c
index d658e8f11a94d32184833e38179d9ea5be4795a2..8e3063f43a64a89fe23f064d95059ef89795b4a3 100644 (file)
--- a/src/pkg/runtime/traceback_x86.c
+++ b/src/pkg/runtime/traceback_x86.c
@@ -30,6 +30,16 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
        String file;
 
        USED(lr0);
+       
+       if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
+               if(gp->syscallstack != (uintptr)nil) {
+                       pc0 = gp->syscallpc;
+                       sp0 = gp->syscallsp;
+               } else {
+                       pc0 = gp->sched.pc;
+                       sp0 = gp->sched.sp;
+               }
+       }
 
        nprint = 0;
        runtime·memclr((byte*)&frame, sizeof frame);