Commit eafc482d authored by Russ Cox's avatar Russ Cox

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/2065Reviewed-by: default avatarRick Hudson <rlh@golang.org>
Reviewed-by: default avatarAustin Clements <austin@google.com>
parent a73c1cef
......@@ -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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment