Bug #10014: Do not expose no-pinned down IDs

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.2.0
• ruby -v: 2.2
• Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN

[ko1 (Koichi Sasada)]

概要

ID と Symbol の関係を整理し、よりバグの少ないコードへと変更する。

現状と問題点

Symbol GC が導入され、Symbol に dynamic symbol と static symbol に別れる
ことになった。

pindown されていない dynamic symbol は、GC 可能であり、ここでは mortal
symbol と呼ぶことにする。

逆に、static symbol および pindown された symbol を、ここでは immortal
symbol と呼ぶことにする。

現在、コードの各所に、「ID を取り出したいが、mortal symbol を immortal
symbol へ変換したくない」という用途がある。具体的には、「ID を key とし
ているデータに対し、与えられた引数 name から ID を取り出し、それと合致す
るか判定したい場合」である。もし name が対応するものが mortal symbol で
あった場合、この判定のためだけに immortal symbol と変換すると、GC するこ
とができず効率に問題が出るためである。

この目的を解決するために、rb_check_id_without_pindown() （および、類似の
関数）が提供されている。これは、mortal symbol から計算される ID を、
immortal symbol に変更せずに返す。

しかし、mortal symbol から計算された ID なので、これをどこかに保存し、GC
を経過すると、この ID が無くなってしまう危険がある。

現状は、中田さんが職人的に「この ID は保存しない」という用途でのみ使うよ
うにしているため、問題は起きていないが、うっかり pindown していない ID
をメソッド ID などに使って保存してしまい、GC されてしまう、という危険が
ある。

解決策

現状の

(a) ID があれば対応する Symbol がある
(b) Symbol があれば対応する ID がある

という原則を変更し、

