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>

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"

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)

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

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++

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 {

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)

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

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

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
+}