nobu	2014-04-30 17:06:51 +0900 (Wed, 30 Apr 2014)

  New Revision: 45756

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

  Log:
    parse.y: pin down dynamic symbol only
    
    * parse.y (rb_id_attrset): pin down dynamic symbol only. it is
      possibe that attrset ID can be registered as a static symbol
      after the corresponding attrget ID has been registered as a
      dynamic, and then the latter may be collected.
      [ruby-core:62226] [Bug #9787]

  Modified files:
    trunk/ChangeLog
    trunk/parse.y
    trunk/test/ruby/test_symbol.rb
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 45755)
+++ ChangeLog	(revision 45756)
@@ -1,3 +1,11 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Apr 30 17:06:49 2014  Nobuyoshi Nakada  <nobu@r...>
+
+	* parse.y (rb_id_attrset): pin down dynamic symbol only. it is
+	  possibe that attrset ID can be registered as a static symbol
+	  after the corresponding attrget ID has been registered as a
+	  dynamic, and then the latter may be collected.
+	  [ruby-core:62226] [Bug #9787]
+
 Tue Apr 29 14:17:57 2014  Tanaka Akira  <akr@f...>
 
 	* lib/tmpdir.rb: Rescue LoadError on etc.so for miniruby.
Index: parse.y
===================================================================
--- parse.y	(revision 45755)
+++ parse.y	(revision 45756)
@@ -46,6 +46,7 @@ static ID register_static_symid_str(ID, https://github.com/ruby/ruby/blob/trunk/parse.y#L46
 #define REGISTER_SYMID(id, name) register_static_symid((id), (name), strlen(name), enc)
 #include "id.c"
 #endif
+#define ID_DYNAMIC_SYM_P(id) (!(id&ID_STATIC_SYM)&&id>tLAST_TOKEN)
 
 static inline int id_type(ID);
 #define is_notop_id(id) ((id)>tLAST_OP_ID)
@@ -8863,7 +8864,11 @@ rb_id_attrset(ID id) https://github.com/ruby/ruby/blob/trunk/parse.y#L8864
 	str = rb_str_dup(RSYMBOL((VALUE)id)->fstr);
 	rb_str_cat(str, "=", 1);
 	id = (ID)rb_str_dynamic_intern(str);
-	rb_pin_dynamic_symbol((VALUE)id);
+	if (ID_DYNAMIC_SYM_P(id)) {
+	    /* attrset ID may have been registered as a static
+	     * symbol */
+	    rb_pin_dynamic_symbol((VALUE)id);
+	}
     }
     return id;
 }
@@ -10450,6 +10455,15 @@ sym_check_asciionly(VALUE str) https://github.com/ruby/ruby/blob/trunk/parse.y#L10455
  */
 static ID intern_str(VALUE str);
 
+static void
+must_be_dynamic_symbol(VALUE x)
+{
+    if (SPECIAL_CONST_P(x) || BUILTIN_TYPE(x) != T_SYMBOL) {
+	rb_raise(rb_eTypeError, "wrong argument type %s (expected Symbol)",
+		 rb_builtin_class_name(x));
+    }
+}
+
 static VALUE
 setup_fake_str(struct RString *fake_str, const char *name, long len)
 {
@@ -10461,11 +10475,10 @@ setup_fake_str(struct RString *fake_str, https://github.com/ruby/ruby/blob/trunk/parse.y#L10475
     return (VALUE)fake_str;
 }
 
-#define ID_DYNAMIC_SYM_P(id) (!(id&ID_STATIC_SYM)&&id>tLAST_TOKEN)
-
 ID
 rb_pin_dynamic_symbol(VALUE sym)
 {
+    must_be_dynamic_symbol(sym);
     rb_gc_resurrect(sym);
     /* stick dynamic symbol */
     if (!st_insert(global_symbols.pinned_dsym, sym, (st_data_t)sym)) {
@@ -10770,15 +10783,6 @@ lookup_id_str(ID id, st_data_t *data) https://github.com/ruby/ruby/blob/trunk/parse.y#L10783
     return FALSE;
 }
 
-static void
-must_be_dynamic_symbol(VALUE x)
-{
-    if (SPECIAL_CONST_P(x) || BUILTIN_TYPE(x) != T_SYMBOL) {
-	rb_raise(rb_eTypeError, "wrong argument type %s (expected Symbol)",
-		 rb_builtin_class_name(x));
-    }
-}
-
 ID
 rb_sym2id(VALUE x)
 {
@@ -10786,7 +10790,6 @@ rb_sym2id(VALUE x) https://github.com/ruby/ruby/blob/trunk/parse.y#L10790
 	return RSHIFT((unsigned long)(x),RUBY_SPECIAL_SHIFT);
     }
     else {
-	must_be_dynamic_symbol(x);
 	return rb_pin_dynamic_symbol(x);
     }
 }
Index: test/ruby/test_symbol.rb
===================================================================
--- test/ruby/test_symbol.rb	(revision 45755)
+++ test/ruby/test_symbol.rb	(revision 45756)
@@ -237,4 +237,22 @@ class TestSymbol < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_symbol.rb#L237
                        '',
                        child_env: '--disable-gems')
   end
+
+  def test_gc_attrset
+    bug9787 = '[ruby-core:62226] [Bug #9787]'
+    assert_normal_exit(<<-'end;', '', child_env: '--disable-gems')
+      def noninterned_name(prefix = "")
+        prefix += "_#{Thread.current.object_id.to_s(36).tr('-', '_')}"
+        begin
+          name = "#{prefix}_#{rand(0x1000).to_s(16)}_#{Time.now.usec}"
+        end while Symbol.find(name) or Symbol.find(name + "=")
+        name
+      end
+      n = noninterned_name("gc")
+      n.to_sym
+      GC.start(immediate_sweep: false)
+      eval(":#{n}=")
+      eval("proc{self.#{n} = nil}")
+    end;
+  end
 end

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