Inline RB_VM_SAVE_MACHINE_CONTEXT into BLOCKING_REGION

There's an exhaustive explanation of this in the linked redmine bug, but
the short version is as follows:

blocking_region_begin can spill callee-saved registers to the stack for
its own use. That means they're not saved to ec->machine by the call to
setjmp, since by that point they're already on the stack and new,
different values are in the real registers. ec->machine's end-of-stack
pointer is also bumped to accomodate this, BUT, after
blocking_region_begin returns, that points past the end of the stack!

As far as C is concerned, that's fine; the callee-saved registers are
restored when blocking_region_begin returns. But, if another thread
triggers GC, it is relying on finding references to Ruby objects by
walking the stack region pointed to by ec->machine.

If the C code in exec; subsequently does things that use that stack
memory, then the value will be overwritten and the GC might prematurely
collect something it shouldn't.

[Bug #20493]

1 files changed, 6 insertions, 3 deletions

diff --git a/thread.c b/thread.c
index a1c2780c6f..56cba3d6ee 100644
--- a/thread.c
+++ b/thread.c
@@ -196,6 +196,10 @@ static inline void blocking_region_end(rb_thread_t *th, struct rb_blocking_regio
     if (blocking_region_begin(th, &__region, (ubf), (ubfarg), fail_if_interrupted) || \
         /* always return true unless fail_if_interrupted */ \
         !only_if_constant(fail_if_interrupted, TRUE)) { \
+        /* Important that this is inlined into the macro, and not part of \
+         * blocking_region_begin - see bug #20493 */ \
+        RB_GC_SAVE_MACHINE_CONTEXT(th); \
+        thread_sched_to_waiting(TH_SCHED(th)); \
         exec; \
         blocking_region_end(th, &__region); \
     }; \
@@ -1493,9 +1497,6 @@ blocking_region_begin(rb_thread_t *th, struct rb_blocking_region_buffer *region,
         rb_ractor_blocking_threads_inc(th->ractor, __FILE__, __LINE__);
 
         RUBY_DEBUG_LOG("");
-
-        RB_GC_SAVE_MACHINE_CONTEXT(th);
-        thread_sched_to_waiting(TH_SCHED(th));
         return TRUE;
     }
     else {
@@ -1788,6 +1789,8 @@ rb_thread_call_with_gvl(void *(*func)(void *), void *data1)
     /* leave from Ruby world: You can not access Ruby values, etc. */
     int released = blocking_region_begin(th, brb, prev_unblock.func, prev_unblock.arg, FALSE);
     RUBY_ASSERT_ALWAYS(released);
+    RB_GC_SAVE_MACHINE_CONTEXT(th);
+    thread_sched_to_waiting(TH_SCHED(th));
     return r;
 }