Skip to content

Commit 80c1139

Browse files
committed
Restore fixupBoehmStackPointer
This was removed in NixOS#11152. However, we need it for the multi-threaded evaluator, because otherwise Boehm GC will crash while scanning the thread stack: #0 GC_push_all_eager (bottom=<optimized out>, top=<optimized out>) at extra/../mark.c:1488 #1 0x00007ffff74691d5 in GC_push_all_stack_sections (lo=<optimized out>, hi=<optimized out>, traced_stack_sect=0x0) at extra/../mark_rts.c:704 #2 GC_push_all_stacks () at extra/../pthread_stop_world.c:876 #3 GC_default_push_other_roots () at extra/../os_dep.c:2893 #4 0x00007ffff746235c in GC_mark_some (cold_gc_frame=0x7ffee8ecaa50 "`\304G\367\377\177") at extra/../mark.c:374 #5 0x00007ffff7465a8d in GC_stopped_mark (stop_func=stop_func@entry=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:875 #6 0x00007ffff7466724 in GC_try_to_collect_inner (stop_func=0x7ffff7453c80 <GC_never_stop_func>) at extra/../alloc.c:624 #7 0x00007ffff7466a22 in GC_collect_or_expand (needed_blocks=needed_blocks@entry=1, ignore_off_page=ignore_off_page@entry=0, retry=retry@entry=0) at extra/../alloc.c:1688 #8 0x00007ffff746878f in GC_allocobj (gran=<optimized out>, kind=<optimized out>) at extra/../alloc.c:1798 #9 GC_generic_malloc_inner (lb=<optimized out>, k=k@entry=1) at extra/../malloc.c:193 #10 0x00007ffff746cd40 in GC_generic_malloc_many (lb=<optimized out>, k=<optimized out>, result=<optimized out>) at extra/../mallocx.c:477 #11 0x00007ffff746cf35 in GC_malloc_kind (bytes=120, kind=1) at extra/../thread_local_alloc.c:187 #12 0x00007ffff796ede5 in nix::allocBytes (n=<optimized out>, n=<optimized out>) at ../src/libexpr/include/nix/expr/eval-inline.hh:19 This is because it will use the stack pointer of the coroutine, so it will scan a region of memory that doesn't exist, e.g. Stack for thread 0x7ffea4ff96c0 is [0x7ffe80197af0w,0x7ffea4ffa000) (where 0x7ffe80197af0w is the sp of the coroutine and 0x7ffea4ffa000 is the base of the thread stack). We don't scan coroutine stacks, because currently they don't have GC roots (there is no evaluation happening in coroutines). So there is currently no need to restore the other parts of the original patch, such as BoehmGCStackAllocator.
1 parent f78c858 commit 80c1139

File tree

1 file changed

+64
-0
lines changed

1 file changed

+64
-0
lines changed

src/libexpr/eval-gc.cc

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,67 @@ static size_t getFreeMem()
6767
return 0;
6868
}
6969

70+
/**
71+
* When a thread goes into a coroutine, we lose its original sp until
72+
* control flow returns to the thread. This causes Boehm GC to crash
73+
* since it will scan memory between the coroutine's sp and the
74+
* original stack base of the thread. Therefore, we detect when the
75+
* current sp is outside of the original thread stack and push the
76+
* entire thread stack instead, as an approximation.
77+
*
78+
* This is not optimal, because it causes the stack below sp to be
79+
* scanned. However, we usually we don't have active coroutines during
80+
* evaluation, so this is acceptable.
81+
*
82+
* Note that we don't scan coroutine stacks. It's currently assumed
83+
* that we don't have GC roots in coroutines.
84+
*/
85+
void fixupBoehmStackPointer(void ** sp_ptr, void * _pthread_id)
86+
{
87+
void *& sp = *sp_ptr;
88+
auto pthread_id = reinterpret_cast<pthread_t>(_pthread_id);
89+
size_t osStackSize;
90+
// The low address of the stack, which grows down.
91+
void * osStackLimit;
92+
93+
# ifdef __APPLE__
94+
osStackSize = pthread_get_stacksize_np(pthread_id);
95+
osStackLimit = pthread_get_stackaddr_np(pthread_id);
96+
# else
97+
pthread_attr_t pattr;
98+
if (pthread_attr_init(&pattr)) {
99+
throw Error("fixupBoehmStackPointer: pthread_attr_init failed");
100+
}
101+
# ifdef HAVE_PTHREAD_GETATTR_NP
102+
if (pthread_getattr_np(pthread_id, &pattr)) {
103+
throw Error("fixupBoehmStackPointer: pthread_getattr_np failed");
104+
}
105+
# elif HAVE_PTHREAD_ATTR_GET_NP
106+
if (!pthread_attr_init(&pattr)) {
107+
throw Error("fixupBoehmStackPointer: pthread_attr_init failed");
108+
}
109+
if (!pthread_attr_get_np(pthread_id, &pattr)) {
110+
throw Error("fixupBoehmStackPointer: pthread_attr_get_np failed");
111+
}
112+
# else
113+
# error "Need one of `pthread_attr_get_np` or `pthread_getattr_np`"
114+
# endif
115+
if (pthread_attr_getstack(&pattr, &osStackLimit, &osStackSize)) {
116+
throw Error("fixupBoehmStackPointer: pthread_attr_getstack failed");
117+
}
118+
if (pthread_attr_destroy(&pattr)) {
119+
throw Error("fixupBoehmStackPointer: pthread_attr_destroy failed");
120+
}
121+
# endif
122+
123+
void * osStackBase = (char *) osStackLimit + osStackSize;
124+
// NOTE: We assume the stack grows down, as it does on all architectures we support.
125+
// Architectures that grow the stack up are rare.
126+
if (sp >= osStackBase || sp < osStackLimit) { // sp is outside the os stack
127+
sp = osStackLimit;
128+
}
129+
}
130+
70131
static inline void initGCReal()
71132
{
72133
/* Initialise the Boehm garbage collector. */
@@ -97,6 +158,9 @@ static inline void initGCReal()
97158

98159
GC_set_oom_fn(oomHandler);
99160

161+
GC_set_sp_corrector(&fixupBoehmStackPointer);
162+
assert(GC_get_sp_corrector());
163+
100164
/* Set the initial heap size to something fairly big (80% of
101165
free RAM, up to a maximum of 4 GiB) so that in most cases
102166
we don't need to garbage collect at all. (Collection has a

0 commit comments

Comments
 (0)