runtime: fix isSystemGoroutine for gccgo
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.
Change-Id: I17577250d26f955406d13a12b24c8604621e5c53
Reviewed-on: https://go-review.googlesource.com/43156
Reviewed-by: Than McIntosh <thanm@google.com>
diff --git a/libgo/go/runtime/mfinal.go b/libgo/go/runtime/mfinal.go
index f0123b3..615a2b2 100644
--- a/libgo/go/runtime/mfinal.go
+++ b/libgo/go/runtime/mfinal.go
@@ -100,8 +100,7 @@
}
var (
- fingCreate uint32
- fingRunning bool
+ fingCreate uint32
)
func createfing() {
@@ -113,17 +112,19 @@
// 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 @@
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, ¶m, nil)
- fingRunning = false
+ gp.isSystemGoroutine = true
// Drop finalizer queue heap references
// before hiding them from markroot.
diff --git a/libgo/go/runtime/mgc.go b/libgo/go/runtime/mgc.go
index abec9d3..f828e7c 100644
--- a/libgo/go/runtime/mgc.go
+++ b/libgo/go/runtime/mgc.go
@@ -1423,6 +1423,8 @@
}
func gcBgMarkWorker(_p_ *p) {
+ setSystemGoroutine()
+
gp := getg()
type parkInfo struct {
diff --git a/libgo/go/runtime/mgcsweep.go b/libgo/go/runtime/mgcsweep.go
index 9f24fb1..2b698bf 100644
--- a/libgo/go/runtime/mgcsweep.go
+++ b/libgo/go/runtime/mgcsweep.go
@@ -48,6 +48,8 @@
}
func bgsweep(c chan int) {
+ setSystemGoroutine()
+
sweep.g = getg()
lock(&sweep.lock)
diff --git a/libgo/go/runtime/proc.go b/libgo/go/runtime/proc.go
index b28e26b..64735e2 100644
--- a/libgo/go/runtime/proc.go
+++ b/libgo/go/runtime/proc.go
@@ -237,6 +237,8 @@
}
func forcegchelper() {
+ setSystemGoroutine()
+
forcegc.g = getg()
for {
lock(&forcegc.lock)
@@ -450,7 +452,6 @@
sched.maxmcount = 10000
- tracebackinit()
mallocinit()
mcommoninit(_g_.m)
alginit() // maps must not be used before this call
@@ -2688,9 +2689,6 @@
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 @@
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) {
diff --git a/libgo/go/runtime/runtime2.go b/libgo/go/runtime/runtime2.go
index 22847ea..8deb620 100644
--- a/libgo/go/runtime/runtime2.go
+++ b/libgo/go/runtime/runtime2.go
@@ -415,6 +415,8 @@
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
diff --git a/libgo/go/runtime/time.go b/libgo/go/runtime/time.go
index 604ccde..e85fc7a 100644
--- a/libgo/go/runtime/time.go
+++ b/libgo/go/runtime/time.go
@@ -152,6 +152,8 @@
// 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)
diff --git a/libgo/go/runtime/traceback_gccgo.go b/libgo/go/runtime/traceback_gccgo.go
index d060e09..fb51043 100644
--- a/libgo/go/runtime/traceback_gccgo.go
+++ b/libgo/go/runtime/traceback_gccgo.go
@@ -9,7 +9,7 @@
import (
"runtime/internal/sys"
- "unsafe"
+ _ "unsafe" // for go:linkname
)
// For gccgo, use go:linkname to rename compiler-called functions to
@@ -20,34 +20,6 @@
//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 @@
// 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) {
