From 93f5f225772c398be6e409da3d3ef0f07ffbe1cf Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Thu, 26 Oct 2017 01:13:57 +0900 Subject: Heavily refactored how lexical scope links are implemented; fix #3821 Instead of `irep` links, we added a `upper` link to `struct RProc`. To make a space for the `upper` link, we moved `target_class` reference. If a `Proc` does not have `env`, `target_class` is saved in an `union` shared with `env` (if a `Proc` has env, you can tell it by `MRB_PROC_ENV_P()). Otherwise `target_class` is referenced from `env->c`. We removed links in `env` as well. This change removes 2 members from `mrb_irep` struct, thus saving 2 words per method/proc/block. This also fixes potential memory leaks due to the circular references caused by a link from `mrb_irep`. --- mrbgems/mruby-eval/src/eval.c | 48 +++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 27 deletions(-) (limited to 'mrbgems/mruby-eval/src') diff --git a/mrbgems/mruby-eval/src/eval.c b/mrbgems/mruby-eval/src/eval.c index 7fd4f1437..7061565ea 100644 --- a/mrbgems/mruby-eval/src/eval.c +++ b/mrbgems/mruby-eval/src/eval.c @@ -13,11 +13,9 @@ static struct mrb_irep * get_closure_irep(mrb_state *mrb, int level) { struct mrb_context *c = mrb->c; - struct REnv *e = c->ci[-1].proc->env; - struct RProc *proc; + struct RProc *proc = c->ci[-1].proc; if (level == 0) { - proc = c->ci[-1].proc; if (MRB_PROC_CFUNC_P(proc)) { return NULL; } @@ -25,16 +23,11 @@ get_closure_irep(mrb_state *mrb, int level) } while (--level) { - e = (struct REnv*)e->c; - if (!e) return NULL; + proc = proc->upper; + if (!proc) return NULL; } - if (!e) return NULL; - if (!MRB_ENV_STACK_SHARED_P(e)) return NULL; - c = e->cxt.c; - proc = c->cibase[e->cioff].proc; - - if (!proc || MRB_PROC_CFUNC_P(proc)) { + if (MRB_PROC_CFUNC_P(proc)) { return NULL; } return proc->body.irep; @@ -204,7 +197,9 @@ create_proc_from_string(mrb_state *mrb, char *s, mrb_int len, mrb_value binding, struct mrb_parser_state *p; struct RProc *proc; struct REnv *e; - struct mrb_context *c = mrb->c; + mrb_callinfo *ci = mrb->c->ci; + struct RClass *target_class = NULL; + int bidx; if (!mrb_nil_p(binding)) { mrb_raise(mrb, E_ARGUMENT_ERROR, "Binding of eval must be nil."); @@ -251,19 +246,19 @@ create_proc_from_string(mrb_state *mrb, char *s, mrb_int len, mrb_value binding, mrbc_context_free(mrb, cxt); mrb_raise(mrb, E_SCRIPT_ERROR, "codegen error"); } - if (c->ci[-1].proc->target_class) { - proc->target_class = c->ci[-1].proc->target_class; - } - e = c->ci[-1].proc->env; - if (!e) e = c->ci[-1].env; - e = (struct REnv*)mrb_obj_alloc(mrb, MRB_TT_ENV, (struct RClass*)e); - e->cxt.c = c; - e->cioff = c->ci - c->cibase; - e->stack = c->ci->stackent; - MRB_SET_ENV_STACK_LEN(e, c->ci->proc->body.irep->nlocals); - c->ci->target_class = proc->target_class; - c->ci->env = 0; - proc->env = e; + target_class = MRB_PROC_TARGET_CLASS(ci[-1].proc); + e = (struct REnv*)mrb_obj_alloc(mrb, MRB_TT_ENV, + (struct RClass*)target_class); + e->mid = ci->mid; + e->stack = ci->stackent; + MRB_ENV_SET_STACK_LEN(e, ci->proc->body.irep->nlocals); + bidx = ci->argc; + if (ci->argc < 0) bidx = 2; + else bidx += 1; + MRB_ENV_SET_BIDX(e, bidx); + proc->e.env = e; + proc->flags |= MRB_PROC_ENVSET; + ci->target_class = target_class; patch_irep(mrb, proc->body.irep, 0, proc->body.irep); /* mrb_codedump_all(mrb, proc); */ @@ -322,8 +317,7 @@ f_instance_eval(mrb_state *mrb, mrb_value self) mrb_get_args(mrb, "s|zi", &s, &len, &file, &line); cv = mrb_singleton_class(mrb, self); proc = create_proc_from_string(mrb, s, len, mrb_nil_value(), file, line); - proc->target_class = mrb_class_ptr(cv); - mrb->c->ci->env = NULL; + MRB_PROC_SET_TARGET_CLASS(proc, mrb_class_ptr(cv)); mrb_assert(!MRB_PROC_CFUNC_P(proc)); return exec_irep(mrb, self, proc); } -- cgit v1.2.3 From c7c9543bed57db12fd8aa57391e0b652ee27cb23 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Sat, 28 Oct 2017 21:09:25 +0900 Subject: Fixed UPVAR gotchas; fix #3835 Both `uvenv` function and `env` generation in `create_proc_from_string` function have bugs to handling enclosed environment objects. --- mrbgems/mruby-eval/src/eval.c | 58 ++++++++++++++++++++++--------------------- src/vm.c | 21 ++++++++++++---- 2 files changed, 46 insertions(+), 33 deletions(-) (limited to 'mrbgems/mruby-eval/src') diff --git a/mrbgems/mruby-eval/src/eval.c b/mrbgems/mruby-eval/src/eval.c index 7061565ea..997b69e25 100644 --- a/mrbgems/mruby-eval/src/eval.c +++ b/mrbgems/mruby-eval/src/eval.c @@ -12,21 +12,13 @@ mrb_value mrb_obj_instance_eval(mrb_state *mrb, mrb_value self); static struct mrb_irep * get_closure_irep(mrb_state *mrb, int level) { - struct mrb_context *c = mrb->c; - struct RProc *proc = c->ci[-1].proc; + struct RProc *proc = mrb->c->ci[-1].proc; - if (level == 0) { - if (MRB_PROC_CFUNC_P(proc)) { - return NULL; - } - return proc->body.irep; - } - - while (--level) { - proc = proc->upper; + while (level--) { if (!proc) return NULL; + proc = proc->upper; } - + if (!proc) return NULL; if (MRB_PROC_CFUNC_P(proc)) { return NULL; } @@ -60,7 +52,7 @@ search_variable(mrb_state *mrb, mrb_sym vsym, int bnest) int pos; for (level = 0; (virep = get_closure_irep(mrb, level)); level++) { - if (!virep || virep->lv == NULL) { + if (virep->lv == NULL) { continue; } for (pos = 0; pos < virep->nlocals - 1; pos++) { @@ -123,7 +115,7 @@ patch_irep(mrb_state *mrb, mrb_irep *irep, int bnest, mrb_irep *top) if (GETARG_C(c) != 0) { break; } - { + else { mrb_code arg = search_variable(mrb, irep->syms[GETARG_B(c)], bnest); if (arg != 0) { /* must replace */ @@ -197,7 +189,7 @@ create_proc_from_string(mrb_state *mrb, char *s, mrb_int len, mrb_value binding, struct mrb_parser_state *p; struct RProc *proc; struct REnv *e; - mrb_callinfo *ci = mrb->c->ci; + mrb_callinfo *ci = &mrb->c->ci[-1]; /* callinfo of eval caller */ struct RClass *target_class = NULL; int bidx; @@ -246,19 +238,29 @@ create_proc_from_string(mrb_state *mrb, char *s, mrb_int len, mrb_value binding, mrbc_context_free(mrb, cxt); mrb_raise(mrb, E_SCRIPT_ERROR, "codegen error"); } - target_class = MRB_PROC_TARGET_CLASS(ci[-1].proc); - e = (struct REnv*)mrb_obj_alloc(mrb, MRB_TT_ENV, - (struct RClass*)target_class); - e->mid = ci->mid; - e->stack = ci->stackent; - MRB_ENV_SET_STACK_LEN(e, ci->proc->body.irep->nlocals); - bidx = ci->argc; - if (ci->argc < 0) bidx = 2; - else bidx += 1; - MRB_ENV_SET_BIDX(e, bidx); - proc->e.env = e; - proc->flags |= MRB_PROC_ENVSET; - ci->target_class = target_class; + target_class = MRB_PROC_TARGET_CLASS(ci->proc); + if (!MRB_PROC_CFUNC_P(ci->proc)) { + if (ci->env) { + e = ci->env; + } + else { + e = (struct REnv*)mrb_obj_alloc(mrb, MRB_TT_ENV, + (struct RClass*)target_class); + e->mid = ci->mid; + e->stack = ci[1].stackent; + e->cxt = mrb->c; + MRB_ENV_SET_STACK_LEN(e, ci->proc->body.irep->nlocals); + bidx = ci->argc; + if (ci->argc < 0) bidx = 2; + else bidx += 1; + MRB_ENV_SET_BIDX(e, bidx); + } + proc->e.env = e; + proc->flags |= MRB_PROC_ENVSET; + mrb_field_write_barrier(mrb, (struct RBasic*)proc, (struct RBasic*)e); + } + proc->upper = ci->proc; + mrb->c->ci->target_class = target_class; patch_irep(mrb, proc->body.irep, 0, proc->body.irep); /* mrb_codedump_all(mrb, proc); */ diff --git a/src/vm.c b/src/vm.c index 9758ccf1e..b1e2b624b 100644 --- a/src/vm.c +++ b/src/vm.c @@ -217,13 +217,24 @@ uvenv(mrb_state *mrb, int up) struct RProc *proc = mrb->c->ci->proc; struct REnv *e; - do { - e = MRB_PROC_ENV(proc); - if (!e) return NULL; + while (up--) { proc = proc->upper; if (!proc) return NULL; - } while (up--); - return e; + } + e = MRB_PROC_ENV(proc); + if (e) return e; /* proc has enclosed env */ + else { + mrb_callinfo *ci = mrb->c->ci; + mrb_callinfo *cb = mrb->c->cibase; + + while (cb <= ci) { + if (ci->proc == proc) { + return ci->env; + } + ci--; + } + } + return NULL; } static inline struct RProc* -- cgit v1.2.3