runtime: clear g0 stack bounds in dropm

After CL 527715, needm uses callbackUpdateSystemStack to set the stack
bounds for g0 on an M from the extra M list. Since
callbackUpdateSystemStack is also used for recursive cgocallback, it
does nothing if the stack is already in bounds.

Currently, the stack bounds in an extra M may contain stale bounds from
a previous thread that used this M and then returned it to the extra
list in dropm.

Typically a new thread will not have an overlapping stack with an old
thread, but because the old thread has exited there is a small chance
that the C memory allocator will allocate the new thread's stack
partially or fully overlapping with the old thread's stack.

If this occurs, then callbackUpdateSystemStack will not update the stack
bounds. If in addition, the overlap is partial such that SP on
cgocallback is close to the recorded stack lower bound, then Go may
quickly "overflow" the stack and crash with "morestack on g0".

Fix this by clearing the stack bounds in dropm, which ensures that
callbackUpdateSystemStack will unconditionally update the bounds in
needm.

For #62440.

Change-Id: Ic9e2052c2090dd679ed716d1a23a86d66cbcada7
Reviewed-on: https://go-review.googlesource.com/c/go/+/537695
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index fa76d3250cc8aff89c3b577f6f708e9383a84631..d560d3970ec72e6f3c803a9bd3121fadbaf165f4 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -2187,6 +2187,14 @@ func dropm() {
 
        setg(nil)
 
+       // Clear g0 stack bounds to ensure that needm always refreshes the
+       // bounds when reusing this M.
+       g0 := mp.g0
+       g0.stack.hi = 0
+       g0.stack.lo = 0
+       g0.stackguard0 = 0
+       g0.stackguard1 = 0
+
        putExtraM(mp)
 
        msigrestore(sigmask)

diff --git a/src/runtime/testdata/testprogcgo/stackswitch.c b/src/runtime/testdata/testprogcgo/stackswitch.c
index 2f79cc28ed95104469f88fe64f409863589939aa..3473d5bd57d0569607df6a1feae98913bee24457 100644 (file)
--- a/src/runtime/testdata/testprogcgo/stackswitch.c
+++ b/src/runtime/testdata/testprogcgo/stackswitch.c
@@ -43,6 +43,8 @@ static ucontext_t uctx_save, uctx_switch;
 
 extern void stackSwitchCallback(void);
 
+char *stack2;
+
 static void *stackSwitchThread(void *arg) {
        // Simple test: callback works from the normal system stack.
        stackSwitchCallback();
@@ -57,7 +59,9 @@ static void *stackSwitchThread(void *arg) {
 
        // Allocate the second stack before freeing the first to ensure we don't get
        // the same address from malloc.
-       char *stack2 = malloc(STACK_SIZE);
+       //
+       // Will be freed in stackSwitchThread2.
+       stack2 = malloc(STACK_SIZE);
        if (stack1 == NULL) {
                perror("malloc");
                exit(1);
@@ -92,6 +96,40 @@ static void *stackSwitchThread(void *arg) {
        }
 
        free(stack1);
+
+       return NULL;
+}
+
+static void *stackSwitchThread2(void *arg) {
+       // New thread. Use stack bounds that partially overlap the previous
+       // bounds. needm should refresh the stack bounds anyway since this is a
+       // new thread.
+
+       // N.B. since we used a custom stack with makecontext,
+       // callbackUpdateSystemStack had to guess the bounds. Its guess assumes
+       // a 32KiB stack.
+       char *prev_stack_lo = stack2 + STACK_SIZE - (32*1024);
+
+       // New SP is just barely in bounds, but if we don't update the bounds
+       // we'll almost certainly overflow. The SP that
+       // callbackUpdateSystemStack sees already has some data pushed, so it
+       // will be a bit below what we set here. Thus we include some slack.
+       char *new_stack_hi = prev_stack_lo + 128;
+
+       if (getcontext(&uctx_switch) == -1) {
+               perror("getcontext");
+               exit(1);
+       }
+       uctx_switch.uc_stack.ss_sp = new_stack_hi - (STACK_SIZE / 2);
+       uctx_switch.uc_stack.ss_size = STACK_SIZE / 2;
+       uctx_switch.uc_link = &uctx_save;
+       makecontext(&uctx_switch, stackSwitchCallback, 0);
+
+       if (swapcontext(&uctx_save, &uctx_switch) == -1) {
+               perror("swapcontext");
+               exit(1);
+       }
+
        free(stack2);
 
        return NULL;
@@ -101,6 +139,9 @@ void callStackSwitchCallbackFromThread(void) {
        pthread_t thread;
        assert(pthread_create(&thread, NULL, stackSwitchThread, NULL) == 0);
        assert(pthread_join(thread, NULL) == 0);
+
+       assert(pthread_create(&thread, NULL, stackSwitchThread2, NULL) == 0);
+       assert(pthread_join(thread, NULL) == 0);
 }
 
 #endif