From: Dmitry Vyukov <dvyukov@google.com>
Date: Sat, 7 Feb 2015 12:31:18 +0000 (+0300)
Subject: runtime: never show system goroutines in traceback
X-Git-Tag: go1.5beta1~2056
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=59495e8dfda0cfc1fa527337b2fed8a8099137dc;p=gostls13.git

runtime: never show system goroutines in traceback

Fixes #9791

g.issystem flag setup races with other code wherever we set it.
Even if we set both in parent goroutine and in the system goroutine,
it is still possible that some other goroutine crashes
before the flag is set. We could pass issystem flag to newproc1,
but we start all goroutines with go nowadays.

Instead look at g.startpc to distinguish system goroutines (similar to topofstack).

Change-Id: Ia3467968dee27fa07d9fecedd4c2b00928f26645
Reviewed-on: https://go-review.googlesource.com/4113
Reviewed-by: Keith Randall <khr@golang.org>
---

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index 43cea9008a..715b2da232 100644
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime"
 	"strings"
 	"sync"
@@ -17,17 +18,20 @@ import (
 	"text/template"
 )
 
-// testEnv excludes GODEBUG from the environment
-// to prevent its output from breaking tests that
-// are trying to parse other command output.
 func testEnv(cmd *exec.Cmd) *exec.Cmd {
 	if cmd.Env != nil {
 		panic("environment already set")
 	}
 	for _, env := range os.Environ() {
+		// Exclude GODEBUG from the environment to prevent its output
+		// from breaking tests that are trying to parse other command output.
 		if strings.HasPrefix(env, "GODEBUG=") {
 			continue
 		}
+		// Exclude GOTRACEBACK for the same reason.
+		if strings.HasPrefix(env, "GOTRACEBACK=") {
+			continue
+		}
 		cmd.Env = append(cmd.Env, env)
 	}
 	return cmd
@@ -217,6 +221,14 @@ func TestMainGoroutineId(t *testing.T) {
 	}
 }
 
