From ca0dae25ed51627c411dfdbe9dec3901a321bff9 Mon Sep 17 00:00:00 2001
From: KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au>
Date: Fri, 12 Jul 2024 15:13:22 +1000
Subject: [PATCH] Don't crash if madvise(MADV_FREE or MADV_DONTNEED) fails

The M:N threading stack cleanup machinery tries to call MADV_FREE on the native
thread's stack, and calls rb_bug if it fails. Unfortunately, there's no way to
distinguish between "You passed bad parameters to madvise" and "MADV_FREE is
not supported on the kernel you are running on"; both cases just return EINVAL.
This means that if you have a Ruby on a system that was built on a system with
MADV_FREE and run it on a system without it, you get a crash in nt_free_stack.

I ran into this because rr actually emulates MADV_FREE by just returning EINVAL
and pretending it's not supported (since it can otherwise introduce
nondeterministic behaviour). So if you run bootstraptest/test_ractor.rb under
rr, you get this crash.

I think we should just get rid of the error handling here; freeing memory like
this is strictly optional anyway.

[Bug #20632]
---
 thread_pthread_mn.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c
index 6b0631ad5a..08d587e2b2 100644
--- a/thread_pthread_mn.c
+++ b/thread_pthread_mn.c
@@ -337,6 +337,30 @@ nt_alloc_stack(rb_vm_t *vm, void **vm_stack, void **machine_stack)
     return err;
 }
 
+static void
+nt_madvise_free_or_dontneed(void *addr, size_t len)
+{
+    /* There is no real way to perform error handling here. Both MADV_FREE
+     * and MADV_DONTNEED are both documented to pretty much only return EINVAL
+     * for a huge variety of errors. It's indistinguishable if madvise fails
+     * because the parameters were bad, or because the kernel we're running on
+     * does not support the given advice. This kind of free-but-don't-unmap
+     * is best-effort anyway, so don't sweat it.
+     *
+     * n.b. A very common case of "the kernel doesn't support MADV_FREE and
+     * returns EINVAL" is running under the `rr` debugger; it makes all
+     * MADV_FREE calls return EINVAL. */
+
+#if defined(MADV_FREE)
+    int r = madvise(addr, len, MADV_FREE);
+    // Return on success, or else try MADV_DONTNEED
+    if (r == 0) return;
+#endif
+#if defined(MADV_DONTNEED)
+    madvise(addr, len, MADV_DONTNEED);
+#endif
+}
+
 static void
 nt_free_stack(void *mstack)
 {
@@ -358,19 +382,12 @@ nt_free_stack(void *mstack)
         ch->free_stack[ch->free_stack_pos++] = idx;
 
         // clear the stack pages
-#if defined(MADV_FREE)
-        int r = madvise(stack, nt_thread_stack_size(), MADV_FREE);
-#elif defined(MADV_DONTNEED)
-        int r = madvise(stack, nt_thread_stack_size(), MADV_DONTNEED);
-#else
-        int r = 0;
-#endif
-
-        if (r != 0) rb_bug("madvise errno:%d", errno);
+        nt_madvise_free_or_dontneed(stack, nt_thread_stack_size());
     }
     rb_native_mutex_unlock(&nt_machine_stack_lock);
 }
 
+
 static int
 native_thread_check_and_create_shared(rb_vm_t *vm)
 {