From 3acaa44a70a44a816076dee65310f0f2487aeebd Mon Sep 17 00:00:00 2001
From: "Yukihiro \"Matz\" Matsumoto" <matz@ruby-lang.org>
Date: Mon, 4 Sep 2017 06:51:31 +0900
Subject: Restructure `irep->outer` chain; fix #3804

Instead of `irep -> proc` chain, we use `irep -> irep` chain to
avoid GC bugs like #3804. We added `target_class` reference to
`mrb_irep` struct. That means one more word consumption per `irep`.
---
 src/gc.c       |  2 +-
 src/kernel.c   | 10 ++++------
 src/state.c    |  2 ++
 src/variable.c | 13 ++++++-------
 src/vm.c       | 36 +++++++++++++++++++++++++++---------
 5 files changed, 40 insertions(+), 23 deletions(-)

(limited to 'src')

diff --git a/src/gc.c b/src/gc.c
index 65727ece9..d602bfb70 100644
--- a/src/gc.c
+++ b/src/gc.c
@@ -648,7 +648,7 @@ gc_mark_children(mrb_state *mrb, mrb_gc *gc, struct RBasic *obj)
       mrb_gc_mark(mrb, (struct RBasic*)p->env);
       mrb_gc_mark(mrb, (struct RBasic*)p->target_class);
       if (!MRB_PROC_CFUNC_P(p) && p->body.irep) {
-        mrb_gc_mark(mrb, (struct RBasic*)p->body.irep->outer);
+        mrb_gc_mark(mrb, (struct RBasic*)p->body.irep->target_class);
       }
     }
     break;
diff --git a/src/kernel.c b/src/kernel.c
index 9fcee2413..4e95ab24b 100644
--- a/src/kernel.c
+++ b/src/kernel.c
@@ -1151,8 +1151,8 @@ static mrb_value
 mrb_local_variables(mrb_state *mrb, mrb_value self)
 {
   struct RProc *proc;
+  mrb_irep *irep;
   mrb_value vars;
-  struct mrb_irep *irep;
   size_t i;
 
   proc = mrb->c->ci[-1].proc;
@@ -1161,18 +1161,16 @@ mrb_local_variables(mrb_state *mrb, mrb_value self)
     return mrb_ary_new(mrb);
   }
   vars = mrb_hash_new(mrb);
