Skip to content

Commit 7afc16a

Browse files
KJ TsanaktsidisKJTsanaktsidis
authored andcommitted
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 parent ad63603 commit 7afc16a

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

thread.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,10 @@ static inline void blocking_region_end(rb_thread_t *th, struct rb_blocking_regio
197197
if (blocking_region_begin(th, &__region, (ubf), (ubfarg), fail_if_interrupted) || \
198198
/* always return true unless fail_if_interrupted */ \
199199
!only_if_constant(fail_if_interrupted, TRUE)) { \
200+
/* Important that this is inlined into the macro, and not part of \
201+
* blocking_region_begin - see bug #20493 */ \
202+
RB_VM_SAVE_MACHINE_CONTEXT(th); \
203+
thread_sched_to_waiting(TH_SCHED(th), th); \
200204
exec; \
201205
blocking_region_end(th, &__region); \
202206
}; \
@@ -1482,9 +1486,6 @@ blocking_region_begin(rb_thread_t *th, struct rb_blocking_region_buffer *region,
14821486
rb_ractor_blocking_threads_inc(th->ractor, __FILE__, __LINE__);
14831487

14841488
RUBY_DEBUG_LOG("thread_id:%p", (void *)th->nt->thread_id);
1485-
1486-
RB_VM_SAVE_MACHINE_CONTEXT(th);
1487-
thread_sched_to_waiting(TH_SCHED(th), th);
14881489
return TRUE;
14891490
}
14901491
else {
@@ -1893,6 +1894,8 @@ rb_thread_call_with_gvl(void *(*func)(void *), void *data1)
18931894
/* leave from Ruby world: You can not access Ruby values, etc. */
18941895
int released = blocking_region_begin(th, brb, prev_unblock.func, prev_unblock.arg, FALSE);
18951896
RUBY_ASSERT_ALWAYS(released);
1897+
RB_VM_SAVE_MACHINE_CONTEXT(th);
1898+
thread_sched_to_waiting(TH_SCHED(th), th);
18961899
return r;
18971900
}
18981901

0 commit comments

Comments
 (0)