Revert ruby#2486

This reverts commits: 10d6a3a 8ba48c1 fba8627 dd883de
6c6a25f 167e6b4 7cb96d4 3207979 595b3c4 1521f7c
c11c5e6 cf33608 3632a81 f56506b 86427a3 .

The reason for the revert is that we observe ABA problem around
inline method cache.  When a cache misshits, we search for a
method entry.  And if the entry is identical to what was cached
before, we reuse the cache.  But the commits we are reverting here
introduced situations where a method entry is freed, then the
identical memory region is used for another method entry.  An
inline method cache cannot detect that ABA.

Here is a code that reproduce such situation:

```ruby
require 'prime'

class << Integer
  alias org_sqrt sqrt
  def sqrt(n)
    raise
  end

  GC.stress = true
  Prime.each(7*37){} rescue nil # <- Here we populate CC
  class << Object.new; end

  # These adjacent remove-then-alias maneuver
  # frees a method entry, then immediately
  # reuses it for another.
  remove_method :sqrt
  alias sqrt org_sqrt
end

Prime.each(7*37).to_a # <- SEGV
```

Author: shyouhei

class.c

@@ -956,41 +956,25 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super)
     return method_changed;
 }
 
-typedef struct tuple {
-    struct RClass *klass;
-    struct RClass *origin;
-} tuple;
-
-static enum rb_id_table_iterator_result
-inject_refined_method(ID *key, VALUE *value, void *data, int _)
-{
-    const tuple             *ptr     = data;
-    const rb_method_entry_t *me      = *(const rb_method_entry_t **) value;
-    const rb_method_entry_t *orig_me = me->def->body.refined.orig_me;
-    const rb_method_entry_t *new_me  =
-        rb_method_entry_from_template(
-            me, &(rb_method_refined_t) {
-                .orig_me = NULL,
-                .owner   = me->def->body.refined.owner, });
-    rb_id_table_insert(RCLASS_M_TBL(ptr->klass), *key, (VALUE)new_me);
-    RB_OBJ_WRITTEN(ptr->klass, Qundef, new_me);
-    *value = (VALUE)rb_method_entry_clone(orig_me);
-    RB_OBJ_WRITTEN(ptr->origin, Qundef, orig_me);
-    return ID_TABLE_CONTINUE;
-}
-
 static enum rb_id_table_iterator_result
 move_refined_method(ID key, VALUE value, void *data)
 {
-    const tuple             *ptr = data;
-    const rb_method_entry_t *me  = (const rb_method_entry_t *) value;
+    rb_method_entry_t *me = (rb_method_entry_t *) value;
+    VALUE klass = (VALUE)data;
+    struct rb_id_table *tbl = RCLASS_M_TBL(klass);
 
     if (me->def->type == VM_METHOD_TYPE_REFINED) {
 	if (me->def->body.refined.orig_me) {
-            return ID_TABLE_REPLACE;
+	    const rb_method_entry_t *orig_me = me->def->body.refined.orig_me, *new_me;
+	    RB_OBJ_WRITE(me, &me->def->body.refined.orig_me, NULL);
+	    new_me = rb_method_entry_clone(me);
+	    rb_id_table_insert(tbl, key, (VALUE)new_me);
+	    RB_OBJ_WRITTEN(klass, Qundef, new_me);
+	    rb_method_entry_copy(me, orig_me);
+	    return ID_TABLE_CONTINUE;
 	}
 	else {
-            rb_id_table_insert(RCLASS_M_TBL(ptr->klass), key, (VALUE)me);
+	    rb_id_table_insert(tbl, key, (VALUE)me);
 	    return ID_TABLE_DELETE;
 	}
     }
@@ -1016,12 +1000,7 @@ rb_prepend_module(VALUE klass, VALUE module)
 	RCLASS_SET_ORIGIN(klass, origin);
 	RCLASS_M_TBL(origin) = RCLASS_M_TBL(klass);
 	RCLASS_M_TBL_INIT(klass);
-        rb_id_table_foreach_with_replace_with_key(
-            RCLASS_M_TBL(origin),
-            move_refined_method,
-            inject_refined_method,
-            &(tuple) { RCLASS(klass), RCLASS(origin), },
-            true);
+	rb_id_table_foreach(RCLASS_M_TBL(origin), move_refined_method, (void *)klass);
     }
     changed = include_modules_at(klass, klass, module, FALSE);
     if (changed < 0)