+func TestNoHelperGoroutines(t *testing.T) {
+	output := executeTest(t, noHelperGoroutinesSource, nil)
+	matches := regexp.MustCompile(`goroutine [0-9]+ \[`).FindAllStringSubmatch(output, -1)
+	if len(matches) != 1 || matches[0][0] != "goroutine 1 [" {
+		t.Fatalf("want to see only goroutine 1, see:\n%s", output)
+	}
+}
+
 func TestBreakpoint(t *testing.T) {
 	output := executeTest(t, breakpointSource, nil)
 	want := "runtime.Breakpoint()"
@@ -431,6 +443,22 @@ func main() {
 }
 `
 
+const noHelperGoroutinesSource = `
+package main
+import (
+	"runtime"
+	"time"
+)
+func init() {
+	i := 0
+	runtime.SetFinalizer(&i, func(p *int) {})
+	time.AfterFunc(time.Hour, func() {})
+	panic("oops")
+}
+func main() {
+}
+`
+
 const breakpointSource = `
 package main
 import "runtime"
diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
index 6cbb5f3775..7b4a846195 100644
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -344,7 +344,7 @@ func dumpgoroutine(gp *g) {
 	dumpint(uint64(gp.goid))
 	dumpint(uint64(gp.gopc))
 	dumpint(uint64(readgstatus(gp)))
-	dumpbool(gp.issystem)
+	dumpbool(isSystemGoroutine(gp))
 	dumpbool(false) // isbackground
 	dumpint(uint64(gp.waitsince))
 	dumpstr(gp.waitreason)
diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go
index 28afa0dfab..525aa0955a 100644
--- a/src/runtime/mfinal.go
+++ b/src/runtime/mfinal.go
@@ -102,7 +102,10 @@ func wakefing() *g {
 	return res
 }
 
-var fingCreate uint32
+var (
+	fingCreate  uint32
+	fingRunning bool
+)
 
 func createfing() {
 	// start the finalizer goroutine exactly once
@@ -126,9 +129,7 @@ func runfinq() {
 			gp := getg()
 			fing = gp
 			fingwait = true
-			gp.issystem = true
 			goparkunlock(&finlock, "finalizer wait", traceEvGoBlock)
-			gp.issystem = false
 			continue
 		}
 		unlock(&finlock)
@@ -169,7 +170,9 @@ func runfinq() {
 				default:
 					throw("bad kind in runfinq")
 				}
+				fingRunning = true
 				reflectcall(nil, unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz))
+				fingRunning = false
 
 				// drop finalizer queue references to finalized object
 				f.fn = nil
diff --git a/src/runtime/mgc0.go b/src/runtime/mgc0.go
index bbd786d519..f54d93377d 100644
--- a/src/runtime/mgc0.go
+++ b/src/runtime/mgc0.go
@@ -62,7 +62,6 @@ func clearpools() {
 // bggc holds the state of the backgroundgc.
 func backgroundgc() {
 	bggc.g = getg()
-	bggc.g.issystem = true
 	for {
 		gcwork(0)
 		lock(&bggc.lock)
@@ -73,7 +72,6 @@ func backgroundgc() {
 
 func bgsweep() {
 	sweep.g = getg()
-	getg().issystem = true
 	for {
 		for gosweepone() != ^uintptr(0) {
 			sweep.nbgsweep++
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index c8f6de1ac8..0411d965a4 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -110,7 +110,6 @@ func init() {
 
 func forcegchelper() {
 	forcegc.g = getg()
-	forcegc.g.issystem = true
 	for {
 		lock(&forcegc.lock)
 		if forcegc.idle != 0 {
diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index 70addbffad..1f3ae500fc 100644
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -2636,7 +2636,7 @@ func checkdead() {
 	lock(&allglock)
 	for i := 0; i < len(allgs); i++ {
 		gp := allgs[i]
-		if gp.issystem {
+		if isSystemGoroutine(gp) {
 			continue
 		}
 		s := readgstatus(gp)
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index e38d11a59d..fab2ccbebf 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -199,7 +199,6 @@ type g struct {
 	waitsince    int64  // approx time when the g become blocked
 	waitreason   string // if status==gwaiting
 	schedlink    *g
-	issystem     bool // do not output in stack dump, ignore in deadlock detector
 	preempt      bool // preemption signal, duplicates stackguard0 = stackpreempt
 	paniconfault bool // panic (instead of crash) on unexpected fault address
 	preemptscan  bool // preempted g does scan for gc
diff --git a/src/runtime/time.go b/src/runtime/time.go
index 50895ca8ec..6a2cc2136c 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -153,7 +153,6 @@ func deltimer(t *timer) bool {
 // If addtimer inserts a new earlier event, addtimer1 wakes timerproc early.
 func timerproc() {
 	timers.gp = getg()
-	timers.gp.issystem = true
 	for {
 		lock(&timers.lock)
 		timers.sleeping = false
diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 6c87d7e2e4..8c31c5abad 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -39,6 +39,11 @@ var (
 	mstartPC             uintptr
 	rt0_goPC             uintptr
 	sigpanicPC           uintptr
+	runfinqPC            uintptr
+	backgroundgcPC       uintptr
+	bgsweepPC            uintptr
+	forcegchelperPC      uintptr
+	timerprocPC          uintptr
 	systemstack_switchPC uintptr
 
 	externalthreadhandlerp uintptr // initialized elsewhere
@@ -56,6 +61,11 @@ func tracebackinit() {
 	mstartPC = funcPC(mstart)
 	rt0_goPC = funcPC(rt0_go)
 	sigpanicPC = funcPC(sigpanic)
+	runfinqPC = funcPC(runfinq)
+	backgroundgcPC = funcPC(backgroundgc)
+	bgsweepPC = funcPC(bgsweep)
+	forcegchelperPC = funcPC(forcegchelper)
+	timerprocPC = funcPC(timerproc)
 	systemstack_switchPC = funcPC(systemstack_switch)
 }
 
@@ -606,7 +616,7 @@ func tracebackothers(me *g) {
 
 	lock(&allglock)
 	for _, gp := range allgs {
-		if gp == me || gp == g.m.curg || readgstatus(gp) == _Gdead || gp.issystem && level < 2 {
+		if gp == me || gp == g.m.curg || readgstatus(gp) == _Gdead || isSystemGoroutine(gp) && level < 2 {
 			continue
 		}
 		print("\n")
@@ -631,3 +641,14 @@ func topofstack(f *_func) bool {
 		pc == rt0_goPC ||
 		externalthreadhandlerp != 0 && pc == externalthreadhandlerp
 }
+
+// isSystemGoroutine returns true if the goroutine g must be omitted in
+// stack dumps and deadlock detector.
+func isSystemGoroutine(gp *g) bool {
+	pc := gp.startpc
+	return pc == runfinqPC && !fingRunning ||
+		pc == backgroundgcPC ||
+		pc == bgsweepPC ||
+		pc == forcegchelperPC ||
+		pc == timerprocPC
+}