diff options
author | Aaron Patterson <tenderlove@ruby-lang.org> | 2019-09-11 14:02:05 -0700 |
---|---|---|
committer | Aaron Patterson <tenderlove@ruby-lang.org> | 2019-09-11 14:58:51 -0700 |
commit | 515b1989b1093a4dddef83d0cda763c9ae6760e3 (patch) | |
tree | 2643015bd03caf3de2d9ba2a383052dd8fcdec2c | |
parent | 21994b7fd686f263544fcac1616ecf3189fb78b3 (diff) | |
download | ruby-515b1989b1093a4dddef83d0cda763c9ae6760e3.tar.gz |
Make NODE_ARYPTN layout consistent between Ripper and AST
We are seeing SEGVs in CI:
http://ci.rvm.jp/results/trunk-gc-asserts@ruby-sky1/2253563
This is happening because Ripper constructs AST nodes differently than
parse.y normally does. Specifically in this case Ripper is assigning 3
`VALUE` objects:
https://github.com/ruby/ruby/blob/1febb6f4a14f7222c6d30250bfdc252d34238187/parse.y#L757-L761
Where parse.y will normally assign other things:
https://github.com/ruby/ruby/blob/1febb6f4a14f7222c6d30250bfdc252d34238187/parse.y#L11258-L11260
The important one is the last one, the `struct rb_ary_pattern_info`. The
mark function assumed that `NODE_ARYPTN` have a pointer to `struct
rb_ary_pattern_info`, and used it:
https://github.com/ruby/ruby/blob/1febb6f4a14f7222c6d30250bfdc252d34238187/node.c#L1269-L1274
In the case of Ripper, `NODE_ARYPTN` doesn't point to an
`rb_ary_pattern_info`, so the mark function would SEGV. This commit
changes Ripper so that its `NODE_ARYPTN` nodes also point at an
`rb_ary_pattern_info`, and the mark function can continue with the same
assumption.
-rw-r--r-- | parse.y | 24 |
1 files changed, 19 insertions, 5 deletions
@@ -732,7 +732,15 @@ static VALUE new_array_pattern(struct parser_params *p, VALUE constant, VALUE pre_arg, VALUE aryptn, const YYLTYPE *loc) { NODE *t = (NODE *)aryptn; - VALUE pre_args = t->u1.value, rest_arg = t->u2.value, post_args = t->u3.value; + struct rb_ary_pattern_info *apinfo = t->nd_apinfo; + VALUE pre_args = Qnil, rest_arg = Qnil, post_args = Qnil; + + if (apinfo) { + pre_args = rb_ary_entry(apinfo->imemo, 0); + rest_arg = rb_ary_entry(apinfo->imemo, 1); + post_args = rb_ary_entry(apinfo->imemo, 2); + } + if (!NIL_P(pre_arg)) { if (!NIL_P(pre_args)) { rb_ary_unshift(pre_args, pre_arg); @@ -748,17 +756,23 @@ static VALUE new_array_pattern_tail(struct parser_params *p, VALUE pre_args, VALUE has_rest, VALUE rest_arg, VALUE post_args, const YYLTYPE *loc) { NODE *t; + struct rb_ary_pattern_info *apinfo; + if (has_rest) { rest_arg = dispatch1(var_field, rest_arg ? rest_arg : Qnil); } else { rest_arg = Qnil; } - t = rb_node_newnode(NODE_ARYPTN, pre_args, rest_arg, post_args, &NULL_LOC); - add_mark_object(p, pre_args); - add_mark_object(p, rest_arg); - add_mark_object(p, post_args); + apinfo = ZALLOC(struct rb_ary_pattern_info); + /* VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(apinfo); */ + VALUE tmpbuf = rb_imemo_new(imemo_tmpbuf, (VALUE)apinfo, 0, 0, 0); + apinfo->imemo = rb_ary_new_from_args(4, pre_args, rest_arg, post_args, tmpbuf); + + t = rb_node_newnode(NODE_ARYPTN, Qnil, Qnil, (VALUE)apinfo, &NULL_LOC); + RB_OBJ_WRITTEN(p->ast, Qnil, apinfo->imemo); + return (VALUE)t; } |