From 46b69073465f58847676d46ffcbdc93a0a46ce08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Mon, 11 Aug 2025 19:18:04 +0200 Subject: [PATCH] Revert "Merge pull request #13709 from NixOS/boehm-coroutines-sp" This reverts commit 4b3ca9bd802f3cbdffb743cc399bad0af2ebfcf5, reversing changes made to 867b69f53324226c35455471f3b10f2ffd12e67c. Since this commit we get reproducible segfaults building Nix ci in macos github runners: https://github.com/NixOS/nix/actions/runs/16885882321/job/47837390248 --- src/libexpr/eval-gc.cc | 64 ------------------------------------------ 1 file changed, 64 deletions(-) diff --git a/src/libexpr/eval-gc.cc b/src/libexpr/eval-gc.cc index 9c050dc92..b17336a90 100644 --- a/src/libexpr/eval-gc.cc +++ b/src/libexpr/eval-gc.cc @@ -33,67 +33,6 @@ static void * oomHandler(size_t requested) throw std::bad_alloc(); } -/** - * When a thread goes into a coroutine, we lose its original sp until - * control flow returns to the thread. This causes Boehm GC to crash - * since it will scan memory between the coroutine's sp and the - * original stack base of the thread. Therefore, we detect when the - * current sp is outside of the original thread stack and push the - * entire thread stack instead, as an approximation. - * - * This is not optimal, because it causes the stack below sp to be - * scanned. However, we usually we don't have active coroutines during - * evaluation, so this is acceptable. - * - * Note that we don't scan coroutine stacks. It's currently assumed - * that we don't have GC roots in coroutines. - */ -void fixupBoehmStackPointer(void ** sp_ptr, void * _pthread_id) -{ - void *& sp = *sp_ptr; - auto pthread_id = reinterpret_cast(_pthread_id); - size_t osStackSize; - // The low address of the stack, which grows down. - void * osStackLimit; - -# ifdef __APPLE__ - osStackSize = pthread_get_stacksize_np(pthread_id); - osStackLimit = pthread_get_stackaddr_np(pthread_id); -# else - pthread_attr_t pattr; - if (pthread_attr_init(&pattr)) { - throw Error("fixupBoehmStackPointer: pthread_attr_init failed"); - } -# ifdef HAVE_PTHREAD_GETATTR_NP - if (pthread_getattr_np(pthread_id, &pattr)) { - throw Error("fixupBoehmStackPointer: pthread_getattr_np failed"); - } -# elif HAVE_PTHREAD_ATTR_GET_NP - if (!pthread_attr_init(&pattr)) { - throw Error("fixupBoehmStackPointer: pthread_attr_init failed"); - } - if (!pthread_attr_get_np(pthread_id, &pattr)) { - throw Error("fixupBoehmStackPointer: pthread_attr_get_np failed"); - } -# else -# error "Need one of `pthread_attr_get_np` or `pthread_getattr_np`" -# endif - if (pthread_attr_getstack(&pattr, &osStackLimit, &osStackSize)) { - throw Error("fixupBoehmStackPointer: pthread_attr_getstack failed"); - } - if (pthread_attr_destroy(&pattr)) { - throw Error("fixupBoehmStackPointer: pthread_attr_destroy failed"); - } -# endif - - void * osStackBase = (char *) osStackLimit + osStackSize; - // NOTE: We assume the stack grows down, as it does on all architectures we support. - // Architectures that grow the stack up are rare. - if (sp >= osStackBase || sp < osStackLimit) { // sp is outside the os stack - sp = osStackLimit; - } -} - static inline void initGCReal() { /* Initialise the Boehm garbage collector. */ @@ -124,9 +63,6 @@ static inline void initGCReal() GC_set_oom_fn(oomHandler); - GC_set_sp_corrector(&fixupBoehmStackPointer); - assert(GC_get_sp_corrector()); - /* Set the initial heap size to something fairly big (25% of physical RAM, up to a maximum of 384 MiB) so that in most cases we don't need to garbage collect at all. (Collection has a