Commit 6a86dbe7 authored by Austin Clements's avatar Austin Clements

runtime: always call stackfree on the system stack

Currently when the garbage collector frees stacks of dead goroutines
in markrootFreeGStacks, it calls stackfree on a regular user stack.
This is a problem, since stackfree manipulates the stack cache in the
per-P mcache, so if it grows the stack or gets preempted in the middle
of manipulating the stack cache (which are both possible since it's on
a user stack), it can easily corrupt the stack cache.

Fix this by calling markrootFreeGStacks on the system stack, so that
all calls to stackfree happen on the system stack. To prevent this bug
in the future, mark stack functions that manipulate the mcache as
go:systemstack.

Fixes #15853.

Change-Id: Ic0d1c181efb342f134285a152560c3a074f14a3d
Reviewed-on: https://go-review.googlesource.com/23511
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
Reviewed-by: default avatarRick Hudson <rlh@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 966baedf
......@@ -174,7 +174,9 @@ func markroot(gcw *gcWork, i uint32) {
// Only do this once per GC cycle; preferably
// concurrently.
if !work.markrootDone {
markrootFreeGStacks()
// Switch to the system stack so we can call
// stackfree.
systemstack(markrootFreeGStacks)
}
case baseSpans <= i && i < baseStacks:
......
......@@ -251,6 +251,8 @@ func stackpoolfree(x gclinkptr, order uint8) {
// stackcacherefill/stackcacherelease implement a global pool of stack segments.
// The pool is required to prevent unlimited growth of per-thread caches.
//
//go:systemstack
func stackcacherefill(c *mcache, order uint8) {
if stackDebug >= 1 {
print("stackcacherefill order=", order, "\n")
......@@ -272,6 +274,7 @@ func stackcacherefill(c *mcache, order uint8) {
c.stackcache[order].size = size
}
//go:systemstack
func stackcacherelease(c *mcache, order uint8) {
if stackDebug >= 1 {
print("stackcacherelease order=", order, "\n")
......@@ -290,6 +293,7 @@ func stackcacherelease(c *mcache, order uint8) {
c.stackcache[order].size = size
}
//go:systemstack
func stackcache_clear(c *mcache) {
if stackDebug >= 1 {
print("stackcache clear\n")
......@@ -308,6 +312,12 @@ func stackcache_clear(c *mcache) {
unlock(&stackpoolmu)
}
// stackalloc allocates an n byte stack.
//
// stackalloc must run on the system stack because it uses per-P
// resources and must not split the stack.
//
//go:systemstack
func stackalloc(n uint32) (stack, []stkbar) {
// Stackalloc must be called on scheduler stack, so that we
// never try to grow the stack during the code that stackalloc runs.
......@@ -405,6 +415,12 @@ func stackalloc(n uint32) (stack, []stkbar) {
return stack{uintptr(v), uintptr(v) + top}, *(*[]stkbar)(unsafe.Pointer(&stkbarSlice))
}
// stackfree frees an n byte stack allocation at stk.
//
// stackfree must run on the system stack because it uses per-P
// resources and must not split the stack.
//
//go:systemstack
func stackfree(stk stack, n uintptr) {
gp := getg()
v := unsafe.Pointer(stk.lo)
......
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