From ea520ca9273699fc1c77a71bbeba4b6e06ccfc6c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 22 Apr 2019 17:14:36 -0700 Subject: [PATCH] Prevent rb_define_(class|module) classes from moving Before this commit, classes and modules would be registered with the VM's `defined_module_hash`. The key was the ID of the class, but that meant that it was possible for hash collisions to occur. The compactor doesn't allow classes in the `defined_module_hash` to move, but if there is a conflict, then it's possible a class would be removed from the hash and not get pined. This commit changes the key / value of the hash just to be the class itself, thus preventing movement. --- class.c | 3 +++ gc.c | 2 +- vm.c | 10 ++++++---- vm_core.h | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/class.c b/class.c index d0ff92b6eb..0cee5caae2 100644 --- a/class.c +++ b/class.c @@ -659,6 +659,9 @@ rb_define_class(const char *name, VALUE super) if (rb_class_real(RCLASS_SUPER(klass)) != super) { rb_raise(rb_eTypeError, "superclass mismatch for class %s", name); } + + /* Class may have been defined in Ruby and not pin-rooted */ + rb_vm_add_root_module(id, klass); return klass; } if (!super) { diff --git a/gc.c b/gc.c index f90bdd99bf..9529b2d4a4 100644 --- a/gc.c +++ b/gc.c @@ -8159,7 +8159,7 @@ gc_verify_compaction_references(VALUE mod) heap_eden->using_page = NULL; gc_verify_internal_consistency(mod); - rb_gc_enable(); + /* GC after compaction to eliminate T_MOVED */ rb_gc(); diff --git a/vm.c b/vm.c index c4144b5b99..41064f07c3 100644 --- a/vm.c +++ b/vm.c @@ -2251,9 +2251,8 @@ rb_vm_mark(void *ptr) rb_gc_mark(vm->loaded_features_snapshot); rb_gc_mark(vm->top_self); RUBY_MARK_UNLESS_NULL(vm->coverages); - rb_gc_mark(vm->defined_module_hash); /* Prevent classes from moving */ - rb_mark_tbl(rb_hash_tbl(vm->defined_module_hash, __FILE__, __LINE__)); + rb_mark_tbl(vm->defined_module_hash); if (vm->loading_table) { rb_mark_tbl(vm->loading_table); @@ -2286,7 +2285,7 @@ rb_vm_add_root_module(ID id, VALUE module) { rb_vm_t *vm = GET_VM(); - rb_hash_aset(vm->defined_module_hash, ID2SYM(id), module); + st_insert(vm->defined_module_hash, (st_data_t)module, (st_data_t)module); return TRUE; } @@ -2561,6 +2560,9 @@ thread_mark(void *ptr) RUBY_MARK_UNLESS_NULL(th->top_self); RUBY_MARK_UNLESS_NULL(th->top_wrapper); if (th->root_fiber) rb_fiber_mark_self(th->root_fiber); + + /* Ensure EC stack objects are pinned */ + rb_execution_context_mark(th->ec); RUBY_MARK_UNLESS_NULL(th->stat_insn_usage); RUBY_MARK_UNLESS_NULL(th->last_status); RUBY_MARK_UNLESS_NULL(th->locking_mutex); @@ -3230,7 +3232,7 @@ Init_vm_objects(void) { rb_vm_t *vm = GET_VM(); - vm->defined_module_hash = rb_hash_new(); + vm->defined_module_hash = st_init_numtable(); /* initialize mark object array, hash */ vm->mark_object_ary = rb_ary_tmp_new(128); diff --git a/vm_core.h b/vm_core.h index 2e8ddfe37f..2c38911d58 100644 --- a/vm_core.h +++ b/vm_core.h @@ -660,7 +660,7 @@ typedef struct rb_vm_struct { VALUE coverages; int coverage_mode; - VALUE defined_module_hash; + st_table * defined_module_hash; struct rb_objspace *objspace;