From: nagachika00@... Date: 2018-02-05T14:05:59+00:00 Subject: [ruby-core:85398] [Ruby trunk Bug#14070] Refining a module dumps core Issue #14070 has been updated by nagachika (Tomoyuki Chikanaga). Backport changed from 2.3: DONTNEED, 2.4: REQUIRED to 2.3: DONTNEED, 2.4: DONE ruby_2_4 r62233 merged revision(s) 60980,60984. ---------------------------------------- Bug #14070: Refining a module dumps core https://bugs.ruby-lang.org/issues/14070#change-70177 * Author: mame (Yusuke Endoh) * Status: Closed * Priority: Normal * Assignee: matz (Yukihiro Matsumoto) * Target version: 2.5 * ruby -v: ruby 2.5.0dev (2017-10-30 trunk 60565) [x86_64-linux] * Backport: 2.3: DONTNEED, 2.4: DONE ---------------------------------------- Including and refining one module at a time, seems to cause "double free" bug. Here is a short example (ko1 found): ~~~ loop do Class.new do include Enumerable end Module.new do refine Enumerable do def foo end end end end ~~~ `make test-spec` fails with SEGV very infrequently. The refinement module (the return value of `refine Enumerable`) seems to be included to Enumerable, and simultaneously, to have Enumerable as a superclass. Is this intended? The current VM assumes that a module has no superclass, and that a class is not included to other class. This discrepancy causes SEGV. If this is really intended, we need to extend VM to support a module that has a superclass and is included. The following patch fixes the issue by keeping a included module list in addition to a subclass list. I'm unsure if this is enough or not. ~~~ diff --git a/class.c b/class.c index 364f258333..a092fd7cc9 100644 --- a/class.c +++ b/class.c @@ -62,14 +62,14 @@ rb_module_add_to_subclasses_list(VALUE module, VALUE iclass) entry->klass = iclass; entry->next = NULL; - head = RCLASS_EXT(module)->subclasses; + head = RCLASS_EXT(module)->submodules; if (head) { entry->next = head; RCLASS_EXT(head->klass)->module_subclasses = &entry->next; } - RCLASS_EXT(module)->subclasses = entry; - RCLASS_EXT(iclass)->module_subclasses = &RCLASS_EXT(module)->subclasses; + RCLASS_EXT(module)->submodules = entry; + RCLASS_EXT(iclass)->module_subclasses = &RCLASS_EXT(module)->submodules; } void @@ -110,10 +110,8 @@ rb_class_remove_from_module_subclasses(VALUE klass) } void -rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE arg) +rb_class_foreach_subclass(rb_subclass_entry_t *cur, void (*f)(VALUE, VALUE), VALUE arg) { - rb_subclass_entry_t *cur = RCLASS_EXT(klass)->subclasses; - /* do not be tempted to simplify this loop into a for loop, the order of operations is important here if `f` modifies the linked list */ while (cur) { @@ -132,7 +130,7 @@ class_detach_subclasses(VALUE klass, VALUE arg) void rb_class_detach_subclasses(VALUE klass) { - rb_class_foreach_subclass(klass, class_detach_subclasses, Qnil); + rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, class_detach_subclasses, Qnil); } static void @@ -144,7 +142,7 @@ class_detach_module_subclasses(VALUE klass, VALUE arg) void rb_class_detach_module_subclasses(VALUE klass) { - rb_class_foreach_subclass(klass, class_detach_module_subclasses, Qnil); + rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, class_detach_module_subclasses, Qnil); } /** diff --git a/gc.c b/gc.c index 0d6163ea98..a85375ffd7 100644 --- a/gc.c +++ b/gc.c @@ -2220,14 +2220,12 @@ obj_free(rb_objspace_t *objspace, VALUE obj) if (RCLASS_IV_INDEX_TBL(obj)) { st_free_table(RCLASS_IV_INDEX_TBL(obj)); } + if (RCLASS_EXT(obj)->submodules) { + rb_class_detach_module_subclasses(obj); + RCLASS_EXT(obj)->submodules = NULL; + } if (RCLASS_EXT(obj)->subclasses) { - if (BUILTIN_TYPE(obj) == T_MODULE) { - rb_class_detach_module_subclasses(obj); - } - else { - rb_class_detach_subclasses(obj); - } - RCLASS_EXT(obj)->subclasses = NULL; + rb_class_detach_subclasses(obj); } rb_class_remove_from_module_subclasses(obj); rb_class_remove_from_super_subclasses(obj); diff --git a/internal.h b/internal.h index ad29434c7c..10b8051748 100644 --- a/internal.h +++ b/internal.h @@ -764,6 +764,7 @@ struct rb_classext_struct { * in the module's `subclasses` list that indicates that the klass has been * included. Hopefully that makes sense. */ + rb_subclass_entry_t *submodules; rb_subclass_entry_t **module_subclasses; rb_serial_t class_serial; const VALUE origin_; @@ -1077,7 +1078,7 @@ VALUE rb_class_boot(VALUE); VALUE rb_class_inherited(VALUE, VALUE); VALUE rb_make_metaclass(VALUE, VALUE); VALUE rb_include_class_new(VALUE, VALUE); -void rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE); +void rb_class_foreach_subclass(rb_subclass_entry_t *cur, void (*f)(VALUE, VALUE), VALUE); void rb_class_detach_subclasses(VALUE); void rb_class_detach_module_subclasses(VALUE); void rb_class_remove_from_module_subclasses(VALUE); diff --git a/vm_method.c b/vm_method.c index 3378f10bd5..817ae09fcb 100644 --- a/vm_method.c +++ b/vm_method.c @@ -77,7 +77,8 @@ rb_class_clear_method_cache(VALUE klass, VALUE arg) } } - rb_class_foreach_subclass(klass, rb_class_clear_method_cache, arg); + rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, rb_class_clear_method_cache, arg); + rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, rb_class_clear_method_cache, arg); } void @@ -103,7 +104,14 @@ rb_clear_method_cache_by_class(VALUE klass) } if (klass == rb_mKernel) { - rb_subclass_entry_t *entry = RCLASS_EXT(klass)->subclasses; + rb_subclass_entry_t *entry = RCLASS_EXT(klass)->submodules; + + for (; entry != NULL; entry = entry->next) { + struct rb_id_table *table = RCLASS_CALLABLE_M_TBL(entry->klass); + if (table)rb_id_table_clear(table); + } + + entry = RCLASS_EXT(klass)->subclasses; for (; entry != NULL; entry = entry->next) { struct rb_id_table *table = RCLASS_CALLABLE_M_TBL(entry->klass); @@ -479,7 +487,8 @@ check_override_opt_method(VALUE klass, VALUE arg) if (newme != me) rb_vm_check_redefinition_opt_method(me, me->owner); } } - rb_class_foreach_subclass(klass, check_override_opt_method, (VALUE)mid); + rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, check_override_opt_method, (VALUE)mid); + rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, check_override_opt_method, (VALUE)mid); } /* ~~~ -- https://bugs.ruby-lang.org/ Unsubscribe: