runtime: change Gobuf.g to uintptr, not pointer

The Gobuf.g goroutine pointer is almost always updated by assembly code.
In one of the few places it is updated by Go code - func save - it must be
treated as a uintptr to avoid a write barrier being emitted at a bad time.
Instead of figuring out how to emit the write barriers missing in the
assembly manipulation, change the type of the field to uintptr, so that
it does not require write barriers at all.

Goroutine structs are published in the allg list and never freed.
That will keep the goroutine structs from being collected.
There is never a time that Gobuf.g's contain the only references
to a goroutine: the publishing of the goroutine in allg comes first.

Goroutine pointers are also kept in non-GC-visible places like TLS,
so I can't see them ever moving. If we did want to start moving data
in the GC, we'd need to allocate the goroutine structs from an
alternate arena. This CL doesn't make that problem any worse.

Found with GODEBUG=wbshadow=1 mode.
Eventually that will run automatically, but right now
it still detects other missing write barriers.

Change-Id: I85f91312ec3e0ef69ead0fff1a560b0cfb095e1a
Reviewed-on: https://go-review.googlesource.com/2065
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index 00dbeda3f9c7b3fbdb7c70800da77463e5e7a062..6fcbd6946af621b75b80ffb62b659b04037d4e48 100644 (file)
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -908,7 +908,7 @@ func newextram() {
        gp.sched.sp = gp.stack.hi
        gp.sched.sp -= 4 * regSize // extra space in case of reads slightly beyond frame
        gp.sched.lr = 0
-       gp.sched.g = gp
+       gp.sched.g = guintptr(unsafe.Pointer(gp))
        gp.syscallpc = gp.sched.pc
        gp.syscallsp = gp.sched.sp
        // malg returns status as Gidle, change to Gsyscall before adding to allg
@@ -1580,8 +1580,7 @@ func save(pc, sp uintptr) {
        _g_.sched.lr = 0
        _g_.sched.ret = 0
        _g_.sched.ctxt = nil
-       // _g_.sched.g = _g_, but avoid write barrier, which smashes _g_.sched
-       *(*uintptr)(unsafe.Pointer(&_g_.sched.g)) = uintptr(unsafe.Pointer(_g_))
+       _g_.sched.g = guintptr(unsafe.Pointer(_g_))
 }
 
 // The goroutine g is about to enter a system call.
@@ -1984,7 +1983,7 @@ func newproc1(fn *funcval, argp *uint8, narg int32, nret int32, callerpc uintptr
        memclr(unsafe.Pointer(&newg.sched), unsafe.Sizeof(newg.sched))
        newg.sched.sp = sp
        newg.sched.pc = funcPC(goexit) + _PCQuantum // +PCQuantum so that previous instruction is in same function
-       newg.sched.g = newg
+       newg.sched.g = guintptr(unsafe.Pointer(newg))
        gostartcallfn(&newg.sched, fn)
        newg.gopc = callerpc
        casgstatus(newg, _Gdead, _Grunnable)

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 04c8440ebfa8f4dcc6fe1cd502377a533552998d..3afc67baff6eeb1b69f488941ec98b5a89875942 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -93,11 +93,35 @@ type slice struct {
        cap   uint  // allocated number of elements
 }
 
+// A guintptr holds a goroutine pointer, but typed as a uintptr
+// to bypass write barriers. It is used in the Gobuf goroutine state.
+//
+// The Gobuf.g goroutine pointer is almost always updated by assembly code.
+// In one of the few places it is updated by Go code - func save - it must be
+// treated as a uintptr to avoid a write barrier being emitted at a bad time.
+// Instead of figuring out how to emit the write barriers missing in the
+// assembly manipulation, we change the type of the field to uintptr,
+// so that it does not require write barriers at all.
+//
+// Goroutine structs are published in the allg list and never freed.
+// That will keep the goroutine structs from being collected.
+// There is never a time that Gobuf.g's contain the only references
+// to a goroutine: the publishing of the goroutine in allg comes first.
+// Goroutine pointers are also kept in non-GC-visible places like TLS,
+// so I can't see them ever moving. If we did want to start moving data
+// in the GC, we'd need to allocate the goroutine structs from an
+// alternate arena. Using guintptr doesn't make that problem any worse.
+type guintptr uintptr
+
+func (gp guintptr) ptr() *g {
+       return (*g)(unsafe.Pointer(gp))
+}
+
 type gobuf struct {
        // The offsets of sp, pc, and g are known to (hard-coded in) libmach.
        sp   uintptr
        pc   uintptr
-       g    *g
+       g    guintptr
        ctxt unsafe.Pointer // this has to be a pointer so that gc scans it
        ret  uintreg
        lr   uintptr