From c4bc9b575898ba352d0fea8289ebcaf8921dacd1 Mon Sep 17 00:00:00 2001 From: ko1 Date: Fri, 22 Jun 2012 09:32:56 +0000 Subject: [PATCH] * iseq.c, vm_eval.c: set th->base_block properly. th->base_block is information for (a) parsing, (b) compiling and (c) setting up the frame to execute the program passed by `eval' method. For example, (1) parser need to know up-level variables to detect it is variable or method without paren. Befor (a), (b) and (c), VM set th->base_block by passed bindng (or previous frame information). After execute (a), (b) and (c), VM should clear th->base_block. However, if (a), (b) or (c) raises an exception, then th->base_block is not cleared. Problem is that the uncleared value th->balo_block is used for irrelevant iseq compilation. It causes SEGV or critical error. I tried to solve this problem: to clear them before exception, but finally I found out that it is difficult to do it (Ruby program can be run in many places). Because of this background, I set th->base_block before compiling iseq and restore it after compiling. Basically, th->base_block is dirty hack (similar to global variable) and this patch is also dirty. * bootstraptest/test_eval.rb: add a test for above. * internal.h: remove unused decl. * iseq.c (rb_iseq_compile_with_option): add base_block parameter. set th->base_block before compation and restore it after compilation. * ruby.c (require_libraries): pass 0 as base_block instead of setting th->base_block * tool/compile_prelude.rb (prelude_eval): apply above changes. * vm.c, vm_eval.c: ditto. * vm_core.h: add comments. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@36179 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 38 ++++++++++++++++++++++++++ bootstraptest/test_eval.rb | 10 +++++++ internal.h | 1 - iseq.c | 56 ++++++++++++++++++++++++++++---------- ruby.c | 3 -- tool/compile_prelude.rb | 2 +- vm.c | 11 +++----- vm_core.h | 8 +++++- vm_eval.c | 17 ++++++------ 9 files changed, 109 insertions(+), 37 deletions(-) diff --git a/ChangeLog b/ChangeLog index 89a2292c1a..c481c4ffa5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,41 @@ +Fri Jun 22 18:04:26 2012 Koichi Sasada + + * iseq.c, vm_eval.c: set th->base_block properly. + th->base_block is information for (a) parsing, (b) compiling + and (c) setting up the frame to execute the program passed by + `eval' method. For example, (1) parser need to know up-level + variables to detect it is variable or method without paren. + Befor (a), (b) and (c), VM set th->base_block by passed bindng + (or previous frame information). After execute (a), (b) and (c), + VM should clear th->base_block. However, if (a), (b) or (c) + raises an exception, then th->base_block is not cleared. + Problem is that the uncleared value th->balo_block is used for + irrelevant iseq compilation. It causes SEGV or critical error. + I tried to solve this problem: to clear them before exception, + but finally I found out that it is difficult to do it (Ruby + program can be run in many places). + Because of this background, I set th->base_block before + compiling iseq and restore it after compiling. + Basically, th->base_block is dirty hack (similar to global + variable) and this patch is also dirty. + + * bootstraptest/test_eval.rb: add a test for above. + + * internal.h: remove unused decl. + + * iseq.c (rb_iseq_compile_with_option): add base_block parameter. + set th->base_block before compation and restore it after + compilation. + + * ruby.c (require_libraries): pass 0 as base_block instead of + setting th->base_block + + * tool/compile_prelude.rb (prelude_eval): apply above changes. + + * vm.c, vm_eval.c: ditto. + + * vm_core.h: add comments. + Fri Jun 22 18:19:38 2012 Tanaka Akira * process.c: pass struct rb_execarg value instead of its options diff --git a/bootstraptest/test_eval.rb b/bootstraptest/test_eval.rb index 18d4d6f11f..aab1a7b1a4 100644 --- a/bootstraptest/test_eval.rb +++ b/bootstraptest/test_eval.rb @@ -318,3 +318,13 @@ assert_normal_exit %q{ eval "class C; @@h = #{hash.inspect}; end" end }, '[ruby-core:25714]' + +assert_normal_exit %q{ + begin + eval("# encoding:utf-16le\nfoo") + rescue Exception => e + p e + RubyVM::InstructionSequence.compile("p:hello") + end +}, 'check escaping the internal value th->base_block' + diff --git a/internal.h b/internal.h index 24e219c8fd..1c73eadb84 100644 --- a/internal.h +++ b/internal.h @@ -112,7 +112,6 @@ ssize_t rb_io_bufread(VALUE io, void *buf, size_t size); void rb_stdio_set_default_encoding(void); /* iseq.c */ -VALUE rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE filepath, VALUE line, VALUE opt); VALUE rb_iseq_clone(VALUE iseqval, VALUE newcbase); /* load.c */ diff --git a/iseq.c b/iseq.c index 91597c3e69..890c6b248c 100644 --- a/iseq.c +++ b/iseq.c @@ -11,6 +11,7 @@ #include "ruby/ruby.h" #include "internal.h" +#include "eval_intern.h" /* #define RUBY_MARK_FREE_DEBUG 1 */ #include "gc.h" @@ -566,30 +567,55 @@ parse_string(VALUE str, const char *file, int line) } VALUE -rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE absolute_path, VALUE line, VALUE opt) +rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE absolute_path, VALUE line, rb_block_t *base_block, VALUE opt) { - rb_compile_option_t option; - const char *fn = StringValueCStr(file); - int ln = NUM2INT(line); - NODE *node = parse_string(StringValue(src), fn, ln); + int state; rb_thread_t *th = GET_THREAD(); - make_compile_option(&option, opt); + rb_block_t *prev_base_block = th->base_block; + VALUE iseqval; - if (th->base_block && th->base_block->iseq) { - return rb_iseq_new_with_opt(node, th->base_block->iseq->location.label, - file, absolute_path, line, th->base_block->iseq->self, - ISEQ_TYPE_EVAL, &option); + th->base_block = base_block; + + TH_PUSH_TAG(th); + if ((state = EXEC_TAG()) == 0) { + int ln = NUM2INT(line); + const char *fn = StringValueCStr(file); + NODE *node = parse_string(StringValue(src), fn, ln); + rb_compile_option_t option; + + make_compile_option(&option, opt); + + if (base_block && base_block->iseq) { + iseqval = rb_iseq_new_with_opt(node, base_block->iseq->location.label, + file, absolute_path, line, base_block->iseq->self, + ISEQ_TYPE_EVAL, &option); + } + else { + iseqval = rb_iseq_new_with_opt(node, rb_str_new2(""), file, absolute_path, line, Qfalse, + ISEQ_TYPE_TOP, &option); + } } - else { - return rb_iseq_new_with_opt(node, rb_str_new2(""), file, absolute_path, line, Qfalse, - ISEQ_TYPE_TOP, &option); + TH_POP_TAG(); + + th->base_block = prev_base_block; + + if (state) { + JUMP_TAG(state); } + + return iseqval; } VALUE rb_iseq_compile(VALUE src, VALUE file, VALUE line) { - return rb_iseq_compile_with_option(src, file, Qnil, line, Qnil); + return rb_iseq_compile_with_option(src, file, Qnil, line, 0, Qnil); +} + +VALUE +rb_iseq_compile_on_base(VALUE src, VALUE file, VALUE line, rb_block_t *base_block) +{ + return rb_iseq_compile_with_option(src, file, Qnil, line, base_block, Qnil); } static VALUE @@ -603,7 +629,7 @@ iseq_s_compile(int argc, VALUE *argv, VALUE self) if (NIL_P(file)) file = rb_str_new2(""); if (NIL_P(line)) line = INT2FIX(1); - return rb_iseq_compile_with_option(src, file, path, line, opt); + return rb_iseq_compile_with_option(src, file, path, line, 0, opt); } static VALUE diff --git a/ruby.c b/ruby.c index 3fd0f75ada..0e43503f3c 100644 --- a/ruby.c +++ b/ruby.c @@ -479,9 +479,7 @@ require_libraries(VALUE *req_list) VALUE self = rb_vm_top_self(); ID require; rb_thread_t *th = GET_THREAD(); - rb_block_t *prev_base_block = th->base_block; int prev_parse_in_eval = th->parse_in_eval; - th->base_block = 0; th->parse_in_eval = 0; Init_ext(); /* should be called here for some reason :-( */ @@ -493,7 +491,6 @@ require_libraries(VALUE *req_list) *req_list = 0; th->parse_in_eval = prev_parse_in_eval; - th->base_block = prev_base_block; } static rb_env_t* diff --git a/tool/compile_prelude.rb b/tool/compile_prelude.rb index 6ad9fce7da..1047e418b5 100755 --- a/tool/compile_prelude.rb +++ b/tool/compile_prelude.rb @@ -119,7 +119,7 @@ prelude_prefix_path(VALUE self) static void prelude_eval(VALUE code, VALUE name, VALUE line) { - rb_iseq_eval(rb_iseq_compile_with_option(code, name, Qnil, line, Qtrue)); + rb_iseq_eval(rb_iseq_compile_with_option(code, name, Qnil, line, 0, Qtrue)); } % end diff --git a/vm.c b/vm.c index 075219236a..06d2db3ddd 100644 --- a/vm.c +++ b/vm.c @@ -141,15 +141,14 @@ vm_set_top_stack(rb_thread_t * th, VALUE iseqval) } static void -vm_set_eval_stack(rb_thread_t * th, VALUE iseqval, const NODE *cref) +vm_set_eval_stack(rb_thread_t * th, VALUE iseqval, const NODE *cref, rb_block_t *base_block) { rb_iseq_t *iseq; - rb_block_t * const block = th->base_block; GetISeqPtr(iseqval, iseq); CHECK_STACK_OVERFLOW(th->cfp, iseq->local_size + iseq->stack_max); - vm_push_frame(th, iseq, VM_FRAME_MAGIC_EVAL | VM_FRAME_FLAG_FINISH, block->self, - VM_ENVVAL_PREV_EP_PTR(block->ep), iseq->iseq_encoded, + vm_push_frame(th, iseq, VM_FRAME_MAGIC_EVAL | VM_FRAME_FLAG_FINISH, base_block->self, + VM_ENVVAL_PREV_EP_PTR(base_block->ep), iseq->iseq_encoded, th->cfp->sp, iseq->local_size); if (cref) { @@ -167,9 +166,7 @@ vm_set_main_stack(rb_thread_t *th, VALUE iseqval) GetBindingPtr(toplevel_binding, bind); GetEnvPtr(bind->env, env); - th->base_block = &env->block; - vm_set_eval_stack(th, iseqval, 0); - th->base_block = 0; + vm_set_eval_stack(th, iseqval, 0, &env->block); /* save binding */ GetISeqPtr(iseqval, iseq); diff --git a/vm_core.h b/vm_core.h index c0c9ac2ba4..ef13b2f4ee 100644 --- a/vm_core.h +++ b/vm_core.h @@ -520,13 +520,19 @@ typedef struct rb_thread_struct { #if defined __GNUC__ && __GNUC__ >= 4 #pragma GCC visibility push(default) #endif + +/* node -> iseq */ VALUE rb_iseq_new(NODE*, VALUE, VALUE, VALUE, VALUE, enum iseq_type); VALUE rb_iseq_new_top(NODE *node, VALUE name, VALUE path, VALUE absolute_path, VALUE parent); VALUE rb_iseq_new_main(NODE *node, VALUE path, VALUE absolute_path); VALUE rb_iseq_new_with_bopt(NODE*, VALUE, VALUE, VALUE, VALUE, VALUE, enum iseq_type, VALUE); VALUE rb_iseq_new_with_opt(NODE*, VALUE, VALUE, VALUE, VALUE, VALUE, enum iseq_type, const rb_compile_option_t*); + +/* src -> iseq */ VALUE rb_iseq_compile(VALUE src, VALUE file, VALUE line); -VALUE rb_iseq_compile_with_option(VALUE src, VALUE path, VALUE absolute_path, VALUE line, VALUE opt); +VALUE rb_iseq_compile_on_base(VALUE src, VALUE file, VALUE line, rb_block_t *base_block); +VALUE rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE absolute_path, VALUE line, rb_block_t *base_block, VALUE opt); + VALUE rb_iseq_disasm(VALUE self); int rb_iseq_disasm_insn(VALUE str, VALUE *iseqval, size_t pos, rb_iseq_t *iseq, VALUE child); const char *ruby_node_name(int node); diff --git a/vm_eval.c b/vm_eval.c index 457143c2bc..7ec768f7f6 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -16,7 +16,7 @@ static inline VALUE vm_yield_with_cref(rb_thread_t *th, int argc, const VALUE *a static inline VALUE vm_yield(rb_thread_t *th, int argc, const VALUE *argv); static NODE *vm_cref_push(rb_thread_t *th, VALUE klass, int noex, rb_block_t *blockptr); static VALUE vm_exec(rb_thread_t *th); -static void vm_set_eval_stack(rb_thread_t * th, VALUE iseqval, const NODE *cref); +static void vm_set_eval_stack(rb_thread_t * th, VALUE iseqval, const NODE *cref, rb_block_t *base_block); static int vm_collect_local_variables_in_heap(rb_thread_t *th, VALUE *dfp, VALUE ary); /* vm_backtrace.c */ @@ -996,7 +996,7 @@ eval_string_with_cref(VALUE self, VALUE src, VALUE scope, NODE *cref, const char rb_binding_t *bind = 0; rb_thread_t *th = GET_THREAD(); rb_env_t *env = NULL; - rb_block_t block; + rb_block_t block, *base_block; volatile int parse_in_eval; volatile int mild_compile_error; @@ -1027,16 +1027,16 @@ eval_string_with_cref(VALUE self, VALUE src, VALUE scope, NODE *cref, const char rb_obj_classname(scope)); } GetEnvPtr(envval, env); - th->base_block = &env->block; + base_block = &env->block; } else { rb_control_frame_t *cfp = rb_vm_get_ruby_level_next_cfp(th, th->cfp); if (cfp != 0) { block = *RUBY_VM_GET_BLOCK_PTR_IN_CFP(cfp); - th->base_block = █ - th->base_block->self = self; - th->base_block->iseq = cfp->iseq; /* TODO */ + base_block = █ + base_block->self = self; + base_block->iseq = cfp->iseq; /* TODO */ } else { rb_raise(rb_eRuntimeError, "Can't eval on top of Fiber or Thread"); @@ -1046,12 +1046,11 @@ eval_string_with_cref(VALUE self, VALUE src, VALUE scope, NODE *cref, const char /* make eval iseq */ th->parse_in_eval++; th->mild_compile_error++; - iseqval = rb_iseq_compile(src, rb_str_new2(file), INT2FIX(line)); + iseqval = rb_iseq_compile_on_base(src, rb_str_new2(file), INT2FIX(line), base_block); th->mild_compile_error--; th->parse_in_eval--; - vm_set_eval_stack(th, iseqval, cref); - th->base_block = 0; + vm_set_eval_stack(th, iseqval, cref, base_block); if (0) { /* for debug */ VALUE disasm = rb_iseq_disasm(iseqval);