Skip to content

Commit

Permalink
Fix write barrier order for klass to cme edge
Browse files Browse the repository at this point in the history
Previously, the following crashes with
`CFLAGS=-DRGENGC_CHECK_MODE=2 -DRUBY_DEBUG=1 -fno-inline`:

    $ ./miniruby -e 'GC.stress = true; Marshal.dump({})'

It crashes with a write barrier (WB) miss assertion on an edge from the
`Hash` class object to a newly allocated negative method entry.

This is due to usages of vm_ccs_create() running the WB too early,
before the method entry is inserted into the cc table, so before the
reference edge is established. The insertion can trigger GC and promote
the class object, so running the WB after the insertion is necessary.
Move the insertion into vm_ccs_create() and run the WB after the
insertion.

Discovered on CI:
http://ci.rvm.jp/results/trunk-asserts@ruby-sp2-docker/4391770
  • Loading branch information
XrXr committed Jan 12, 2023
1 parent da7e5c7 commit 537183c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
3 changes: 1 addition & 2 deletions vm_eval.c
Expand Up @@ -396,8 +396,7 @@ cc_new(VALUE klass, ID mid, int argc, const rb_callable_method_entry_t *cme)
ccs = (struct rb_class_cc_entries *)ccs_data;
}
else {
ccs = vm_ccs_create(klass, cme);
rb_id_table_insert(cc_tbl, mid, (VALUE)ccs);
ccs = vm_ccs_create(klass, cc_tbl, mid, cme);
}

for (int i=0; i<ccs->len; i++) {
Expand Down
10 changes: 6 additions & 4 deletions vm_insnhelper.c
Expand Up @@ -1871,17 +1871,20 @@ static VALUE vm_call_general(rb_execution_context_t *ec, rb_control_frame_t *reg
static VALUE vm_mtbl_dump(VALUE klass, ID target_mid);

static struct rb_class_cc_entries *
vm_ccs_create(VALUE klass, const rb_callable_method_entry_t *cme)
vm_ccs_create(VALUE klass, struct rb_id_table *cc_tbl, ID mid, const rb_callable_method_entry_t *cme)
{
struct rb_class_cc_entries *ccs = ALLOC(struct rb_class_cc_entries);
#if VM_CHECK_MODE > 0
ccs->debug_sig = ~(VALUE)ccs;
#endif
ccs->capa = 0;
ccs->len = 0;
RB_OBJ_WRITE(klass, &ccs->cme, cme);
ccs->cme = cme;
METHOD_ENTRY_CACHED_SET((rb_callable_method_entry_t *)cme);
ccs->entries = NULL;

rb_id_table_insert(cc_tbl, mid, (VALUE)ccs);
RB_OBJ_WRITTEN(klass, Qundef, cme);
return ccs;
}

Expand Down Expand Up @@ -2032,8 +2035,7 @@ vm_search_cc(const VALUE klass, const struct rb_callinfo * const ci)
}
else {
// TODO: required?
ccs = vm_ccs_create(klass, cme);
rb_id_table_insert(cc_tbl, mid, (VALUE)ccs);
ccs = vm_ccs_create(klass, cc_tbl, mid, cme);
}
}

Expand Down
3 changes: 1 addition & 2 deletions vm_method.c
Expand Up @@ -1330,8 +1330,7 @@ cache_callable_method_entry(VALUE klass, ID mid, const rb_callable_method_entry_
VM_ASSERT(ccs->cme == cme);
}
else {
ccs = vm_ccs_create(klass, cme);
rb_id_table_insert(cc_tbl, mid, (VALUE)ccs);
ccs = vm_ccs_create(klass, cc_tbl, mid, cme);
}
}

Expand Down

0 comments on commit 537183c

Please sign in to comment.