runtime: fix isSystemGoroutine for gccgo
authorIan Lance Taylor <ian@gcc.gnu.org>
Thu, 11 May 2017 20:55:41 +0000 (20:55 +0000)
committerIan Lance Taylor <ian@gcc.gnu.org>
Thu, 11 May 2017 20:55:41 +0000 (20:55 +0000)
    The gc toolchain decides whether a goroutine is a system goroutine by
    comparing startpc to a list of saved special PCs.  In gccgo that
    approach does not work as startpc is often a thunk that invokes the
    real function with arguments, so the thunk address never matches the
    saved special PCs.

    This patch fixes gccgo's understanding of system goroutines.  Since
    there are only a limited number of them, we simply change each one to
    mark itself as special.

    This fixes stack dumps and functions like runtime.NumGoroutine to
    behave more like gc.  It also fixes the goprint test in the gc
    testsuite.

    Reviewed-on: https://go-review.googlesource.com/43156

From-SVN: r247931

gcc/go/gofrontend/MERGE
libgo/go/runtime/mfinal.go
libgo/go/runtime/mgc.go
libgo/go/runtime/mgcsweep.go
libgo/go/runtime/proc.go
libgo/go/runtime/runtime2.go
libgo/go/runtime/time.go
libgo/go/runtime/traceback_gccgo.go

