https://git.ruby-lang.org/ruby.git/commit/?id=d8a4af47a5

From d8a4af47a5e5f72be2cd2897712d1718013b2e4c Mon Sep 17 00:00:00 2001
From: Aaron Patterson <tenderlove@r...>
Date: Mon, 9 Sep 2019 10:15:07 -0700
Subject: Only use `add_mark_object` in Ripper

This patch changes parse.y to only use `add_mark_object` in Ripper.
Previously we were seeing a bug in write barrier verification.  I had
changed `add_mark_object` to execute the write barrier, but the problem
is that we had code like this:

```
NEW_STR(add_mark_object(p, obj), loc)
```

In this case, `add_mark_object` would execute the write barrier between
the ast and `obj`, but the problem is that `obj` isn't actually
reachable from the AST at the time the write barrier executed.
`NEW_STR` can possibly call `malloc` which can kick a GC, and since
`obj` isn't actually reachable from the AST at the time of WB execution,
verification would fail.

Basically the steps were like this:

1. RB_OBJ_WRITTEN via `add_mark_object`
2. Allocate node
3. *Possibly* execute GC via malloc
4. Write obj in to allocated node

This patch changes the steps to:

1. Allocate node
2. *Possibly* execute GC via malloc
3. Write obj in to allocated node
4. RB_OBJ_WRITTEN

diff --git a/parse.y b/parse.y
index 7e2cfe0..852a5ed 100644
--- a/parse.y
+++ b/parse.y
@@ -336,22 +336,18 @@ rb_discard_node(struct parser_params *p, NODE *n) https://github.com/ruby/ruby/blob/trunk/parse.y#L336
 }
 #endif
 
+#ifdef RIPPER
 static inline VALUE
 add_mark_object(struct parser_params *p, VALUE obj)
 {
     if (!SPECIAL_CONST_P(obj)
-#ifdef RIPPER
 	&& !RB_TYPE_P(obj, T_NODE) /* Ripper jumbles NODE objects and other objects... */
-#endif
     ) {
-#ifdef RIPPER
 	rb_ast_add_mark_object(p->ast, obj);
-#else
-        RB_OBJ_WRITTEN(p->ast, Qundef, obj);
-#endif
     }
     return obj;
 }
+#endif
 
 static NODE* node_newnode(struct parser_params *, enum node_type, VALUE, VALUE, VALUE, const rb_code_location_t*);
 #define rb_node_newnode(type, a1, a2, a3, loc) node_newnode(p, (type), (a1), (a2), (a3), (loc))
@@ -4245,7 +4241,8 @@ strings		: string https://github.com/ruby/ruby/blob/trunk/parse.y#L4241
 		    /*%%%*/
 			NODE *node = $1;
 			if (!node) {
-			    node = NEW_STR(add_mark_object(p, STR_NEW0()), &@$);
+			    node = NEW_STR(STR_NEW0(), &@$);
+                            RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit);
 			}
 			else {
 			    node = evstr2dstr(p, node);
@@ -4609,7 +4606,7 @@ numeric 	: simple_numeric https://github.com/ruby/ruby/blob/trunk/parse.y#L4606
 		    {
 		    /*%%%*/
 			$$ = $2;
-			add_mark_object(p, $$->nd_lit = negate_lit(p, $$->nd_lit));
+			RB_OBJ_WRITTEN(p->ast, &$$->nd_lit, $$->nd_lit = negate_lit(p, $$->nd_lit));
 		    /*% %*/
 		    /*% ripper: unary!(ID2VAL(idUMinus), $2) %*/
 		    }
@@ -5186,7 +5183,7 @@ assoc		: arg_value tASSOC arg_value https://github.com/ruby/ruby/blob/trunk/parse.y#L5183
 		    /*%%%*/
 			if (nd_type($1) == NODE_STR) {
 			    nd_set_type($1, NODE_LIT);
-			    add_mark_object(p, $1->nd_lit = rb_fstring($1->nd_lit));
+			    RB_OBJ_WRITTEN(p->ast, &$1->nd_lit, $1->nd_lit = rb_fstring($1->nd_lit));
 			}
 			$$ = list_append(p, NEW_LIST($1, &@$), $3);
 		    /*% %*/
@@ -5304,8 +5301,16 @@ static enum yytokentype here_document(struct parser_params*,rb_strterm_heredoc_t https://github.com/ruby/ruby/blob/trunk/parse.y#L5301
   rb_parser_set_location(p, &_cur_loc);			\
   yylval.node = (x);					\
 }
-# define set_yylval_str(x) set_yylval_node(NEW_STR(add_mark_object(p, (x)), &_cur_loc))
-# define set_yylval_literal(x) set_yylval_node(NEW_LIT(add_mark_object(p, (x)), &_cur_loc))
+# define set_yylval_str(x) \
+do { \
+  set_yylval_node(NEW_STR(x, &_cur_loc)); \
+  RB_OBJ_WRITTEN(p->ast, Qnil, x); \
+} while(0)
+# define set_yylval_literal(x) \
+do { \
+  set_yylval_node(NEW_LIT(x, &_cur_loc)); \
+  RB_OBJ_WRITTEN(p->ast, Qnil, x); \
+} while(0)
 # define set_yylval_num(x) (yylval.num = (x))
 # define set_yylval_id(x)  (yylval.id = (x))
 # define set_yylval_name(x)  (yylval.id = (x))
@@ -9549,7 +9554,8 @@ literal_concat(struct parser_params *p, NODE *head, NODE *tail, const YYLTYPE *l https://github.com/ruby/ruby/blob/trunk/parse.y#L9554
 
     htype = nd_type(head);
     if (htype == NODE_EVSTR) {
-	NODE *node = NEW_DSTR(add_mark_object(p, STR_NEW0()), loc);
+	NODE *node = NEW_DSTR(STR_NEW0(), loc);
+        RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit);
 	head = list_append(p, node, head);
 	htype = NODE_DSTR;
     }
@@ -9630,7 +9636,9 @@ static NODE * https://github.com/ruby/ruby/blob/trunk/parse.y#L9636
 evstr2dstr(struct parser_params *p, NODE *node)
 {
     if (nd_type(node) == NODE_EVSTR) {
-	node = list_append(p, NEW_DSTR(add_mark_object(p, STR_NEW0()), &node->nd_loc), node);
+	NODE * dstr = NEW_DSTR(STR_NEW0(), &node->nd_loc);
+        RB_OBJ_WRITTEN(p->ast, Qnil, dstr->nd_lit);
+	node = list_append(p, dstr, node);
     }
     return node;
 }
@@ -9785,14 +9793,18 @@ gettable(struct parser_params *p, ID id, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L9793
 		file = rb_str_new(0, 0);
 	    else
 		file = rb_str_dup(file);
-	    node = NEW_STR(add_mark_object(p, file), loc);
+	    node = NEW_STR(file, loc);
+            RB_OBJ_WRITTEN(p->ast, Qnil, file);
 	}
 	return node;
       case keyword__LINE__:
 	WARN_LOCATION("__LINE__");
 	return NEW_LIT(INT2FIX(p->tokline), loc);
       case keyword__ENCODING__:
-	return NEW_LIT(add_mark_object(p, rb_enc_from_encoding(p->enc)), loc);
+        node = NEW_LIT(rb_enc_from_encoding(p->enc), loc);
+        RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit);
+        return node;
+
     }
     switch (id_type(id)) {
       case ID_INTERNAL:
@@ -9882,7 +9894,7 @@ symbol_append(struct parser_params *p, NODE *symbols, NODE *symbol) https://github.com/ruby/ruby/blob/trunk/parse.y#L9894
     }
     else {
 	nd_set_type(symbol, NODE_LIT);
-	symbol->nd_lit = add_mark_object(p, rb_str_intern(symbol->nd_lit));
+	RB_OBJ_WRITTEN(p->ast, Qnil, symbol->nd_lit = rb_str_intern(symbol->nd_lit));
     }
     return list_append(p, symbols, symbol);
 }
