runtime: account for stack guard when shrinking the stack

Currently, when shrinkstack computes whether the halved stack
allocation will have enough room for the stack, it accounts for the
stack space that's actively in use but fails to leave extra room for
the stack guard space. As a result, *if* the minimum stack size is
small enough or the guard large enough, it may shrink the stack and
leave less than enough room to run nosplit functions. If the next
function called after the stack shrink is a nosplit function, it may
overflow the stack without noticing and overwrite non-stack memory.

We don't think this is happening under normal conditions right now.
The minimum stack allocation is 2K and the guard is 640 bytes. The
"worst case" stack shrink is from 4K (4048 bytes after stack barrier
array reservation) to 2K (2016 bytes after stack barrier array
reservation), which means the largest "used" size that will qualify
for shrinking is 4048/4 - 8 = 1004 bytes. After copying, that leaves
2016 - 1004 = 1012 bytes of available stack, which is significantly
more than the guard space.

If we were to reduce the minimum stack size to 1K or raise the guard
space above 1012 bytes, the logic in shrinkstack would no longer leave
enough space.

It's also possible to trigger this problem by setting
firstStackBarrierOffset to 0, which puts stack barriers in a debug
mode that steals away *half* of the stack for the stack barrier array
reservation. Then, the largest "used" size that qualifies for
shrinking is (4096/2)/4 - 8 = 504 bytes. After copying, that leaves
(2096/2) - 504 = 8 bytes of available stack; much less than the
required guard space. This causes failures like those in issue #11027
because func gc() shrinks its own stack and then immediately calls
casgstatus (a nosplit function), which overflows the stack and
overwrites a free list pointer in the neighboring span. However, since
this seems to require the special debug mode, we don't think it's
responsible for issue #11027.

To forestall all of these subtle issues, this commit modifies
shrinkstack to correctly account for the guard space when considering
whether to halve the stack allocation.

Change-Id: I7312584addc63b5bfe55cc384a1012f6181f1b9d
Reviewed-on: https://go-review.googlesource.com/10714
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/stack1.go b/src/runtime/stack1.go
index 933e3d0b2868ae222da8300988647423bc9598fa..cb2110efb6b89f789cc6c45d6ebb7ffd320b199b 100644 (file)
--- a/src/runtime/stack1.go
+++ b/src/runtime/stack1.go
@@ -575,7 +575,7 @@ func copystack(gp *g, newsize uintptr) {
                fillstack(new, 0xfd)
        }
        if stackDebug >= 1 {
-               print("copystack gp=", gp, " [", hex(old.lo), " ", hex(old.hi-used), " ", hex(old.hi), "]/", old.hi-old.lo, " -> [", hex(new.lo), " ", hex(new.hi-used), " ", hex(new.hi), "]/", newsize, "\n")
+               print("copystack gp=", gp, " [", hex(old.lo), " ", hex(old.hi-used), " ", hex(old.hi), "]/", gp.stackAlloc, " -> [", hex(new.lo), " ", hex(new.hi-used), " ", hex(new.hi), "]/", newsize, "\n")
        }
 
        // adjust pointers in the to-be-copied frames
@@ -832,12 +832,19 @@ func shrinkstack(gp *g) {
 
        oldsize := gp.stackAlloc
        newsize := oldsize / 2
+       // Don't shrink the allocation below the minimum-sized stack
+       // allocation.
        if newsize < _FixedStack {
-               return // don't shrink below the minimum-sized stack
+               return
        }
-       used := gp.stack.hi - gp.sched.sp
-       if used >= oldsize/4 {
-               return // still using at least 1/4 of the segment.
+       // Compute how much of the stack is currently in use and only
+       // shrink the stack if gp is using less than a quarter of its
+       // current stack. The currently used stack includes everything
+       // down to the SP plus the stack guard space that ensures
+       // there's room for nosplit functions.
+       avail := gp.stack.hi - gp.stack.lo
+       if used := gp.stack.hi - gp.sched.sp + _StackLimit; used >= avail/4 {
+               return
        }
 
        // We can't copy the stack if we're in a syscall.