-  while (proc) {
-    if (MRB_PROC_CFUNC_P(proc)) break;
-    irep = proc->body.irep;
+  irep = proc->body.irep;
+  while (irep) {
     if (!irep->lv) break;
     for (i = 0; i + 1 < irep->nlocals; ++i) {
       if (irep->lv[i].name) {
         mrb_hash_set(mrb, vars, mrb_symbol_value(irep->lv[i].name), mrb_true_value());
       }
     }
-    if (MRB_PROC_CLASS_P(proc)) break;
     if (!proc->env) break;
-    proc = proc->body.irep->outer;
+    irep = irep->outer;
   }
 
   return mrb_hash_keys(mrb, vars);
diff --git a/src/state.c b/src/state.c
index f09b20903..039d67d57 100644
--- a/src/state.c
+++ b/src/state.c
@@ -157,6 +157,8 @@ mrb_irep_free(mrb_state *mrb, mrb_irep *irep)
   for (i=0; i<irep->rlen; i++) {
     mrb_irep_decref(mrb, irep->reps[i]);
   }
+  if (irep->outer)
+    mrb_irep_decref(mrb, irep->outer);
   mrb_free(mrb, irep->reps);
   mrb_free(mrb, irep->lv);
   if (irep->own_filename) {
diff --git a/src/variable.c b/src/variable.c
index 96ae7ea25..50fc70682 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -702,7 +702,7 @@ mrb_vm_const_get(mrb_state *mrb, mrb_sym sym)
   struct RClass *c = mrb->c->ci->proc->target_class;
   struct RClass *c2;
   mrb_value v;
-  struct RProc *proc;
+  mrb_irep *irep;
 
   if (!c) c = mrb->c->ci->target_class;
   mrb_assert(c != NULL);
@@ -719,16 +719,15 @@ mrb_vm_const_get(mrb_state *mrb, mrb_sym sym)
   }
   if (c2->tt == MRB_TT_CLASS || c2->tt == MRB_TT_MODULE) c = c2;
   mrb_assert(!MRB_PROC_CFUNC_P(mrb->c->ci->proc));
-  proc = mrb->c->ci->proc->body.irep->outer;
-  while (proc && proc->tt == MRB_TT_PROC) {
-    mrb_assert(!MRB_PROC_CFUNC_P(proc));
-    if (MRB_PROC_CLASS_P(proc) && proc->target_class) {
-      c2 = proc->target_class;
+  irep = mrb->c->ci->proc->body.irep;
+  while (irep) {
+    if (irep->target_class) {
+      c2 = irep->target_class;
       if (c2->iv && iv_get(mrb, c2->iv, sym, &v)) {
         return v;
       }
     }
-    proc = proc->body.irep->outer;
+    irep = irep->outer;
   }
   return const_get(mrb, c, sym);
 }
diff --git a/src/vm.c b/src/vm.c
index 9486bd1f2..f413211e7 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -770,17 +770,23 @@ mrb_value
 mrb_mod_s_nesting(mrb_state *mrb, mrb_value mod)
 {
   struct RProc *proc;
+  mrb_irep *irep;
   mrb_value ary;
+  struct RClass *c;
 
   mrb_get_args(mrb, "");
   ary = mrb_ary_new(mrb);
   proc = mrb->c->ci[-1].proc;   /* callee proc */
-  while (proc) {
-    mrb_assert(!MRB_PROC_CFUNC_P(proc));
-    if (MRB_PROC_CLASS_P(proc) && proc->target_class) {
-      mrb_ary_push(mrb, ary, mrb_obj_value(proc->target_class));
-    }
-    proc = proc->body.irep->outer;
+  c = proc->target_class;
+  mrb_ary_push(mrb, ary, mrb_obj_value(c));
+  mrb_assert(!MRB_PROC_CFUNC_P(proc));
+  irep = proc->body.irep;
+  while (irep) {
+    if (irep->target_class && irep->target_class != c) {
+      c = irep->target_class;
+      mrb_ary_push(mrb, ary, mrb_obj_value(c));
+    }
+    irep = irep->outer;
   }
   return ary;
 }
@@ -846,6 +852,18 @@ argnum_error(mrb_state *mrb, mrb_int num)
   mrb_exc_set(mrb, exc);
 }
 
+void
+irep_uplink(mrb_state *mrb, mrb_irep *outer, mrb_irep *irep)
+{
+  if (irep->outer != outer) {
+    if (irep->outer) {
+      mrb_irep_decref(mrb, irep->outer);
+    }
+    irep->outer = outer;
+    mrb_irep_incref(mrb, outer);
+  }
+}
+
 #define ERR_PC_SET(mrb, pc) mrb->c->ci->err = pc;
 #define ERR_PC_CLR(mrb)     mrb->c->ci->err = 0;
 #ifdef MRB_ENABLE_DEBUG_HOOK
@@ -2647,7 +2665,7 @@ RETRY_TRY_BLOCK:
       int c = GETARG_c(i);
       mrb_irep *nirep = irep->reps[b];
 
-      nirep->outer = mrb->c->ci->proc;
+      irep_uplink(mrb, irep, nirep);
       if (c & OP_L_CAPTURE) {
         p = mrb_closure_new(mrb, nirep);
       }
@@ -2716,11 +2734,11 @@ RETRY_TRY_BLOCK:
       struct RProc *p;
       mrb_irep *nirep = irep->reps[bx];
 
-      nirep->outer = mrb->c->ci->proc;
+      irep_uplink(mrb, irep, nirep);
+      nirep->target_class = mrb_class_ptr(recv);
       /* prepare closure */
       p = mrb_closure_new(mrb, nirep);
       p->c = NULL;
-      p->flags |= MRB_PROC_CLASS;
 
       /* prepare stack */
       ci = cipush(mrb);
-- 
cgit v1.2.3