@@ -9894,7 +9906,9 @@ new_regexp(struct parser_params *p, NODE *node, int options, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L9906
     VALUE lit;
 
     if (!node) {
-	return NEW_LIT(add_mark_object(p, reg_compile(p, STR_NEW0(), options)), loc);
+	node = NEW_LIT(reg_compile(p, STR_NEW0(), options), loc);
+	RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit);
+        return node;
     }
     switch (nd_type(node)) {
       case NODE_STR:
@@ -9902,12 +9916,13 @@ new_regexp(struct parser_params *p, NODE *node, int options, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L9916
 	    VALUE src = node->nd_lit;
 	    nd_set_type(node, NODE_LIT);
 	    nd_set_loc(node, loc);
-	    add_mark_object(p, node->nd_lit = reg_compile(p, src, options));
+	    RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit = reg_compile(p, src, options));
 	}
 	break;
       default:
-	add_mark_object(p, lit = STR_NEW0());
+	lit = STR_NEW0();
 	node = NEW_NODE(NODE_DSTR, lit, 1, NEW_LIST(node, loc), loc);
+        RB_OBJ_WRITTEN(p->ast, Qnil, lit);
 	/* fall through */
       case NODE_DSTR:
 	nd_set_type(node, NODE_DREGX);
@@ -9939,7 +9954,7 @@ new_regexp(struct parser_params *p, NODE *node, int options, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L9954
 	if (!node->nd_next) {
 	    VALUE src = node->nd_lit;
 	    nd_set_type(node, NODE_LIT);
-	    add_mark_object(p, node->nd_lit = reg_compile(p, src, options));
+	    RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit = reg_compile(p, src, options));
 	}
 	if (options & RE_OPTION_ONCE) {
 	    node = NEW_NODE(NODE_ONCE, 0, node, 0, loc);
@@ -9962,7 +9977,7 @@ new_xstring(struct parser_params *p, NODE *node, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L9977
     if (!node) {
 	VALUE lit = STR_NEW0();
 	NODE *xstr = NEW_XSTR(lit, loc);
-	add_mark_object(p, lit);
+	RB_OBJ_WRITTEN(p->ast, Qnil, lit);
 	return xstr;
     }
     switch (nd_type(node)) {
@@ -9991,7 +10006,7 @@ check_literal_when(struct parser_params *p, NODE *arg, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L10006
     lit = rb_node_case_when_optimizable_literal(arg);
     if (lit == Qundef) return;
     if (nd_type(arg) == NODE_STR) {
-	arg->nd_lit = add_mark_object(p, lit);
+	RB_OBJ_WRITTEN(p->ast, Qnil, arg->nd_lit = lit);
     }
 
     if (NIL_P(p->case_labels)) {
@@ -11307,7 +11322,7 @@ dsym_node(struct parser_params *p, NODE *node, const YYLTYPE *loc) https://github.com/ruby/ruby/blob/trunk/parse.y#L11322
 	break;
       case NODE_STR:
 	lit = node->nd_lit;
-	add_mark_object(p, node->nd_lit = ID2SYM(rb_intern_str(lit)));
+	RB_OBJ_WRITTEN(p->ast, Qnil, node->nd_lit = ID2SYM(rb_intern_str(lit)));
 	nd_set_type(node, NODE_LIT);
 	nd_set_loc(node, loc);
 	break;
-- 
cgit v0.10.2


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