Skip to content

Commit

Permalink
boehmgc: Crude support for coroutines
Browse files Browse the repository at this point in the history
Fixes the problem where a stack pointer outside the original
thread causes the collector to crash.

It could be made more accurate by recording the stack pointer
every time we switch to a coroutine. We can use this information
to update our own coroutine stacks like normal data. When the
stack pointer is on a thread, we can add a field to GC_thread
"fallback_sp" to be used when the thread sp is outside the original
thread range.
  • Loading branch information
roberth committed Jun 24, 2021
1 parent bec83a6 commit 5740924
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
44 changes: 44 additions & 0 deletions boehmgc-coroutine-sp-fallback.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 1cee6a0b..8977b0dd 100644
--- a/pthread_stop_world.c
+++ b/pthread_stop_world.c
@@ -674,6 +674,8 @@ GC_INNER void GC_push_all_stacks(void)
struct GC_traced_stack_sect_s *traced_stack_sect;
pthread_t self = pthread_self();
word total_size = 0;
+ size_t stack_limit;
+ pthread_attr_t pattr;

if (!EXPECT(GC_thr_initialized, TRUE))
GC_thr_init();
@@ -723,6 +725,30 @@ GC_INNER void GC_push_all_stacks(void)
hi = p->altstack + p->altstack_size;
/* FIXME: Need to scan the normal stack too, but how ? */
/* FIXME: Assume stack grows down */
+ } else {
+ if (pthread_getattr_np(p->id, &pattr)) {
+ ABORT("GC_push_all_stacks: pthread_getattr_np failed!");
+ }
+ if (pthread_attr_getstacksize(&pattr, &stack_limit)) {
+ ABORT("GC_push_all_stacks: pthread_attr_getstacksize failed!");
+ }
+ // When a thread goes into a coroutine, we lose its original sp until
+ // control flow returns to the thread.
+ // While in the coroutine, the sp points outside the thread stack,
+ // so we can detect this and push the entire thread stack instead,
+ // as an approximation.
+ // We assume that the coroutine has similarly added its entire stack.
+ // This could be made accurate by cooperating with the application
+ // via new functions and/or callbacks.
+ #ifndef STACK_GROWS_UP
+ if (lo >= hi || lo < hi - stack_limit) { // sp outside stack
+ lo = hi - stack_limit;
+ }
+ #else
+ if (lo < hi || lo >= hi + stack_limit) { // sp outside stack
+ lo = hi + stack_limit;

This comment has been minimized.

Copy link
@ivmai

ivmai May 19, 2023

So, if sp is outside stack, we scan the entire stack (e.g. 8MB by default on Linux), right? But it might contain unmapped pages and thus cause SIGSEV in GC_mark_from. Is there some trick on nix side to avoid this?

This comment has been minimized.

Copy link
@ivmai

ivmai May 19, 2023

Aha, got it, there's a guard page.

This comment has been minimized.

Copy link
@roberth

roberth May 19, 2023

Author Member

We assume that the entire stack (pthread_attr_getstacksize) is mapped, and indeed we detect overflows with a guard page. That works quite well.
We do have a regression with the stack overflow detection, but that's not during GC iirc.

This comment has been minimized.

Copy link
@ivmai

ivmai May 19, 2023

Have you tested with approach on platforms other than Linux? I briefly checked it on FreeBSD - the fault stack size is 512 MB and got SIGSEGV from GC_mark_from.

+ }
+ #endif
}
GC_push_all_stack_sections(lo, hi, traced_stack_sect);
# ifdef STACK_GROWS_UP
8 changes: 7 additions & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,13 @@
});

propagatedDeps =
[ (boehmgc.override { enableLargeConfig = true; })
[ ((boehmgc.override {
enableLargeConfig = true;
}).overrideAttrs(o: {
patches = (o.patches or []) ++ [
./boehmgc-coroutine-sp-fallback.diff
];
}))
];

perlDeps =
Expand Down

0 comments on commit 5740924

Please sign in to comment.