From 633574eee5ed28367d868bbe6391b3e8f7bba08f Mon Sep 17 00:00:00 2001 From: ko1 Date: Mon, 6 Apr 2015 07:14:28 +0000 Subject: * vm_args.c: protect value stack from calling other methods during complex parameter setting process (splat, kw, and so on). [Bug #11027] * vm_core.h: remove rb_thead_t::mark_stack_len. With this modification, we don't need to use th->mark_stack_len. * test/ruby/test_keyword.rb: add a test. * cont.c (cont_capture): catch up this fix. * vm.c (rb_thread_mark): ditto. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@50172 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 15 +++++++++++++++ cont.c | 2 +- test/ruby/test_keyword.rb | 10 ++++++++++ vm.c | 1 - vm_args.c | 44 +++++++++++++++++++++++++++++--------------- vm_core.h | 1 - 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4c6253d086..7bcb70b884 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +Mon Apr 6 16:09:58 2015 Koichi Sasada + + * vm_args.c: protect value stack from calling other methods + during complex parameter setting process (splat, kw, and so on). + [Bug #11027] + + * vm_core.h: remove rb_thead_t::mark_stack_len. + With this modification, we don't need to use th->mark_stack_len. + + * test/ruby/test_keyword.rb: add a test. + + * cont.c (cont_capture): catch up this fix. + + * vm.c (rb_thread_mark): ditto. + Mon Apr 6 11:26:42 2015 NARUSE, Yui * tool/downloader.rb (http_options): prevent content auto decoding diff --git a/cont.c b/cont.c index 78ae0892b0..22e0c5abe0 100644 --- a/cont.c +++ b/cont.c @@ -490,7 +490,7 @@ cont_capture(volatile int *stat) contval = cont->self; #ifdef CAPTURE_JUST_VALID_VM_STACK - cont->vm_stack_slen = th->cfp->sp + th->mark_stack_len - th->stack; + cont->vm_stack_slen = th->cfp->sp - th->stack; cont->vm_stack_clen = th->stack + th->stack_size - (VALUE*)th->cfp; cont->vm_stack = ALLOC_N(VALUE, cont->vm_stack_slen + cont->vm_stack_clen); MEMCPY(cont->vm_stack, th->stack, VALUE, cont->vm_stack_slen); diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 70cdba1db2..9c76e15c38 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -566,4 +566,14 @@ class TestKeywordArguments < Test::Unit::TestCase result = m(["a" => 10]) { |a = nil, **b| [a, b] } assert_equal([{"a" => 10}, {}], result) end + + def method_for_test_to_hash_call_during_setup_complex_parameters k1:, k2:, **rest_kw + [k1, k2, rest_kw] + end + + def test_to_hash_call_during_setup_complex_parameters + sym = "sym_#{Time.now}".to_sym + h = method_for_test_to_hash_call_during_setup_complex_parameters k1: "foo", k2: "bar", sym => "baz" + assert_equal ["foo", "bar", {sym => "baz"}], h, '[Bug #11027]' + end end diff --git a/vm.c b/vm.c index 4b6e0c7225..15ec57207e 100644 --- a/vm.c +++ b/vm.c @@ -2048,7 +2048,6 @@ rb_thread_mark(void *ptr) rb_control_frame_t *limit_cfp = (void *)(th->stack + th->stack_size); rb_gc_mark_values((long)(sp - p), p); - rb_gc_mark_locations(sp, sp + th->mark_stack_len); while (cfp != limit_cfp) { rb_iseq_t *iseq = cfp->iseq; diff --git a/vm_args.c b/vm_args.c index e4ade6bb2f..2f5c7c986e 100644 --- a/vm_args.c +++ b/vm_args.c @@ -83,19 +83,17 @@ args_reduce(struct args_info *args, int over_argc) } static inline int -args_check_block_arg0(struct args_info *args, rb_thread_t *th, const int msl) +args_check_block_arg0(struct args_info *args, rb_thread_t *th) { VALUE ary = Qnil; if (args->rest && RARRAY_LEN(args->rest) == 1) { VALUE arg0 = RARRAY_AREF(args->rest, 0); ary = rb_check_array_type(arg0); - th->mark_stack_len = msl; } else if (args->argc == 1) { VALUE arg0 = args->argv[0]; ary = rb_check_array_type(arg0); - th->mark_stack_len = msl; args->argv[0] = arg0; /* see: https://bugs.ruby-lang.org/issues/8484 */ } @@ -173,10 +171,9 @@ args_rest_array(struct args_info *args) } static int -keyword_hash_p(VALUE *kw_hash_ptr, VALUE *rest_hash_ptr, rb_thread_t *th, const int msl) +keyword_hash_p(VALUE *kw_hash_ptr, VALUE *rest_hash_ptr, rb_thread_t *th) { *rest_hash_ptr = rb_check_hash_type(*kw_hash_ptr); - th->mark_stack_len = msl; if (!NIL_P(*rest_hash_ptr)) { VALUE hash = rb_extract_keywords(rest_hash_ptr); @@ -191,7 +188,7 @@ keyword_hash_p(VALUE *kw_hash_ptr, VALUE *rest_hash_ptr, rb_thread_t *th, const } static VALUE -args_pop_keyword_hash(struct args_info *args, VALUE *kw_hash_ptr, rb_thread_t *th, const int msl) +args_pop_keyword_hash(struct args_info *args, VALUE *kw_hash_ptr, rb_thread_t *th) { VALUE rest_hash; @@ -200,7 +197,7 @@ args_pop_keyword_hash(struct args_info *args, VALUE *kw_hash_ptr, rb_thread_t *t assert(args->argc > 0); *kw_hash_ptr = args->argv[args->argc-1]; - if (keyword_hash_p(kw_hash_ptr, &rest_hash, th, msl)) { + if (keyword_hash_p(kw_hash_ptr, &rest_hash, th)) { if (rest_hash) { args->argv[args->argc-1] = rest_hash; } @@ -216,7 +213,7 @@ args_pop_keyword_hash(struct args_info *args, VALUE *kw_hash_ptr, rb_thread_t *t if (len > 0) { *kw_hash_ptr = RARRAY_AREF(args->rest, len - 1); - if (keyword_hash_p(kw_hash_ptr, &rest_hash, th, msl)) { + if (keyword_hash_p(kw_hash_ptr, &rest_hash, th)) { if (rest_hash) { RARRAY_ASET(args->rest, len - 1, rest_hash); } @@ -511,9 +508,27 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r int given_argc; struct args_info args_body, *args; VALUE keyword_hash = Qnil; - const int msl = ci->argc + iseq->param.size; + VALUE * const orig_sp = th->cfp->sp; + int i; - th->mark_stack_len = msl; + /* + * Extend SP for GC. + * + * [pushed values] [uninitialized values] + * <- ci->argc --> + * <- iseq->param.size------------------> + * ^ locals ^ sp + * + * => + * [pushed values] [initialized values ] + * <- ci->argc --> + * <- iseq->param.size------------------> + * ^ locals ^ sp + */ + for (i=ci->argc; iparam.size; i++) { + locals[i] = Qnil; + } + th->cfp->sp = &locals[i]; /* setup args */ args = &args_body; @@ -556,7 +571,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r (min_argc > 0 || iseq->param.opt_num > 1 || iseq->param.flags.has_kw || iseq->param.flags.has_kwrest) && !iseq->param.flags.ambiguous_param0 && - args_check_block_arg0(args, th, msl)) { + args_check_block_arg0(args, th)) { given_argc = RARRAY_LENINT(args->rest); } break; @@ -564,7 +579,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r if (given_argc == 1 && given_argc != iseq->param.lead_num && !iseq->param.flags.has_rest && - args_check_block_arg0(args, th, msl)) { + args_check_block_arg0(args, th)) { given_argc = RARRAY_LENINT(args->rest); } } @@ -590,7 +605,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r if (given_argc > min_argc && (iseq->param.flags.has_kw || iseq->param.flags.has_kwrest) && args->kw_argv == NULL) { - if (args_pop_keyword_hash(args, &keyword_hash, th, msl)) { + if (args_pop_keyword_hash(args, &keyword_hash, th)) { given_argc--; } } @@ -662,8 +677,7 @@ setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, r } #endif - th->mark_stack_len = 0; - + th->cfp->sp = orig_sp; return opt_pc; } diff --git a/vm_core.h b/vm_core.h index 7c775d8019..97df64f729 100644 --- a/vm_core.h +++ b/vm_core.h @@ -631,7 +631,6 @@ typedef struct rb_thread_struct { enum rb_thread_status status; int to_kill; int priority; - int mark_stack_len; native_thread_data_t native_thread_data; void *blocking_region_buffer; -- cgit v1.2.3