ext/coverage/coverage.c

@@ -123,7 +123,7 @@ method_coverage_i(void *vstart, void *vend, size_t stride, void *data)
 
     for (v = (VALUE)vstart; v != (VALUE)vend; v += stride) {
 	if (RB_TYPE_P(v, T_IMEMO) && imemo_type(v) == imemo_ment) {
-            const rb_method_entry_t *me = (const rb_method_entry_t *) v;
+	    const rb_method_entry_t *me = (rb_method_entry_t *) v;
 	    VALUE path, first_lineno, first_column, last_lineno, last_column;
 	    VALUE data[5], ncoverage, methods;
 	    VALUE methods_id = ID2SYM(rb_intern("methods"));

gc.c

@@ -7829,9 +7829,9 @@ void rb_update_st_references(struct st_table *ht)
 }
 
 static void
-gc_ref_update_method_entry(rb_objspace_t *objspace, const rb_method_entry_t *me)
+gc_ref_update_method_entry(rb_objspace_t *objspace, rb_method_entry_t *me)
 {
-    const rb_method_definition_t *def = me->def;
+    rb_method_definition_t *def = me->def;
 
     UPDATE_IF_MOVED(objspace, me->owner);
     UPDATE_IF_MOVED(objspace, me->defined_class);

id_table.c

@@ -269,62 +269,57 @@ rb_id_table_delete(struct rb_id_table *tbl, ID id)
 void
 rb_id_table_foreach_with_replace(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, rb_id_table_update_callback_func_t *replace, void *data)
 {
-    rb_id_table_foreach_with_replace_with_key(tbl, func, replace, data, false);
+    int i, capa = tbl->capa;
+
+    for (i=0; i<capa; i++) {
+        if (ITEM_KEY_ISSET(tbl, i)) {
+            const id_key_t key = ITEM_GET_KEY(tbl, i);
+            enum rb_id_table_iterator_result ret = (*func)(Qundef, tbl->items[i].val, data);
+            assert(key != 0);
+
+            if (ret == ID_TABLE_REPLACE) {
+                VALUE val = tbl->items[i].val;
+                ret = (*replace)(NULL, &val, data, TRUE);
+                tbl->items[i].val = val;
+            }
+            else if (ret == ID_TABLE_STOP)
+                return;
+        }
+    }
 }
 
 void
 rb_id_table_foreach(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, void *data)
 {
-    rb_id_table_foreach_with_replace_with_key(tbl, func, 0, data, true);
-}
-
-typedef struct tuple {
-    rb_id_table_foreach_values_func_t *const func;
-    void *const data;
-} tuple;
+    int i, capa = tbl->capa;
 
-static enum rb_id_table_iterator_result
-cdr(ID car, VALUE cdr, void *data)
-{
-    const tuple *ptr = data;
-    return ptr->func(cdr, ptr->data);
+    for (i=0; i<capa; i++) {
+	if (ITEM_KEY_ISSET(tbl, i)) {
+	    const id_key_t key = ITEM_GET_KEY(tbl, i);
+	    enum rb_id_table_iterator_result ret = (*func)(key2id(key), tbl->items[i].val, data);
+	    assert(key != 0);
+
+	    if (ret == ID_TABLE_DELETE)
+		hash_delete_index(tbl, i);
+	    else if (ret == ID_TABLE_STOP)
+		return;
+	}
+    }
 }
 
 void
 rb_id_table_foreach_values(struct rb_id_table *tbl, rb_id_table_foreach_values_func_t *func, void *data)
 {
-    rb_id_table_foreach_with_replace(
-        tbl, cdr, 0, &(tuple) { func, data, });
-}
+    int i, capa = tbl->capa;
 
-void
-rb_id_table_foreach_with_replace_with_key(
-    struct rb_id_table *tbl,
-    rb_id_table_foreach_func_t *func,
-    rb_id_table_update_callback_func_t *replace,
-    void *data,
-    bool needkey)
-{
-    for (int i = 0; i < tbl->capa; i++) {
-        if (ITEM_KEY_ISSET(tbl, i)) {
-            const id_key_t key = ITEM_GET_KEY(tbl, i);
-            assert(key != 0);
-            ID k = needkey ? key2id(key) : 0;
-            VALUE v = tbl->items[i].val;
-            switch (func(k, v, data)) {
-              case ID_TABLE_DELETE:
-                hash_delete_index(tbl, i);
-                /* FALLTHROUGH */
-              case ID_TABLE_CONTINUE:
-                continue;
-              case ID_TABLE_STOP:
-                return;
-              case ID_TABLE_REPLACE:
-                if (replace) {
-                    replace(&k, &v, data, true);
-                    tbl->items[i].val = v;
-                }
-            }
-        }
+    for (i=0; i<capa; i++) {
+	if (ITEM_KEY_ISSET(tbl, i)) {
+	    enum rb_id_table_iterator_result ret = (*func)(tbl->items[i].val, data);
+
+	    if (ret == ID_TABLE_DELETE)
+		hash_delete_index(tbl, i);
+	    else if (ret == ID_TABLE_STOP)
+		return;
+	}
     }
 }

id_table.h

@@ -10,6 +10,7 @@ enum rb_id_table_iterator_result {
     ID_TABLE_STOP     = ST_STOP,
     ID_TABLE_DELETE   = ST_DELETE,
     ID_TABLE_REPLACE  = ST_REPLACE,
+    ID_TABLE_ITERATOR_RESULT_END
 };
 
 struct rb_id_table *rb_id_table_create(size_t size);
@@ -28,7 +29,6 @@ typedef enum rb_id_table_iterator_result rb_id_table_foreach_func_t(ID id, VALUE
 typedef enum rb_id_table_iterator_result rb_id_table_foreach_values_func_t(VALUE val, void *data);
 void rb_id_table_foreach(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, void *data);
 void rb_id_table_foreach_with_replace(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, rb_id_table_update_callback_func_t *replace, void *data);
-void rb_id_table_foreach_with_replace_with_key(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, rb_id_table_update_callback_func_t *replace, void *data, bool needkey);
 void rb_id_table_foreach_values(struct rb_id_table *tbl, rb_id_table_foreach_values_func_t *func, void *data);
 
 #endif	/* RUBY_ID_TABLE_H */

insns.def

@@ -911,7 +911,7 @@ invokeblock
 // attr rb_snum_t sp_inc = sp_inc_of_invokeblock(ci);
 {
     static struct rb_call_cache cc = {
-        0, 0, NULL, vm_invokeblock_i,
+        0, 0, NULL, NULL, vm_invokeblock_i,
     };
 
     VALUE bh = VM_BLOCK_HANDLER_NONE;

internal.h

@@ -2330,6 +2330,7 @@ enum method_missing_reason {
     MISSING_NONE      = 0x40
 };
 struct rb_callable_method_entry_struct;
+struct rb_method_definition_struct;
 struct rb_execution_context_struct;
 struct rb_control_frame_struct;
 struct rb_calling_info;
@@ -2341,6 +2342,7 @@ struct rb_call_cache {
 
     /* inline cache: values */
     const struct rb_callable_method_entry_struct *me;
+    const struct rb_method_definition_struct *def;
 
     VALUE (*call)(struct rb_execution_context_struct *ec,
                   struct rb_control_frame_struct *cfp,

method.h

@@ -49,23 +49,54 @@ typedef struct rb_cref_struct {
 /* method data type */
 
 typedef struct rb_method_entry_struct {
-    const VALUE flags;
-    const VALUE defined_class;
+    VALUE flags;
+    VALUE defined_class;
     struct rb_method_definition_struct * const def;
-    const ID called_id;
-    const VALUE owner;
+    ID called_id;
+    VALUE owner;
 } rb_method_entry_t;
 
 typedef struct rb_callable_method_entry_struct { /* same fields with rb_method_entry_t */
-    const VALUE flags;
+    VALUE flags;
     const VALUE defined_class;
     struct rb_method_definition_struct * const def;
-    const ID called_id;
+    ID called_id;
     const VALUE owner;
 } rb_callable_method_entry_t;
 
 #define METHOD_ENTRY_VISI(me)  (rb_method_visibility_t)(((me)->flags & (IMEMO_FL_USER0 | IMEMO_FL_USER1)) >> (IMEMO_FL_USHIFT+0))
 #define METHOD_ENTRY_BASIC(me) (int)                   (((me)->flags & (IMEMO_FL_USER2                 )) >> (IMEMO_FL_USHIFT+2))
+#define METHOD_ENTRY_COMPLEMENTED(me)     ((me)->flags & IMEMO_FL_USER3)
+#define METHOD_ENTRY_COMPLEMENTED_SET(me) ((me)->flags = (me)->flags | IMEMO_FL_USER3)
+
+static inline void
+METHOD_ENTRY_VISI_SET(rb_method_entry_t *me, rb_method_visibility_t visi)
+{
+    VM_ASSERT((int)visi >= 0 && visi <= 3);
+    me->flags = (me->flags & ~(IMEMO_FL_USER0 | IMEMO_FL_USER1)) | (visi << (IMEMO_FL_USHIFT+0));
+}
+static inline void
+METHOD_ENTRY_BASIC_SET(rb_method_entry_t *me, unsigned int basic)
+{
+    VM_ASSERT(basic <= 1);
+    me->flags = (me->flags & ~(IMEMO_FL_USER2                 )) | (basic << (IMEMO_FL_USHIFT+2));
+}
+static inline void
+METHOD_ENTRY_FLAGS_SET(rb_method_entry_t *me, rb_method_visibility_t visi, unsigned int basic)
+{
+    VM_ASSERT((int)visi >= 0 && visi <= 3);
+    VM_ASSERT(basic <= 1);
+    me->flags =
+      (me->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2)) |
+	((visi << (IMEMO_FL_USHIFT+0)) | (basic << (IMEMO_FL_USHIFT+2)));
+}
+static inline void
+METHOD_ENTRY_FLAGS_COPY(rb_method_entry_t *dst, const rb_method_entry_t *src)
+{
+    dst->flags =
+      (dst->flags & ~(IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2)) |
+	(src->flags & (IMEMO_FL_USER0|IMEMO_FL_USER1|IMEMO_FL_USER2));
+}
 
 typedef enum {
     VM_METHOD_TYPE_ISEQ,      /*!< Ruby method */
@@ -93,32 +124,32 @@ typedef struct rb_iseq_struct rb_iseq_t;
 #endif
 
 typedef struct rb_method_iseq_struct {
-    const rb_iseq_t *const iseqptr; /*!< iseq pointer, should be separated from iseqval */
-    rb_cref_t *const cref;          /*!< class reference, should be marked */
-} rb_method_iseq_t;
+    rb_iseq_t * iseqptr; /*!< iseq pointer, should be separated from iseqval */
+    rb_cref_t * cref;          /*!< class reference, should be marked */
+} rb_method_iseq_t; /* check rb_add_method_iseq() when modify the fields */
 
 typedef struct rb_method_cfunc_struct {
-    VALUE (*const func)(ANYARGS);
-    VALUE (*const invoker)(VALUE recv, int argc, const VALUE *argv, VALUE (*func)(ANYARGS));
-    const int argc;
+    VALUE (*func)(ANYARGS);
+    VALUE (*invoker)(VALUE recv, int argc, const VALUE *argv, VALUE (*func)(ANYARGS));
+    int argc;
 } rb_method_cfunc_t;
 
 typedef struct rb_method_attr_struct {
-    const ID id;
-    const VALUE location; /* should be marked */
+    ID id;
+    VALUE location; /* should be marked */
 } rb_method_attr_t;
 
 typedef struct rb_method_alias_struct {
-    const struct rb_method_entry_struct *const original_me; /* original_me->klass is original owner */
+    struct rb_method_entry_struct * original_me; /* original_me->klass is original owner */
 } rb_method_alias_t;
 
 typedef struct rb_method_refined_struct {
-    const struct rb_method_entry_struct *const orig_me;
-    const VALUE owner;
+    struct rb_method_entry_struct * orig_me;
+    VALUE owner;
 } rb_method_refined_t;
 
 typedef struct rb_method_bmethod_struct {
-    const VALUE proc; /* should be marked */
+    VALUE proc; /* should be marked */
     struct rb_hook_list_struct *hooks;
 } rb_method_bmethod_t;
 
@@ -130,22 +161,22 @@ enum method_optimized_type {
 };
 
 struct rb_method_definition_struct {
-    BITFIELD(rb_method_type_t, const type, VM_METHOD_TYPE_MINIMUM_BITS);
+    BITFIELD(rb_method_type_t, type, VM_METHOD_TYPE_MINIMUM_BITS);
     int alias_count : 28;
     int complemented_count : 28;
 
     union {
-        const rb_method_iseq_t iseq;
-        const rb_method_cfunc_t cfunc;
-        const rb_method_attr_t attr;
-        const rb_method_alias_t alias;
-        const rb_method_refined_t refined;
+	rb_method_iseq_t iseq;
+	rb_method_cfunc_t cfunc;
+	rb_method_attr_t attr;
+	rb_method_alias_t alias;
+	rb_method_refined_t refined;
         rb_method_bmethod_t bmethod;
 
-        const enum method_optimized_type optimize_type;
+        enum method_optimized_type optimize_type;
     } body;
 
-    const ID original_id;
+    ID original_id;
 };
 
 typedef struct rb_method_definition_struct rb_method_definition_t;
@@ -161,9 +192,8 @@ void rb_add_method_iseq(VALUE klass, ID mid, const rb_iseq_t *iseq, rb_cref_t *c
 void rb_add_refined_method_entry(VALUE refined_class, ID mid);
 void rb_add_method(VALUE klass, ID mid, rb_method_type_t type, void *option, rb_method_visibility_t visi);
 
-const rb_method_entry_t *rb_method_entry_set(VALUE klass, ID mid, const rb_method_entry_t *, rb_method_visibility_t noex);
-const rb_method_entry_t *rb_method_entry_from_template(const rb_method_entry_t *template, const void *opts);
-const rb_method_entry_t *rb_method_entry_for_missing(ID mid, VALUE klass);
+rb_method_entry_t *rb_method_entry_set(VALUE klass, ID mid, const rb_method_entry_t *, rb_method_visibility_t noex);
+rb_method_entry_t *rb_method_entry_create(ID called_id, VALUE klass, rb_method_visibility_t visi, const rb_method_definition_t *def);
 
 const rb_method_entry_t *rb_method_entry_at(VALUE obj, ID id);
 
@@ -193,6 +223,7 @@ void rb_sweep_method_entry(void *vm);
 
 const rb_method_entry_t *rb_method_entry_clone(const rb_method_entry_t *me);
 const rb_callable_method_entry_t *rb_method_entry_complement_defined_class(const rb_method_entry_t *src_me, ID called_id, VALUE defined_class);
+void rb_method_entry_copy(rb_method_entry_t *dst, const rb_method_entry_t *src);
 
 void rb_scope_visibility_set(rb_method_visibility_t);