runtime: dropg before CAS g status to _Grunnable/_Gwaiting

    Currently, we dropg (which clears gp.m) after we CAS the g status
    to _Grunnable or _Gwaiting. Immediately after CASing the g status,
    another thread may CAS it to _Gscan status and scan its stack.
    With precise stack scan, it accesses gp.m in order to switch to g
    and back (in doscanstackswitch). This races with dropg. If
    doscanstackswitch reads gp.m, then dropg runs, when we restore
    the m at the end of the scan it will set to a stale value. Worse,
    if dropg runs after doscanstackswitch sets the new m, gp will be
    running with a nil m.

    To fix this, we do dropg before CAS g status to _Grunnable or
    _Gwaiting. We can do this safely if we are CASing from _Grunning,
    as we own the g when it is in _Grunning. There is one case where
    we CAS from _Gsyscall to _Grunnable. It is not safe to dropg when
    it is in _Gsyscall, as precise stack scan needs to read gp.m in
    order to signal the m. So we need to introduce a transient state,
    _Gexitingsyscall, between _Gsyscall and _Grunnable, where the GC
    should not scan its stack.

    In is a little unfortunate that we have to add another g status.
    We could reuse an existing one (e.g. _Gcopystack), but it is
    clearer and safer to just use a new one, as Austin suggested.

    Reviewed-on: https://go-review.googlesource.com/c/158157

From-SVN: r268001

diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 67abd997051b49ef9ad2e2ac5713858d8331118a..94604b270aeb98dfa8a3b70940b44c319eb0c0ed 100644 (file)
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-ee94431c133a90ca5c3c5ebbebcb019c60258dac
+d6576c83016d856217758c06d945bfc363ffb817
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.

diff --git a/libgo/go/runtime/proc.go b/libgo/go/runtime/proc.go
index ef166cb9d28d57d35f658b4313e19d7326022700..655d0a9c760642ccd19303b468d53b01e949aa4c 100644 (file)
--- a/libgo/go/runtime/proc.go
+++ b/libgo/go/runtime/proc.go
@@ -956,6 +956,10 @@ loop:
                                break loop
                        }
 
+               case _Gexitingsyscall:
+                       // This is a transient state during which we should not scan its stack.
+                       // Try again.
+
                case _Gscanwaiting:
                        // newstack is doing a scan for us right now. Wait.
 
@@ -2635,8 +2639,8 @@ func park_m(gp *g) {
                traceGoPark(_g_.m.waittraceev, _g_.m.waittraceskip)
        }
 
-       casgstatus(gp, _Grunning, _Gwaiting)
        dropg()
+       casgstatus(gp, _Grunning, _Gwaiting)
 
        if _g_.m.waitunlockf != nil {
                fn := *(*func(*g, unsafe.Pointer) bool)(unsafe.Pointer(&_g_.m.waitunlockf))
@@ -2660,8 +2664,8 @@ func goschedImpl(gp *g) {
                dumpgstatus(gp)
                throw("bad g status")
        }
-       casgstatus(gp, _Grunning, _Grunnable)
        dropg()
+       casgstatus(gp, _Grunning, _Grunnable)
        lock(&sched.lock)
        globrunqput(gp)
        unlock(&sched.lock)
@@ -3054,8 +3058,9 @@ func exitsyscallfast_pidle() bool {
 func exitsyscall0(gp *g) {
        _g_ := getg()
 
-       casgstatus(gp, _Gsyscall, _Grunnable)
+       casgstatus(gp, _Gsyscall, _Gexitingsyscall)
        dropg()
+       casgstatus(gp, _Gexitingsyscall, _Grunnable)
        lock(&sched.lock)
        _p_ := pidleget()
        if _p_ == nil {

diff --git a/libgo/go/runtime/runtime2.go b/libgo/go/runtime/runtime2.go
index 6eb9491018d7acc22f0066af9494120fb98c4e86..0e9cf63c10dc52a04d463c4df94cf6b85b0e0657 100644 (file)
--- a/libgo/go/runtime/runtime2.go
+++ b/libgo/go/runtime/runtime2.go
@@ -70,6 +70,12 @@ const (
        // stack is owned by the goroutine that put it in _Gcopystack.
        _Gcopystack // 8
 
+       // _Gexitingsyscall means this goroutine is exiting from a
+       // system call. This is like _Gsyscall, but the GC should not
+       // scan its stack. Currently this is only used in exitsyscall0
+       // as a transient state when it drops the G.
+       _Gexitingsyscall // 9
+
        // _Gscan combined with one of the above states other than
        // _Grunning indicates that GC is scanning the stack. The
        // goroutine is not executing user code and the stack is owned

diff --git a/libgo/go/runtime/traceback_gccgo.go b/libgo/go/runtime/traceback_gccgo.go
index 7347cea71b22ea0597797957eca00d0910229237..72a83a5ab2d72ad8fc48286c3ab84c64591e82ac 100644 (file)
--- a/libgo/go/runtime/traceback_gccgo.go
+++ b/libgo/go/runtime/traceback_gccgo.go
@@ -122,13 +122,14 @@ func isExportedRuntime(name string) bool {
 }
 
 var gStatusStrings = [...]string{
-       _Gidle:      "idle",
-       _Grunnable:  "runnable",
-       _Grunning:   "running",
-       _Gsyscall:   "syscall",
-       _Gwaiting:   "waiting",
-       _Gdead:      "dead",
-       _Gcopystack: "copystack",
+       _Gidle:           "idle",
+       _Grunnable:       "runnable",
+       _Grunning:        "running",
+       _Gsyscall:        "syscall",
+       _Gwaiting:        "waiting",
+       _Gdead:           "dead",
+       _Gcopystack:      "copystack",
+       _Gexitingsyscall: "exiting syscall",
 }
 
 func goroutineheader(gp *g) {