index 1082abd40b5cf00898d327d0ca6c9f0454847f1c..a0e661bd61bc5eda9c1ebfee1912d8088c204a09 100644 (file)
@@ -1,4 +1,4 @@
-822ab419bf7d1c705cdce1c12133e7a11f56be2e
+619848ccd463ac385e9912df008e7e8e6301a284
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
index f0123b399f25607261813cd1fa615a14bf9bda9f..615a2b295222fa86bed4bc7a870b98ab555b4de0 100644 (file)
@@ -100,8 +100,7 @@ func wakefing() *g {
 }
 
 var (
-       fingCreate  uint32
-       fingRunning bool
+       fingCreate uint32
 )
 
 func createfing() {
@@ -113,17 +112,19 @@ func createfing() {
 
 // This is the goroutine that runs all of the finalizers
 func runfinq() {
+       setSystemGoroutine()
+
        var (
                ef   eface
                ifac iface
        )
 
+       gp := getg()
        for {
                lock(&finlock)
                fb := finq
                finq = nil
                if fb == nil {
-                       gp := getg()
                        fing = gp
                        fingwait = true
                        goparkunlock(&finlock, "finalizer wait", traceEvGoBlock, 1)
@@ -160,9 +161,17 @@ func runfinq() {
                                default:
                                        throw("bad kind in runfinq")
                                }
-                               fingRunning = true
+                               // This is not a system goroutine while
+                               // running the actual finalizer.
+                               // This matters because we want this
+                               // goroutine to appear in a stack dump
+                               // if the finalizer crashes.
+                               // The gc toolchain handles this using
+                               // a global variable fingRunning,
+                               // but we don't need that.
+                               gp.isSystemGoroutine = false
                                reflectcall(f.ft, f.fn, false, false, &param, nil)
-                               fingRunning = false
+                               gp.isSystemGoroutine = true
 
                                // Drop finalizer queue heap references
                                // before hiding them from markroot.
index abec9d3cd247b0f8a816437cdda83345f1c9c147..f828e7c28f328b4d343c001372955adbf4ac9e0b 100644 (file)
@@ -1423,6 +1423,8 @@ func gcBgMarkPrepare() {
 }
 
 func gcBgMarkWorker(_p_ *p) {
+       setSystemGoroutine()
+
        gp := getg()
 
        type parkInfo struct {
index 9f24fb1b628340fb6842c09f72caa91860d127ff..2b698bf74a506dfcf404e4873a788ccb4b75b87a 100644 (file)
@@ -48,6 +48,8 @@ func finishsweep_m() {
 }
 
 func bgsweep(c chan int) {
+       setSystemGoroutine()
+
        sweep.g = getg()
 
        lock(&sweep.lock)
index b28e26bec6dc4b139954478cdf5bb12210fde0bc..64735e2a653259f1f641939507989bb6c6b3e896 100644 (file)
@@ -237,6 +237,8 @@ func init() {
 }
 
 func forcegchelper() {
+       setSystemGoroutine()
+
        forcegc.g = getg()
        for {
                lock(&forcegc.lock)
@@ -450,7 +452,6 @@ func schedinit() {
 
        sched.maxmcount = 10000
 
-       tracebackinit()
        mallocinit()
        mcommoninit(_g_.m)
        alginit() // maps must not be used before this call
@@ -2688,9 +2689,6 @@ func newproc(fn uintptr, arg unsafe.Pointer) *g {
        newg.param = arg
        newg.gopc = getcallerpc(unsafe.Pointer(&fn))
        newg.startpc = fn
-       if isSystemGoroutine(newg) {
-               atomic.Xadd(&sched.ngsys, +1)
-       }
        // The stack is dirty from the argument frame, so queue it for
        // scanning. Do this before setting it to runnable so we still
        // own the G. If we're recycling a G, it may already be on the
@@ -2729,6 +2727,18 @@ func newproc(fn uintptr, arg unsafe.Pointer) *g {
        return newg
 }
 
+// setSystemGoroutine marks this goroutine as a "system goroutine".
+// In the gc toolchain this is done by comparing startpc to a list of
+// saved special PCs. In gccgo that approach does not work as startpc
+// is often a thunk that invokes the real function with arguments,
+// so the thunk address never matches the saved special PCs. Instead,
+// since there are only a limited number of "system goroutines",
+// we force each one to mark itself as special.
+func setSystemGoroutine() {
+       getg().isSystemGoroutine = true
+       atomic.Xadd(&sched.ngsys, +1)
+}
+
 // Put on gfree list.
 // If local list is too long, transfer a batch to the global list.
 func gfput(_p_ *p, gp *g) {
index 22847eaf59850f44c34e1ba323f47075fe580619..8deb6202bae4fa2168fa8e952c3f841202a39dab 100644 (file)
@@ -415,6 +415,8 @@ type g struct {
 
        scanningself bool // whether goroutine is scanning its own stack
 
+       isSystemGoroutine bool // whether goroutine is a "system" goroutine
+
        traceback *tracebackg // stack traceback buffer
 
        context      g_ucontext_t // saved context for setcontext
index 604ccded8953ba96737d2dc4808c201aa40d062d..e85fc7a54fa2af5cbad857c665ba77cbe1748914 100644 (file)
@@ -152,6 +152,8 @@ func deltimer(t *timer) bool {
 // It sleeps until the next event in the timers heap.
 // If addtimer inserts a new earlier event, it wakes timerproc early.
 func timerproc() {
+       setSystemGoroutine()
+
        timers.gp = getg()
        for {
                lock(&timers.lock)
index d060e09162751a442810fe1fcf718ca3fd42c1bc..fb51043e13da992c79a8c4764ab14aabb95d7c6d 100644 (file)
@@ -9,7 +9,7 @@ package runtime
 
 import (
        "runtime/internal/sys"
-       "unsafe"
+       _ "unsafe" // for go:linkname
 )
 
 // For gccgo, use go:linkname to rename compiler-called functions to
@@ -20,34 +20,6 @@ import (
 //go:linkname goroutineheader runtime.goroutineheader
 //go:linkname printcreatedby runtime.printcreatedby
 
-var (
-       // initialized in tracebackinit
-       runfinqPC        uintptr
-       bgsweepPC        uintptr
-       forcegchelperPC  uintptr
-       timerprocPC      uintptr
-       gcBgMarkWorkerPC uintptr
-)
-
-func tracebackinit() {
-       // Go variable initialization happens late during runtime startup.
-       // Instead of initializing the variables above in the declarations,
-       // schedinit calls this function so that the variables are
-       // initialized and available earlier in the startup sequence.
-       // This doesn't use funcPC to avoid memory allocation.
-       // FIXME: We should be able to use funcPC when escape analysis is on.
-       f1 := runfinq
-       runfinqPC = **(**uintptr)(unsafe.Pointer(&f1))
-       f2 := bgsweep
-       bgsweepPC = **(**uintptr)(unsafe.Pointer(&f2))
-       f3 := forcegchelper
-       forcegchelperPC = **(**uintptr)(unsafe.Pointer(&f3))
-       f4 := timerproc
-       timerprocPC = **(**uintptr)(unsafe.Pointer(&f4))
-       f5 := gcBgMarkWorker
-       gcBgMarkWorkerPC = **(**uintptr)(unsafe.Pointer(&f5))
-}
-
 func printcreatedby(gp *g) {
        // Show what created goroutine, except main goroutine (goid 1).
        pc := gp.gopc
@@ -196,15 +168,7 @@ func goroutineheader(gp *g) {
 // isSystemGoroutine reports whether the goroutine g must be omitted in
 // stack dumps and deadlock detector.
 func isSystemGoroutine(gp *g) bool {
-       // FIXME: This doesn't work reliably for gccgo because in many
-       // cases the startpc field will be set to a thunk rather than
-       // to one of these addresses.
-       pc := gp.startpc
-       return pc == runfinqPC && !fingRunning ||
-               pc == bgsweepPC ||
-               pc == forcegchelperPC ||
-               pc == timerprocPC ||
-               pc == gcBgMarkWorkerPC
+       return gp.isSystemGoroutine
 }
 
 func tracebackothers(me *g) {