(a') ID があれば対応する Immortal Symbol がある
(b') Immortal Symbol があれば対応する ID がある
(c') Mortal Symbol は、ID を持たない

と変更する。

「ID を key としているデータに対し、与えられた引数 name から ID を取り出
し、それと合致するか判定したい場合」という用途では、そもそも保存するよう
な ID は immortal symbol であるため、わざわざ mortal symbol に対応する
ID をふる必要がないためだからである。

留意点

いくつかのコードでは、原則 (a) を利用して、name から Symbol の存在確認を
する、というコードがある。つまり、rb_check_id() で ID が取得できれば、
Symbol がある、とするコードである。

このような場合は、新たに作成した rb_check_symbol(namep) を利用して、
Symbol の存在確認、および取り出しをするように変更する必要がある。

Ruby インタプリタに 2 箇所あったが、外部の拡張ライブラリにどの程度影響が
あるかは未調査である。

パッチ

Index: internal.h
===================================================================
--- internal.h	(revision 46756)
+++ internal.h	(working copy)
@@ -809,12 +809,12 @@ void rb_gc_mark_symbols(int full_mark);
 ID rb_make_internal_id(void);
 void rb_gc_free_dsymbol(VALUE);
 VALUE rb_str_dynamic_intern(VALUE);
-ID rb_check_id_without_pindown(VALUE *);
-ID rb_sym2id_without_pindown(VALUE);
+ID rb_id_attrget(ID id);
+
+VALUE rb_check_symbol(volatile VALUE *namep);
 #ifdef RUBY_ENCODING_H
-ID rb_check_id_cstr_without_pindown(const char *, long, rb_encoding *);
+VALUE rb_check_symbol_cstr(const char *ptr, long len, rb_encoding *enc);
 #endif
-ID rb_id_attrget(ID id);
 
 /* proc.c */
 VALUE rb_proc_location(VALUE self);
Index: load.c
===================================================================
--- load.c	(revision 46756)
+++ load.c	(working copy)
@@ -1108,7 +1108,7 @@ rb_mod_autoload(VALUE mod, VALUE sym, VA
 static VALUE
 rb_mod_autoload_p(VALUE mod, VALUE sym)
 {
-    ID id = rb_check_id_without_pindown(&sym);
+    ID id = rb_check_id(&sym);
     if (!id) {
 	return Qnil;
     }
Index: object.c
===================================================================
--- object.c	(revision 46756)
+++ object.c	(working copy)
@@ -2135,7 +2135,7 @@ rb_mod_const_get(int argc, VALUE *argv,
 
 	if (pbeg == p) goto wrong_name;
 
-	id = rb_check_id_cstr_without_pindown(pbeg, len = p-pbeg, enc);
+	id = rb_check_id_cstr(pbeg, len = p-pbeg, enc);
 	beglen = pbeg-path;
 
 	if (p < pend && p[0] == ':') {
@@ -2277,7 +2277,7 @@ rb_mod_const_defined(int argc, VALUE *ar
 
 	if (pbeg == p) goto wrong_name;
 
-	id = rb_check_id_cstr_without_pindown(pbeg, len = p-pbeg, enc);
+	id = rb_check_id_cstr(pbeg, len = p-pbeg, enc);
 	beglen = pbeg-path;
 
 	if (p < pend && p[0] == ':') {
@@ -2348,7 +2348,7 @@ rb_mod_const_defined(int argc, VALUE *ar
 static VALUE
 rb_obj_ivar_get(VALUE obj, VALUE iv)
 {
-    ID id = rb_check_id_without_pindown(&iv);
+    ID id = rb_check_id(&iv);
 
     if (!id) {
 	if (rb_is_instance_name(iv)) {
@@ -2419,7 +2419,7 @@ rb_obj_ivar_set(VALUE obj, VALUE iv, VAL
 static VALUE
 rb_obj_ivar_defined(VALUE obj, VALUE iv)
 {
-    ID id = rb_check_id_without_pindown(&iv);
+    ID id = rb_check_id(&iv);
 
     if (!id) {
 	if (rb_is_instance_name(iv)) {
@@ -2456,7 +2456,7 @@ rb_obj_ivar_defined(VALUE obj, VALUE iv)
 static VALUE
 rb_mod_cvar_get(VALUE obj, VALUE iv)
 {
-    ID id = rb_check_id_without_pindown(&iv);
+    ID id = rb_check_id(&iv);
 
     if (!id) {
 	if (rb_is_class_name(iv)) {
@@ -2522,7 +2522,7 @@ rb_mod_cvar_set(VALUE obj, VALUE iv, VAL
 static VALUE
 rb_mod_cvar_defined(VALUE obj, VALUE iv)
 {
-    ID id = rb_check_id_without_pindown(&iv);
+    ID id = rb_check_id(&iv);
 
     if (!id) {
 	if (rb_is_class_name(iv)) {
Index: parse.y
===================================================================
--- parse.y	(revision 46756)
+++ parse.y	(working copy)
@@ -51,7 +51,10 @@ static ID register_static_symid_str(ID,
 #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)
+#define STATIC_SYM2ID(sym) RSHIFT((unsigned long)(sym), RUBY_SPECIAL_SHIFT)
+#define STATIC_ID2SYM(id)  (((VALUE)(id)<<RUBY_SPECIAL_SHIFT)|SYMBOL_FLAG)
 
 static inline int id_type(ID);
 #define is_notop_id(id) ((id)>tLAST_OP_ID)
@@ -8831,7 +8834,6 @@ block_dup_check_gen(struct parser_params
     }
 }
 
-static ID rb_pin_dynamic_symbol(VALUE);
 static ID attrsetname_to_attr(VALUE name);
 static int lookup_id_str(ID id, st_data_t *data);
 
@@ -10481,7 +10483,7 @@ dsymbol_check(const VALUE sym)
 }
 
 static ID
-rb_pin_dynamic_symbol(VALUE sym)
+dsymbol_pindown(VALUE sym)
 {
     must_be_dynamic_symbol(sym);
 
@@ -10496,20 +10498,56 @@ rb_pin_dynamic_symbol(VALUE sym)
     return (ID)sym;
 }
 
-static int
-lookup_str_id(st_data_t str, st_data_t *data)
+static ID
+lookup_str_id(st_data_t str, st_data_t *id_datap)
 {
-    ID id;
+    if (st_lookup(global_symbols.str_id, str, id_datap)) {
+	const ID id = (ID)*id_datap;
 
-    if (!st_lookup(global_symbols.str_id, str, data)) {
+	if (ID_DYNAMIC_SYM_P(id) && !SYMBOL_PINNED_P(id)) {
+	    *id_datap = 0;
+	    return FALSE;
+	}
+	else {
+	    return TRUE;
+	}
+    }
+    else {
+	return FALSE;
+    }
+}
+
+static VALUE
+lookup_str_sym(const st_data_t str, st_data_t *sym_datap)
+{
+    if (st_lookup(global_symbols.str_id, str, sym_datap)) {
+	const ID id = *sym_datap;
+
+	if (ID_DYNAMIC_SYM_P(id)) {
+	    *sym_datap = dsymbol_check(id);
+	}
+	else {
+	    *sym_datap = STATIC_ID2SYM(id);
+	}
+	return TRUE;
+    }
+    else {
 	return FALSE;
     }
-    id = (ID)*data;
+}
+
+static int
+lookup_id_str(ID id, st_data_t *data)
+{
     if (ID_DYNAMIC_SYM_P(id)) {
-	*data = (st_data_t)rb_pin_dynamic_symbol((VALUE)id);
+	*data = RSYMBOL(id)->fstr;
+	return TRUE;
     }
+    if (st_lookup(global_symbols.id_str, id, data)) {
     return TRUE;
 }
+    return FALSE;
+}
 
 static ID
 intern_cstr_without_pindown(const char *name, long len, rb_encoding *enc)
@@ -10535,7 +10573,7 @@ rb_intern3(const char *name, long len, r
 
     id = intern_cstr_without_pindown(name, len, enc);
     if (ID_DYNAMIC_SYM_P(id)) {
-	id = rb_pin_dynamic_symbol((VALUE)id);
+	id = dsymbol_pindown((VALUE)id);
     }
 
     return id;
@@ -10684,10 +10722,12 @@ rb_intern(const char *name)
 ID
 rb_intern_str(VALUE str)
 {
-    st_data_t id;
+    st_data_t sym;
+
+    if (lookup_str_sym(str, &sym)) {
+	return SYM2ID(sym);
+    }
 
-    if (lookup_str_id(str, &id))
-	return (ID)id;
     return intern_str(rb_str_dup(str));
 }
 
@@ -10733,15 +10773,9 @@ rb_str_dynamic_intern(VALUE str)
 {
 #if USE_SYMBOL_GC
     rb_encoding *enc, *ascii;
-    ID id;
+    VALUE sym;
 
-    if (st_lookup(global_symbols.str_id, str, &id)) {
-	VALUE sym = ID2SYM(id);
-	if (ID_DYNAMIC_SYM_P(id)) {
-	    /* because of lazy sweep, dynamic symbol may be unmarked already and swept
-	     * at next time */
-	    sym = dsymbol_check(sym);
-	}
+    if (lookup_str_sym(str, &sym)) {
 	return sym;
     }
 
@@ -10762,40 +10796,17 @@ rb_str_dynamic_intern(VALUE str)
 #endif
 }
 
-static int
-lookup_id_str(ID id, st_data_t *data)
-{
-    if (ID_DYNAMIC_SYM_P(id)) {
-	id = (ID)dsymbol_check((VALUE)id);
-	*data = RSYMBOL(id)->fstr;
-	return TRUE;
-    }
-    if (st_lookup(global_symbols.id_str, id, data)) {
-	return TRUE;
-    }
-    return FALSE;
-}
-
 ID
-rb_sym2id(VALUE x)
+rb_sym2id(VALUE sym)
 {
-    if (STATIC_SYM_P(x)) {
-	return RSHIFT((unsigned long)(x),RUBY_SPECIAL_SHIFT);
+    if (STATIC_SYM_P(sym)) {
+	return STATIC_SYM2ID(sym);
     }
     else {
-	return rb_pin_dynamic_symbol(x);
+	if (!SYMBOL_PINNED_P(sym)) {
+	    dsymbol_pindown(sym);
     }
-}
-
-ID
-rb_sym2id_without_pindown(VALUE x)
-{
-    if (STATIC_SYM_P(x)) {
-	return RSHIFT((unsigned long)(x),RUBY_SPECIAL_SHIFT);
-    }
-    else {
-	must_be_dynamic_symbol(x);
-	return (ID)x;
+	return (ID)sym;
     }
 }
 
@@ -10803,7 +10814,7 @@ VALUE
 rb_id2sym(ID x)
 {
     if (!ID_DYNAMIC_SYM_P(x)) {
-	return ((VALUE)(x)<<RUBY_SPECIAL_SHIFT)|SYMBOL_FLAG;
+	return STATIC_ID2SYM(x);
     }
     else {
 	return (VALUE)x;
@@ -10814,9 +10825,13 @@ rb_id2sym(ID x)
 VALUE
 rb_sym2str(VALUE sym)
 {
-    return rb_id2str(rb_sym2id_without_pindown(sym));
+    if (DYNAMIC_SYM_P(sym)) {
+	return RSYMBOL(sym)->fstr;
+    }
+    else {
+	return rb_id2str(STATIC_SYM2ID(sym));
+    }
 }
-
 
 VALUE
 rb_id2str(ID id)
@@ -10862,7 +10877,7 @@ rb_id2str(ID id)
 	str = rb_str_dup(str);
 	rb_str_cat(str, "=", 1);
 	register_static_symid_str(id, str);
-	if (st_lookup(global_symbols.id_str, id, &data)) {
+	if (lookup_id_str(id, &data)) {
             VALUE str = (VALUE)data;
             if (RBASIC(str)->klass == 0)
                 RBASIC_SET_CLASS_RAW(str, rb_cString);
@@ -10890,10 +10905,15 @@ rb_make_internal_id(void)
 static int
 symbols_i(VALUE key, ID value, VALUE ary)
 {
-    VALUE sym = ID2SYM(value);
+    VALUE sym;
+
     if (ID_DYNAMIC_SYM_P(value)) {
-	sym = dsymbol_check(sym);
+	sym = (VALUE)value; /* sym = dsymbol_check((VALUE)value); rb_gc_start() prevent such situation */
     }
+    else {
+	sym = STATIC_ID2SYM(value);
+    }
+
     rb_ary_push(ary, sym);
     return ST_CONTINUE;
 }
@@ -10918,7 +10938,7 @@ VALUE
 rb_sym_all_symbols(void)
 {
     VALUE ary = rb_ary_new2(global_symbols.str_id->num_entries);
-
+    rb_gc_start();
     st_foreach(global_symbols.str_id, symbols_i, ary);
     return ary;
 }
@@ -10979,38 +10999,56 @@ rb_is_junk_id(ID id)
 ID
 rb_check_id(volatile VALUE *namep)
 {
-    ID id;
+    st_data_t id;
+    VALUE tmp;
+    VALUE name = *namep;
 
-    id = rb_check_id_without_pindown((VALUE *)namep);
-    if (ID_DYNAMIC_SYM_P(id)) {
-	id = rb_pin_dynamic_symbol((VALUE)id);
+    if (STATIC_SYM_P(name)) {
+	return STATIC_SYM2ID(name);
+    }
+    else if (DYNAMIC_SYM_P(name)) {
+	if (SYMBOL_PINNED_P(name)) {
+	    return (ID)name;
     }
+	else {
+	    *namep = RSYMBOL(name)->fstr;
+	    return 0;
+	}
+    }
+    else if (!RB_TYPE_P(name, T_STRING)) {
+	tmp = rb_check_string_type(name);
+	if (NIL_P(tmp)) {
+	    tmp = rb_inspect(name);
+	    rb_raise(rb_eTypeError, "%s is not a symbol nor a string",
+		     RSTRING_PTR(tmp));
+	}
+	name = tmp;
+	*namep = name;
+    }
+
+    sym_check_asciionly(name);
 
+    if (lookup_str_id(name, &id)) {
     return id;
 }
 
-ID
-rb_check_id_cstr(const char *ptr, long len, rb_encoding *enc)
 {
-    ID id;
-
-    id = rb_check_id_cstr_without_pindown(ptr, len, enc);
-    if (ID_DYNAMIC_SYM_P(id)) {
-	id = rb_pin_dynamic_symbol((VALUE)id);
+	ID gid = attrsetname_to_attr(name);
+	if (gid) return rb_id_attrset(gid);
     }
 
-    return id;
+    return (ID)0;
 }
 
-ID
-rb_check_id_without_pindown(VALUE *namep)
+VALUE
+rb_check_symbol(volatile VALUE *namep)
 {
-    st_data_t id;
+    st_data_t sym;
     VALUE tmp;
     VALUE name = *namep;
 
     if (SYMBOL_P(name)) {
-	return rb_sym2id_without_pindown(name);
+	return name;
     }
     else if (!RB_TYPE_P(name, T_STRING)) {
 	tmp = rb_check_string_type(name);
@@ -11025,55 +11063,81 @@ rb_check_id_without_pindown(VALUE *namep
 
     sym_check_asciionly(name);
 
-    if (st_lookup(global_symbols.str_id, (st_data_t)name, &id))
-	return (ID)id;
+    if (lookup_str_sym(name, &sym)) {
+	return sym;
+    }
 
     {
 	ID gid = attrsetname_to_attr(name);
-	if (gid) return rb_id_attrset(gid);
+	if (gid) return ID2SYM(rb_id_attrset(gid));
     }
 
-    return (ID)0;
+    return Qnil;
 }
 
-static ID
-attrsetname_to_attr(VALUE name)
+ID
+rb_check_id_cstr(const char *ptr, long len, rb_encoding *enc)
 {
-    if (rb_is_attrset_name(name)) {
 	st_data_t id;
 	struct RString fake_str;
-	/* make local name by chopping '=' */
-	const VALUE localname = setup_fake_str(&fake_str, RSTRING_PTR(name), RSTRING_LEN(name) - 1);
-	rb_enc_copy(localname, name);
-	OBJ_FREEZE(localname);
+    const VALUE name = setup_fake_str(&fake_str, ptr, len);
+    rb_enc_associate(name, enc);
 
-	if (st_lookup(global_symbols.str_id, (st_data_t)localname, &id)) {
-	    return (ID)id;
+    sym_check_asciionly(name);
+
+    if (lookup_str_id(name, &id)) {
+	return id;
+    }
+
+    if (rb_is_attrset_name(name)) {
+	fake_str.as.heap.len = len - 1;
+	if (lookup_str_id((st_data_t)name, &id)) {
+	    return rb_id_attrset((ID)id);
 	}
-	RB_GC_GUARD(name);
     }
 
     return (ID)0;
 }
 
-ID
-rb_check_id_cstr_without_pindown(const char *ptr, long len, rb_encoding *enc)
+VALUE
+rb_check_symbol_cstr(const char *ptr, long len, rb_encoding *enc)
 {
-    st_data_t id;
+    st_data_t sym;
     struct RString fake_str;
     const VALUE name = setup_fake_str(&fake_str, ptr, len);
     rb_enc_associate(name, enc);
 
     sym_check_asciionly(name);
 
-    if (st_lookup(global_symbols.str_id, (st_data_t)name, &id))
-	return (ID)id;
+    if (lookup_str_sym(name, &sym)) {
+	return sym;
+    }
 
     if (rb_is_attrset_name(name)) {
 	fake_str.as.heap.len = len - 1;
-	if (st_lookup(global_symbols.str_id, (st_data_t)name, &id)) {
-	    return rb_id_attrset((ID)id);
+	if (lookup_str_sym((st_data_t)name, &sym)) {
+	    return ID2SYM(rb_id_attrset(SYM2ID(sym)));
+	}
+    }
+
+    return Qnil;
+}
+
+static ID
+attrsetname_to_attr(VALUE name)
+{
+    if (rb_is_attrset_name(name)) {
+	st_data_t id;
+	struct RString fake_str;
+	/* make local name by chopping '=' */
+	const VALUE localname = setup_fake_str(&fake_str, RSTRING_PTR(name), RSTRING_LEN(name) - 1);
+	rb_enc_copy(localname, name);
+	OBJ_FREEZE(localname);
+
+	if (lookup_str_id((st_data_t)localname, &id)) {
+	    return (ID)id;
 	}
+	RB_GC_GUARD(name);
     }
 
     return (ID)0;
Index: sprintf.c
===================================================================
--- sprintf.c	(revision 46756)
+++ sprintf.c	(working copy)
@@ -506,7 +506,7 @@ rb_str_format(int argc, const VALUE *arg
     for (; p < end; p++) {
 	const char *t;
 	int n;
-	ID id = 0;
+	VALUE sym = Qnil;
 
 	for (t = p; t < end && *t != '%'; t++) ;
 	PUSH(p, t - p);
@@ -601,16 +601,16 @@ rb_str_format(int argc, const VALUE *arg
 		}
 #endif
 		len = (int)(p - start + 1); /* including parenthesis */
-		if (id) {
+		if (sym != Qnil) {
 		    rb_enc_raise(enc, rb_eArgError, "named%.*s after <%s>",
-				 len, start, rb_id2name(id));
+				 len, start, RSTRING_PTR(rb_sym2str(sym)));
 		}
 		CHECKNAMEARG(start, len, enc);
 		get_hash(&hash, argc, argv);
-		id = rb_check_id_cstr_without_pindown(start + 1,
+		sym = rb_check_symbol_cstr(start + 1,
 						      len - 2 /* without parenthesis */,
 						      enc);
-		if (id) nextvalue = rb_hash_lookup2(hash, ID2SYM(id), Qundef);
+		if (sym != Qnil) nextvalue = rb_hash_lookup2(hash, sym, Qundef);
 		if (nextvalue == Qundef) {
 		    rb_enc_raise(enc, rb_eKeyError, "key%.*s not found", len, start);
 		}
Index: string.c
===================================================================
--- string.c	(revision 46756)
+++ string.c	(working copy)
@@ -8323,14 +8323,7 @@ str_scrub_bang(int argc, VALUE *argv, VA
 static VALUE
 sym_find(VALUE dummy, VALUE sym)
 {
-    ID id = rb_check_id(&sym);
-
-    if (id) {
-	return ID2SYM(id);
-    }
-    else {
-	return Qnil;
-    }
+    return rb_check_symbol(&sym);
 }
 
 /*
Index: struct.c
===================================================================
--- struct.c	(revision 46756)
+++ struct.c	(working copy)
@@ -759,7 +759,7 @@ rb_struct_aref(VALUE s, VALUE idx)
 	return rb_struct_aref_sym(s, idx);
     }
     else if (RB_TYPE_P(idx, T_STRING)) {
-	ID id = rb_check_id_without_pindown(&idx);
+	ID id = rb_check_id(&idx);
 	if (!id) {
 	    rb_name_error_str(idx, "no member '%"PRIsVALUE"' in struct",
 			      QUOTE(idx));
Index: thread.c
===================================================================
--- thread.c	(revision 46756)
+++ thread.c	(working copy)
@@ -2820,7 +2820,7 @@ rb_thread_local_aref(VALUE thread, ID id
 static VALUE
 rb_thread_aref(VALUE thread, VALUE key)
 {
-    ID id = rb_check_id_without_pindown(&key);
+    ID id = rb_check_id(&key);
     if (!id) return Qnil;
     return rb_thread_local_aref(thread, id);
 }
@@ -2906,7 +2906,7 @@ static VALUE
 rb_thread_variable_get(VALUE thread, VALUE key)
 {
     VALUE locals;
-    ID id = rb_check_id_without_pindown(&key);
+    ID id = rb_check_id(&key);
 
     if (!id) return Qnil;
     locals = rb_ivar_get(thread, id_locals);
@@ -2952,7 +2952,7 @@ static VALUE
 rb_thread_key_p(VALUE self, VALUE key)
 {
     rb_thread_t *th;
-    ID id = rb_check_id_without_pindown(&key);
+    ID id = rb_check_id(&key);
 
     GetThreadPtr(self, th);
 
@@ -3073,7 +3073,7 @@ static VALUE
 rb_thread_variable_p(VALUE thread, VALUE key)
 {
     VALUE locals;
-    ID id = rb_check_id_without_pindown(&key);
+    ID id = rb_check_id(&key);
 
     if (!id) return Qfalse;
 
Index: variable.c
===================================================================
--- variable.c	(revision 46756)
+++ variable.c	(working copy)
@@ -353,7 +353,7 @@ rb_path_to_class(VALUE pathname)
     }
     while (*p) {
 	while (*p && *p != ':') p++;
-	id = rb_check_id_cstr_without_pindown(pbeg, p-pbeg, enc);
+	id = rb_check_id_cstr(pbeg, p-pbeg, enc);
 	if (p[0] == ':') {
 	    if (p[1] != ':') goto undefined_class;
 	    p += 2;
@@ -1404,7 +1404,7 @@ VALUE
 rb_obj_remove_instance_variable(VALUE obj, VALUE name)
 {
     VALUE val = Qnil;
-    const ID id = rb_check_id_without_pindown(&name);
+    const ID id = rb_check_id(&name);
     st_data_t n, v;
     struct st_table *iv_index_tbl;
     st_data_t index;
@@ -1920,7 +1920,7 @@ rb_public_const_get_at(VALUE klass, ID i
 VALUE
 rb_mod_remove_const(VALUE mod, VALUE name)
 {
-    const ID id = rb_check_id_without_pindown(&name);
+    const ID id = rb_check_id(&name);
 
     if (!id) {
 	if (rb_is_const_name(name)) {
@@ -2569,7 +2569,7 @@ rb_mod_class_variables(int argc, const V
 VALUE
 rb_mod_remove_cvar(VALUE mod, VALUE name)
 {
-    const ID id = rb_check_id_without_pindown(&name);
+    const ID id = rb_check_id(&name);
     st_data_t val, n = id;
 
     if (!id) {
Index: vm_method.c
===================================================================
--- vm_method.c	(revision 46756)
+++ vm_method.c	(working copy)
@@ -797,7 +797,7 @@ rb_mod_remove_method(int argc, VALUE *ar
 
     for (i = 0; i < argc; i++) {
 	VALUE v = argv[i];
-	ID id = rb_check_id_without_pindown(&v);
+	ID id = rb_check_id(&v);
 	if (!id) {
 	    rb_name_error_str(v, "method `%s' not defined in %s",
 			      RSTRING_PTR(v), rb_class2name(mod));
@@ -1008,7 +1008,7 @@ rb_mod_undef_method(int argc, VALUE *arg
     int i;
     for (i = 0; i < argc; i++) {
 	VALUE v = argv[i];
-	ID id = rb_check_id_without_pindown(&v);
+	ID id = rb_check_id(&v);
 	if (!id) {
 	    rb_method_name_error(mod, v);
 	}
@@ -1048,7 +1048,7 @@ rb_mod_undef_method(int argc, VALUE *arg
 static VALUE
 rb_mod_method_defined(VALUE mod, VALUE mid)
 {
-    ID id = rb_check_id_without_pindown(&mid);
+    ID id = rb_check_id(&mid);
     if (!id || !rb_method_boundp(mod, id, 1)) {
 	return Qfalse;
     }
@@ -1062,7 +1062,7 @@ static VALUE
 check_definition(VALUE mod, VALUE mid, rb_method_flag_t noex)
 {
     const rb_method_entry_t *me;
-    ID id = rb_check_id_without_pindown(&mid);
+    ID id = rb_check_id(&mid);
     if (!id) return Qfalse;
     me = rb_method_entry(mod, id, 0);
     if (me) {
@@ -1691,7 +1691,7 @@ obj_respond_to(int argc, VALUE *argv, VA
     ID id;
 
     rb_scan_args(argc, argv, "11", &mid, &priv);
-    if (!(id = rb_check_id_without_pindown(&mid))) {
+    if (!(id = rb_check_id(&mid))) {
 	if (!rb_method_basic_definition_p(CLASS_OF(obj), idRespond_to_missing)) {
 	    VALUE args[2];
 	    args[0] = ID2SYM(rb_to_id(mid));

[authorNari (Narihiro Nakamura)]

パッチ作成と内容説明ありがとうございます。賛成します。

[ko1 (Koichi Sasada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

Applied in changeset r46764.

---

• parse.y: change Symbol <-> ID relationship to avoid
  exposing IDs from collectable symbols.
  [Bug #10014]
  Now, rb_check_id() returns 0 if corresponding symbol is
  pinned dynamic symbol.
  There is remaining intern_cstr_without_pindown(), it can return
  IDs from collectable symbols. We must be careful to use it
  (only used in parse.y). I think it should be removed if
  it does not have impact for performance.
• parse.y:
  add:
  • STATIC_SYM2ID()
  • STATIC_ID2SYM()
    rename:
  • rb_pin_dynamic_symbol() -> dsymbol_pindown()
• internal.h:
  remove:
  • rb_check_id_without_pindown()
  • rb_sym2id_without_pindown()
    add:
  • rb_check_symbol()
  • rb_check_symbol_cstr()
• load.c: use rb_check_id() or rb_check_id_cstr().
• object.c: ditto.
• struct.c: ditto.
• thread.c: ditto.
• vm_method.c: ditto.
• string.c (sym_find): use only rb_check_symbol().
• sprintf.c (rb_str_format): use rb_check_symbol_cstr().