[前][次][番号順一覧][スレッド一覧]

ruby-changes:37436

From: nobu <ko1@a...>
Date: Fri, 6 Feb 2015 10:32:07 +0900 (JST)
Subject: [ruby-changes:37436] nobu:r49517 (trunk): vm_core.h: fix symbols leak

nobu	2015-02-06 10:31:53 +0900 (Fri, 06 Feb 2015)

  New Revision: 49517

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=49517

  Log:
    vm_core.h: fix symbols leak
    
    * vm_core.h (rb_call_info_kw_arg_struct): make keywords a symbols
      list to get rid of inadvertent creation by variable keyword
      arguments.  [ruby-core:68031] [Bug #10831]

  Modified files:
    trunk/ChangeLog
    trunk/compile.c
    trunk/test/-ext-/symbol/test_inadvertent_creation.rb
    trunk/vm_args.c
    trunk/vm_core.h
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 49516)
+++ ChangeLog	(revision 49517)
@@ -1,3 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Fri Feb  6 10:31:50 2015  Nobuyoshi Nakada  <nobu@r...>
+
+	* vm_core.h (rb_call_info_kw_arg_struct): make keywords a symbols
+	  list to get rid of inadvertent creation by variable keyword
+	  arguments.  [ruby-core:68031] [Bug #10831]
+
 Thu Feb  5 22:42:34 2015  SHIBATA Hiroshi  <shibata.hiroshi@g...>
 
 	* lib/rubygems:  Update to RubyGems HEAD(5c3b6f3).
Index: vm_core.h
===================================================================
--- vm_core.h	(revision 49516)
+++ vm_core.h	(revision 49517)
@@ -137,7 +137,7 @@ struct rb_control_frame_struct; https://github.com/ruby/ruby/blob/trunk/vm_core.h#L137
 
 typedef struct rb_call_info_kw_arg_struct {
     int keyword_len;
-    ID keywords[1];
+    VALUE keywords[1];
 } rb_call_info_kw_arg_t;
 
 /* rb_call_info_t contains calling information including inline cache */
Index: compile.c
===================================================================
--- compile.c	(revision 49516)
+++ compile.c	(revision 49517)
@@ -2409,7 +2409,7 @@ compile_array_keyword_arg(rb_iseq_t *ise https://github.com/ruby/ruby/blob/trunk/compile.c#L2409
 	{
 	    int len = (int)node->nd_alen / 2;
 	    rb_call_info_kw_arg_t *kw_arg  = (rb_call_info_kw_arg_t *)ruby_xmalloc(sizeof(rb_call_info_kw_arg_t) + sizeof(VALUE) * (len - 1));
-	    ID *keywords = kw_arg->keywords;
+	    VALUE *keywords = kw_arg->keywords;
 	    int i = 0;
 	    kw_arg->keyword_len = len;
 
@@ -2418,7 +2418,7 @@ compile_array_keyword_arg(rb_iseq_t *ise https://github.com/ruby/ruby/blob/trunk/compile.c#L2418
 	    for (i=0; node != NULL; i++, node = node->nd_next->nd_next) {
 		NODE *key_node = node->nd_head;
 		NODE *val_node = node->nd_next->nd_head;
-		keywords[i] = SYM2ID(key_node->nd_lit);
+		keywords[i] = key_node->nd_lit;
 		COMPILE(ret, "keyword values", val_node);
 	    }
 	    assert(i == len);
@@ -5844,12 +5844,14 @@ iseq_build_callinfo_from_hash(rb_iseq_t https://github.com/ruby/ruby/blob/trunk/compile.c#L5844
 	if (!NIL_P(vkw_arg)) {
 	    int i;
 	    int len = RARRAY_LENINT(vkw_arg);
-	    size_t n = sizeof(rb_call_info_kw_arg_t) + sizeof(ID) * (len - 1);
+	    size_t n = sizeof(rb_call_info_kw_arg_t) + sizeof(VALUE) * (len - 1);
 
 	    kw_arg = xmalloc(n);
 	    kw_arg->keyword_len = len;
 	    for (i = 0; i < len; i++) {
-		kw_arg->keywords[i] = SYM2ID(RARRAY_AREF(vkw_arg, i));
+		VALUE kw = RARRAY_AREF(vkw_arg, i);
+		SYM2ID(kw);	/* make immortal */
+		kw_arg->keywords[i] = kw;
 	    }
 	}
     }
Index: test/-ext-/symbol/test_inadvertent_creation.rb
===================================================================
--- test/-ext-/symbol/test_inadvertent_creation.rb	(revision 49516)
+++ test/-ext-/symbol/test_inadvertent_creation.rb	(revision 49517)
@@ -426,5 +426,26 @@ module Test_Symbol https://github.com/ruby/ruby/blob/trunk/test/-ext-/symbol/test_inadvertent_creation.rb#L426
         x.method(:send).call(name.to_sym)
       end
     end
+
+    def test_kwarg_symbol_leak_no_rest
+      foo = -> (arg: 42) {}
+      assert_no_immortal_symbol_created("kwarg no rest") do |name|
+        assert_raise(ArgumentError) { foo.call(name.to_sym => 42) }
+      end
+    end
+
+    def test_kwarg_symbol_leak_with_rest
+      foo = -> (arg: 2, **options) {}
+      assert_no_immortal_symbol_created("kwarg with rest") do |name|
+        foo.call(name.to_sym => 42)
+      end
+    end
+
+    def test_kwarg_symbol_leak_just_rest
+      foo = -> (**options) {}
+      assert_no_immortal_symbol_created("kwarg just rest") do |name|
+        foo.call(name.to_sym => 42)
+      end
+    end
   end
 end
Index: vm_args.c
===================================================================
--- vm_args.c	(revision 49516)
+++ vm_args.c	(revision 49517)
@@ -238,7 +238,7 @@ args_pop_keyword_hash(struct args_info * https://github.com/ruby/ruby/blob/trunk/vm_args.c#L238
 static int
 args_kw_argv_to_hash(struct args_info *args)
 {
-    const ID * const passed_keywords = args->ci->kw_arg->keywords;
+    const VALUE *const passed_keywords = args->ci->kw_arg->keywords;
     const int kw_len = args->ci->kw_arg->keyword_len;
     VALUE h = rb_hash_new();
     const int kw_start = args->argc - kw_len;
@@ -247,7 +247,7 @@ args_kw_argv_to_hash(struct args_info *a https://github.com/ruby/ruby/blob/trunk/vm_args.c#L247
 
     args->argc = kw_start + 1;
     for (i=0; i<kw_len; i++) {
-	rb_hash_aset(h, ID2SYM(passed_keywords[i]), kw_argv[i]);
+	rb_hash_aset(h, passed_keywords[i], kw_argv[i]);
     }
 
     args->argv[args->argc - 1] = h;
@@ -260,11 +260,11 @@ args_stored_kw_argv_to_hash(struct args_ https://github.com/ruby/ruby/blob/trunk/vm_args.c#L260
 {
     VALUE h = rb_hash_new();
     int i;
-    const ID * const passed_keywords = args->ci->kw_arg->keywords;
+    const VALUE *const passed_keywords = args->ci->kw_arg->keywords;
     const int passed_keyword_len = args->ci->kw_arg->keyword_len;
 
     for (i=0; i<passed_keyword_len; i++) {
-	rb_hash_aset(h, ID2SYM(passed_keywords[i]), args->kw_argv[i]);
+	rb_hash_aset(h, passed_keywords[i], args->kw_argv[i]);
     }
     args->kw_argv = NULL;
 
@@ -348,7 +348,7 @@ args_setup_rest_parameter(struct args_in https://github.com/ruby/ruby/blob/trunk/vm_args.c#L348
 }
 
 static VALUE
-make_unused_kw_hash(const ID *passed_keywords, int passed_keyword_len, const VALUE *kw_argv, const int key_only)
+make_unused_kw_hash(const VALUE *passed_keywords, int passed_keyword_len, const VALUE *kw_argv, const int key_only)
 {
     int i;
     VALUE obj = key_only ? rb_ary_tmp_new(1) : rb_hash_new();
@@ -356,10 +356,10 @@ make_unused_kw_hash(const ID *passed_key https://github.com/ruby/ruby/blob/trunk/vm_args.c#L356
     for (i=0; i<passed_keyword_len; i++) {
 	if (kw_argv[i] != Qundef) {
 	    if (key_only) {
-		rb_ary_push(obj, ID2SYM(passed_keywords[i]));
+		rb_ary_push(obj, passed_keywords[i]);
 	    }
 	    else {
-		rb_hash_aset(obj, ID2SYM(passed_keywords[i]), kw_argv[i]);
+		rb_hash_aset(obj, passed_keywords[i], kw_argv[i]);
 	    }
 	}
     }
@@ -367,12 +367,13 @@ make_unused_kw_hash(const ID *passed_key https://github.com/ruby/ruby/blob/trunk/vm_args.c#L367
 }
 
 static inline int
-args_setup_kw_parameters_lookup(const ID key, VALUE *ptr, const ID * const passed_keywords, VALUE *passed_values, const int passed_keyword_len)
+args_setup_kw_parameters_lookup(const ID key, VALUE *ptr, const VALUE *const passed_keywords, VALUE *passed_values, const int passed_keyword_len)
 {
     int i;
+    const VALUE keyname = ID2SYM(key);
 
     for (i=0; i<passed_keyword_len; i++) {
-	if (key == passed_keywords[i]) {
+	if (keyname == passed_keywords[i]) {
 	    *ptr = passed_values[i];
 	    passed_values[i] = Qundef;
 	    return TRUE;
@@ -383,7 +384,7 @@ args_setup_kw_parameters_lookup(const ID https://github.com/ruby/ruby/blob/trunk/vm_args.c#L384
 }
 
 static void
-args_setup_kw_parameters(VALUE* const passed_values, const int passed_keyword_len, const ID * const passed_keywords,
+args_setup_kw_parameters(VALUE* const passed_values, const int passed_keyword_len, const VALUE *const passed_keywords,
 			 const rb_iseq_t * const iseq, VALUE * const locals)
 {
     const ID *acceptable_keywords = iseq->param.keyword->table;
@@ -495,7 +496,7 @@ fill_keys_values(st_data_t key, st_data_ https://github.com/ruby/ruby/blob/trunk/vm_args.c#L496
 {
     struct fill_values_arg *arg = (struct fill_values_arg *)ptr;
     int i = arg->argc++;
-    arg->keys[i] = SYM2ID((VALUE)key);
+    arg->keys[i] = (VALUE)key;
     arg->vals[i] = (VALUE)val;
     return ST_CONTINUE;
 }
@@ -722,14 +723,14 @@ vm_caller_setup_arg_splat(rb_control_fra https://github.com/ruby/ruby/blob/trunk/vm_args.c#L723
 static inline void
 vm_caller_setup_arg_kw(rb_control_frame_t *cfp, rb_call_info_t *ci)
 {
-    const ID * const passed_keywords = ci->kw_arg->keywords;
+    const VALUE *const passed_keywords = ci->kw_arg->keywords;
     const int kw_len = ci->kw_arg->keyword_len;
     const VALUE h = rb_hash_new();
     VALUE *sp = cfp->sp;
     int i;
 
     for (i=0; i<kw_len; i++) {
-	rb_hash_aset(h, ID2SYM(passed_keywords[i]), (sp - kw_len)[i]);
+	rb_hash_aset(h, passed_keywords[i], (sp - kw_len)[i]);
     }
     (sp-kw_len)[0] = h;
 

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]