diff options
| author | Michael Pratt <mpratt@google.com> | 2023-09-04 09:55:01 -0400 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2023-09-12 17:08:55 +0000 |
| commit | 4f9fe6d50965020053ab80bf115f08070ce97f33 (patch) | |
| tree | 54afd47fc5f4313feba8cdc1af940f57cf514d52 /src/runtime/testdata | |
| parent | 2399302a251ef54e4669f2065f656b61d2f5c323 (diff) | |
| download | go-4f9fe6d50965020053ab80bf115f08070ce97f33.tar.xz | |
runtime: allow update of system stack bounds on callback from C thread
[This is a redo of CL 525455 with the test fixed on darwin by defining
_XOPEN_SOURCE, and disabled with android, musl, and openbsd, which do
not provide getcontext.]
Since CL 495855, Ms are cached for C threads calling into Go, including
the stack bounds of the system stack.
Some C libraries (e.g., coroutine libraries) do manual stack management
and may change stacks between calls to Go on the same thread.
Changing the stack if there is more Go up the stack would be
problematic. But if the calls are completely independent there is no
particular reason for Go to care about the changing stack boundary.
Thus, this CL allows the stack bounds to change in such cases. The
primary downside here (besides additional complexity) is that normal
systems that do not manipulate the stack may not notice unintentional
stack corruption as quickly as before.
Note that callbackUpdateSystemStack is written to be usable for the
initial setup in needm as well as updating the stack in cgocallbackg.
Fixes #62440.
For #62130.
Change-Id: I0fe0134f865932bbaff1fc0da377c35c013bd768
Reviewed-on: https://go-review.googlesource.com/c/go/+/527715
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Diffstat (limited to 'src/runtime/testdata')
| -rw-r--r-- | src/runtime/testdata/testprogcgo/stackswitch.c | 104 | ||||
| -rw-r--r-- | src/runtime/testdata/testprogcgo/stackswitch.go | 43 |
2 files changed, 147 insertions, 0 deletions
diff --git a/src/runtime/testdata/testprogcgo/stackswitch.c b/src/runtime/testdata/testprogcgo/stackswitch.c new file mode 100644 index 0000000000..36258c1e18 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/stackswitch.c @@ -0,0 +1,104 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build unix && !android && !openbsd + +// Required for darwin ucontext. +#define _XOPEN_SOURCE +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + +#include <assert.h> +#include <pthread.h> +#include <stddef.h> +#include <stdio.h> +#include <stdlib.h> +#include <ucontext.h> + +// musl libc does not provide getcontext, etc. Skip the test there. +// +// musl libc doesn't provide any direct detection mechanism. So assume any +// non-glibc linux is using musl. +// +// Note that bionic does not provide getcontext either, but that is skipped via +// the android build tag. +#if defined(__linux__) && !defined(__GLIBC__) +#define MUSL 1 +#endif +#if defined(MUSL) +void callStackSwitchCallbackFromThread(void) { + printf("SKIP\n"); + exit(0); +} +#else + +// Use a stack size larger than the 32kb estimate in +// runtime.callbackUpdateSystemStack. This ensures that a second stack +// allocation won't accidentally count as in bounds of the first stack +#define STACK_SIZE (64ull << 10) + +static ucontext_t uctx_save, uctx_switch; + +extern void stackSwitchCallback(void); + +static void *stackSwitchThread(void *arg) { + // Simple test: callback works from the normal system stack. + stackSwitchCallback(); + + // Next, verify that switching stacks doesn't break callbacks. + + char *stack1 = malloc(STACK_SIZE); + if (stack1 == NULL) { + perror("malloc"); + exit(1); + } + + // Allocate the second stack before freeing the first to ensure we don't get + // the same address from malloc. + char *stack2 = malloc(STACK_SIZE); + if (stack1 == NULL) { + perror("malloc"); + exit(1); + } + + if (getcontext(&uctx_switch) == -1) { + perror("getcontext"); + exit(1); + } + uctx_switch.uc_stack.ss_sp = stack1; + uctx_switch.uc_stack.ss_size = STACK_SIZE; + uctx_switch.uc_link = &uctx_save; + makecontext(&uctx_switch, stackSwitchCallback, 0); + + if (swapcontext(&uctx_save, &uctx_switch) == -1) { + perror("swapcontext"); + exit(1); + } + + if (getcontext(&uctx_switch) == -1) { + perror("getcontext"); + exit(1); + } + uctx_switch.uc_stack.ss_sp = stack2; + uctx_switch.uc_stack.ss_size = STACK_SIZE; + uctx_switch.uc_link = &uctx_save; + makecontext(&uctx_switch, stackSwitchCallback, 0); + + if (swapcontext(&uctx_save, &uctx_switch) == -1) { + perror("swapcontext"); + exit(1); + } + + free(stack1); + free(stack2); + + return NULL; +} + +void callStackSwitchCallbackFromThread(void) { + pthread_t thread; + assert(pthread_create(&thread, NULL, stackSwitchThread, NULL) == 0); + assert(pthread_join(thread, NULL) == 0); +} + +#endif diff --git a/src/runtime/testdata/testprogcgo/stackswitch.go b/src/runtime/testdata/testprogcgo/stackswitch.go new file mode 100644 index 0000000000..a2e422f077 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/stackswitch.go @@ -0,0 +1,43 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build unix && !android && !openbsd + +package main + +/* +void callStackSwitchCallbackFromThread(void); +*/ +import "C" + +import ( + "fmt" + "runtime/debug" +) + +func init() { + register("StackSwitchCallback", StackSwitchCallback) +} + +//export stackSwitchCallback +func stackSwitchCallback() { + // We want to trigger a bounds check on the g0 stack. To do this, we + // need to call a splittable function through systemstack(). + // SetGCPercent contains such a systemstack call. + gogc := debug.SetGCPercent(100) + debug.SetGCPercent(gogc) +} + + +// Regression test for https://go.dev/issue/62440. It should be possible for C +// threads to call into Go from different stacks without crashing due to g0 +// stack bounds checks. +// +// N.B. This is only OK for threads created in C. Threads with Go frames up the +// stack must not change the stack out from under us. +func StackSwitchCallback() { + C.callStackSwitchCallbackFromThread(); + + fmt.Printf("OK\n") +} |
