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 43cea9008a33788a298181ab4e40c5b8157cf531..715b2da232202e35565d99ad091f163808bf924d 100644 (file)
--- 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 6cbb5f3775088856dba748cf6a043799b6f2b892..7b4a8461958c9233a30197398c484b99d67ab4ae 100644 (file)
--- 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 28afa0dfab1215f168dbf39342bd78c7ab0843f5..525aa0955a194054f65ea0bd1e65475ebd8288fc 100644 (file)
--- 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 bbd786d519949724f47175fa1c32b3be93392e6b..f54d93377d4137d6a41660236c1e79d238aca941 100644 (file)
--- 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 c8f6de1ac8ac6b62cc47b5920619a1de17e91117..0411d965a4e5757d2bde7494c844fb86aeffc25f 100644 (file)
--- 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 70addbffad5cfdee783e443bb1303fca9ee3cad9..1f3ae500fc628befc01b3e4de4aa2adc6682baf4 100644 (file)
--- 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 e38d11a59da08bc6ab46f91de2e2b81a6b64a5d3..fab2ccbebf00a212bb71b4495fc1cbe118c715b5 100644 (file)
--- 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 50895ca8ec7d7d743020fafef45732b84319ace9..6a2cc2136cfa74ee8c880d632ee0e2962a79f257 100644 (file)
--- 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 6c87d7e2e4581a4d26366afd1aeef2da8dfeb0a5..8c31c5abad36e75c366d20605a9c323b9108e74d 100644 (file)
--- 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
+}