Skip to content

Commit 08b3a45

Browse files
XrXrtenderlove
XrXr
and
tenderlove
committed
Push a real iseq in rb_vm_push_frame_fname()
Previously, vm_make_env_each() (used during proc creation and for the debug inspector C API) picked up the non-GC-allocated iseq that rb_vm_push_frame_fname() creates, which led to a SEGV when the GC tried to mark the non GC object. Put a real iseq imemo instead. Speed should be about the same since the old code also did a imemo allocation and a malloc allocation. Real iseq allows ironing out the special-casing of dummy frames in rb_execution_context_mark() and rb_execution_context_update(). A check is added to RubyVM::ISeq#eval, though, to stop attempts to run dummy iseqs. [Bug #21180] Co-authored-by: Aaron Patterson <[email protected]>
1 parent 9b96618 commit 08b3a45

File tree

4 files changed

+58
-39
lines changed

4 files changed

+58
-39
lines changed

‎iseq.c‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,19 @@ rb_iseq_pathobj_set(const rb_iseq_t *iseq, VALUE path, VALUE realpath)
531531
rb_iseq_pathobj_new(path, realpath));
532532
}
533533

534+
// Make a dummy iseq for a dummy frame that exposes a path for profilers to inspect
535+
rb_iseq_t *
536+
rb_iseq_alloc_with_dummy_path(VALUE fname)
537+
{
538+
rb_iseq_t *dummy_iseq = iseq_alloc();
539+
540+
ISEQ_BODY(dummy_iseq)->type = ISEQ_TYPE_TOP;
541+
RB_OBJ_WRITE(dummy_iseq, &ISEQ_BODY(dummy_iseq)->location.pathobj, fname);
542+
RB_OBJ_WRITE(dummy_iseq, &ISEQ_BODY(dummy_iseq)->location.label, fname);
543+
544+
return dummy_iseq;
545+
}
546+
534547
static rb_iseq_location_t *
535548
iseq_location_setup(rb_iseq_t *iseq, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_code_location_t *code_location, const int node_id)
536549
{
@@ -1909,7 +1922,11 @@ rb_iseqw_to_iseq(VALUE iseqw)
19091922
static VALUE
19101923
iseqw_eval(VALUE self)
19111924
{
1912-
return rb_iseq_eval(iseqw_check(self));
1925+
const rb_iseq_t *iseq = iseqw_check(self);
1926+
if (0 == ISEQ_BODY(iseq)->iseq_size) {
1927+
rb_raise(rb_eTypeError, "attempt to evaluate dummy InstructionSequence");
1928+
}
1929+
return rb_iseq_eval(iseq);
19131930
}
19141931

19151932
/*

‎test/fiber/test_scheduler.rb‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,19 @@ def test_autoload
139139
end
140140
end
141141

142+
def test_iseq_compile_under_gc_stress_bug_21180
143+
Thread.new do
144+
scheduler = Scheduler.new
145+
Fiber.set_scheduler scheduler
146+
147+
Fiber.schedule do
148+
EnvUtil.under_gc_stress do
149+
RubyVM::InstructionSequence.compile_file(File::NULL)
150+
end
151+
end
152+
end.join
153+
end
154+
142155
def test_deadlock
143156
mutex = Thread::Mutex.new
144157
condition = Thread::ConditionVariable.new

‎vm.c‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3362,22 +3362,20 @@ rb_execution_context_update(rb_execution_context_t *ec)
33623362
}
33633363

33643364
while (cfp != limit_cfp) {
3365-
if (VM_FRAME_TYPE(cfp) != VM_FRAME_MAGIC_DUMMY) {
3366-
const VALUE *ep = cfp->ep;
3367-
cfp->self = rb_gc_location(cfp->self);
3368-
cfp->iseq = (rb_iseq_t *)rb_gc_location((VALUE)cfp->iseq);
3369-
cfp->block_code = (void *)rb_gc_location((VALUE)cfp->block_code);
3370-
3371-
if (!VM_ENV_LOCAL_P(ep)) {
3372-
const VALUE *prev_ep = VM_ENV_PREV_EP(ep);
3373-
if (VM_ENV_FLAGS(prev_ep, VM_ENV_FLAG_ESCAPED)) {
3374-
VM_FORCE_WRITE(&prev_ep[VM_ENV_DATA_INDEX_ENV], rb_gc_location(prev_ep[VM_ENV_DATA_INDEX_ENV]));
3375-
}
3365+
const VALUE *ep = cfp->ep;
3366+
cfp->self = rb_gc_location(cfp->self);
3367+
cfp->iseq = (rb_iseq_t *)rb_gc_location((VALUE)cfp->iseq);
3368+
cfp->block_code = (void *)rb_gc_location((VALUE)cfp->block_code);
3369+
3370+
if (!VM_ENV_LOCAL_P(ep)) {
3371+
const VALUE *prev_ep = VM_ENV_PREV_EP(ep);
3372+
if (VM_ENV_FLAGS(prev_ep, VM_ENV_FLAG_ESCAPED)) {
3373+
VM_FORCE_WRITE(&prev_ep[VM_ENV_DATA_INDEX_ENV], rb_gc_location(prev_ep[VM_ENV_DATA_INDEX_ENV]));
3374+
}
33763375

3377-
if (VM_ENV_FLAGS(ep, VM_ENV_FLAG_ESCAPED)) {
3378-
VM_FORCE_WRITE(&ep[VM_ENV_DATA_INDEX_ENV], rb_gc_location(ep[VM_ENV_DATA_INDEX_ENV]));
3379-
VM_FORCE_WRITE(&ep[VM_ENV_DATA_INDEX_ME_CREF], rb_gc_location(ep[VM_ENV_DATA_INDEX_ME_CREF]));
3380-
}
3376+
if (VM_ENV_FLAGS(ep, VM_ENV_FLAG_ESCAPED)) {
3377+
VM_FORCE_WRITE(&ep[VM_ENV_DATA_INDEX_ENV], rb_gc_location(ep[VM_ENV_DATA_INDEX_ENV]));
3378+
VM_FORCE_WRITE(&ep[VM_ENV_DATA_INDEX_ME_CREF], rb_gc_location(ep[VM_ENV_DATA_INDEX_ME_CREF]));
33813379
}
33823380
}
33833381

@@ -3413,21 +3411,19 @@ rb_execution_context_mark(const rb_execution_context_t *ec)
34133411
const VALUE *ep = cfp->ep;
34143412
VM_ASSERT(!!VM_ENV_FLAGS(ep, VM_ENV_FLAG_ESCAPED) == vm_ep_in_heap_p_(ec, ep));
34153413

3416-
if (VM_FRAME_TYPE(cfp) != VM_FRAME_MAGIC_DUMMY) {
3417-
rb_gc_mark_movable(cfp->self);
3418-
rb_gc_mark_movable((VALUE)cfp->iseq);
3419-
rb_gc_mark_movable((VALUE)cfp->block_code);
3414+
rb_gc_mark_movable(cfp->self);
3415+
rb_gc_mark_movable((VALUE)cfp->iseq);
3416+
rb_gc_mark_movable((VALUE)cfp->block_code);
34203417

3421-
if (!VM_ENV_LOCAL_P(ep)) {
3422-
const VALUE *prev_ep = VM_ENV_PREV_EP(ep);
3423-
if (VM_ENV_FLAGS(prev_ep, VM_ENV_FLAG_ESCAPED)) {
3424-
rb_gc_mark_movable(prev_ep[VM_ENV_DATA_INDEX_ENV]);
3425-
}
3418+
if (!VM_ENV_LOCAL_P(ep)) {
3419+
const VALUE *prev_ep = VM_ENV_PREV_EP(ep);
3420+
if (VM_ENV_FLAGS(prev_ep, VM_ENV_FLAG_ESCAPED)) {
3421+
rb_gc_mark_movable(prev_ep[VM_ENV_DATA_INDEX_ENV]);
3422+
}
34263423

3427-
if (VM_ENV_FLAGS(ep, VM_ENV_FLAG_ESCAPED)) {
3428-
rb_gc_mark_movable(ep[VM_ENV_DATA_INDEX_ENV]);
3429-
rb_gc_mark(ep[VM_ENV_DATA_INDEX_ME_CREF]);
3430-
}
3424+
if (VM_ENV_FLAGS(ep, VM_ENV_FLAG_ESCAPED)) {
3425+
rb_gc_mark_movable(ep[VM_ENV_DATA_INDEX_ENV]);
3426+
rb_gc_mark(ep[VM_ENV_DATA_INDEX_ME_CREF]);
34313427
}
34323428
}
34333429

‎vm_insnhelper.c‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -469,15 +469,8 @@ rb_vm_pop_frame(rb_execution_context_t *ec)
469469
VALUE
470470
rb_vm_push_frame_fname(rb_execution_context_t *ec, VALUE fname)
471471
{
472-
VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer();
473-
void *ptr = ruby_xcalloc(sizeof(struct rb_iseq_constant_body) + sizeof(struct rb_iseq_struct), 1);
474-
rb_imemo_tmpbuf_set_ptr(tmpbuf, ptr);
475-
476-
struct rb_iseq_struct *dmy_iseq = (struct rb_iseq_struct *)ptr;
477-
struct rb_iseq_constant_body *dmy_body = (struct rb_iseq_constant_body *)&dmy_iseq[1];
478-
dmy_iseq->body = dmy_body;
479-
dmy_body->type = ISEQ_TYPE_TOP;
480-
dmy_body->location.pathobj = fname;
472+
rb_iseq_t *rb_iseq_alloc_with_dummy_path(VALUE fname);
473+
rb_iseq_t *dmy_iseq = rb_iseq_alloc_with_dummy_path(fname);
481474

482475
vm_push_frame(ec,
483476
dmy_iseq, //const rb_iseq_t *iseq,
@@ -490,7 +483,7 @@ rb_vm_push_frame_fname(rb_execution_context_t *ec, VALUE fname)
490483
0, // int local_size,
491484
0); // int stack_max
492485

493-
return tmpbuf;
486+
return (VALUE)dmy_iseq;
494487
}
495488

496489
/* method dispatch */

0 commit comments

Comments
 (0)