Skip to content

Commit

Permalink
[PRISM] Fix PM_CALL_NODE assignment
Browse files Browse the repository at this point in the history 
This PR fixes ruby/prism#1963. Array and variable assignment was broken
for call nodes. The change checks if the `method_id` is related to
assignment and if is adds a `putnil`, `setn` and a `pop`.

The incorrect instructions meant that in some cases (demonstrated in
tests) the wrong value would be returned.

I verified that this fixes the test mentioned in the issue
(run: `RUBY_ISEQ_DUMP_DEBUG=prism make test/-ext-/st/test_numhash.rb`)

Incorrect instructions:

```
"********* Ruby *************"
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(4,10)>
0000 putnil                                                           (   4)[Li]
0001 putself
0002 send                                   <calldata!mid:tbl, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0005 putself
0006 send                                   <calldata!mid:i, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0009 putself
0010 send                                   <calldata!mid:j, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0013 setn                                   3
0015 send                                   <calldata!mid:[]=, argc:2, ARGS_SIMPLE>, nil
0018 pop
0019 leave

"********* PRISM *************"
== disasm: #<ISeq:<compiled>@<compiled>:3 (3,0)-(3,10)>
0000 putself                                                          (   3)[Li]
0001 send                                   <calldata!mid:tbl, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0004 putself
0005 send                                   <calldata!mid:i, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0008 putself
0009 send                                   <calldata!mid:j, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0012 send                                   <calldata!mid:[]=, argc:2, ARGS_SIMPLE>, nil
0015 leave
```

Fixed instructions:

```
"********* Ruby *************"
== disasm: #<ISeq:<compiled>@<compiled>:1 (1,0)-(4,10)>
0000 putnil                                                           (   4)[Li]
0001 putself
0002 send                                   <calldata!mid:tbl, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0005 putself
0006 send                                   <calldata!mid:i, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0009 putself
0010 send                                   <calldata!mid:j, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0013 setn                                   3
0015 send                                   <calldata!mid:[]=, argc:2, ARGS_SIMPLE>, nil
0018 pop
0019 leave

"********* PRISM *************"
== disasm: #<ISeq:<compiled>@<compiled>:3 (3,0)-(3,10)>
0000 putnil                                                           (   3)[Li]
0001 putself
0002 send                                   <calldata!mid:tbl, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0005 putself
0006 send                                   <calldata!mid:i, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0009 putself
0010 send                                   <calldata!mid:j, argc:0, FCALL|VCALL|ARGS_SIMPLE>, nil
0013 setn                                   3
0015 send                                   <calldata!mid:[]=, argc:2, ARGS_SIMPLE>, nil
0018 pop
0019 leave
```

Fixes ruby/prism#1963
  • Loading branch information
@eileencodes @tenderlove
eileencodes authored and tenderlove committed Dec 8, 2023
1 parent 9e7ca2c commit 1cbe114
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
24 changes: 19 additions & 5 deletions prism_compile.c
View file
Expand Up @@ -1465,7 +1465,7 @@ pm_scope_node_init(const pm_node_t *node, pm_scope_node_t *scope, pm_scope_node_
}
}

static void pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *const ret, const uint8_t *src, bool popped, pm_scope_node_t *scope_node);
static void pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *const ret, const uint8_t *src, bool popped, pm_scope_node_t *scope_node, ID method_id);

void
pm_compile_defined_expr0(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret, const uint8_t *src, bool popped, pm_scope_node_t *scope_node, NODE dummy_line_node, int lineno, bool in_condition, LABEL **lfinish, bool explicit_receiver)
Expand Down Expand Up @@ -1637,7 +1637,8 @@ pm_compile_defined_expr0(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *co
pm_compile_defined_expr0(iseq, call_node->receiver, ret, src, popped, scope_node, dummy_line_node, lineno, true, lfinish, true);
if (PM_NODE_TYPE_P(call_node->receiver, PM_CALL_NODE)) {
ADD_INSNL(ret, &dummy_line_node, branchunless, lfinish[2]);
pm_compile_call(iseq, (const pm_call_node_t *)call_node->receiver, ret, src, popped, scope_node);
ID method_id = pm_constant_id_lookup(scope_node, call_node->name);
pm_compile_call(iseq, (const pm_call_node_t *)call_node->receiver, ret, src, popped, scope_node, method_id);
}
else {
ADD_INSNL(ret, &dummy_line_node, branchunless, lfinish[1]);
Expand Down Expand Up @@ -1765,14 +1766,13 @@ pm_compile_defined_expr(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *con
}

static void
pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *const ret, const uint8_t *src, bool popped, pm_scope_node_t *scope_node)
pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *const ret, const uint8_t *src, bool popped, pm_scope_node_t *scope_node, ID method_id)
{
pm_parser_t *parser = scope_node->parser;
pm_newline_list_t newline_list = parser->newline_list;
int lineno = (int)pm_newline_list_line_column(&newline_list, ((pm_node_t *)call_node)->location.start).line;
NODE dummy_line_node = generate_dummy_line_node(lineno, lineno);

ID method_id = pm_constant_id_lookup(scope_node, call_node->name);
int flags = 0;
struct rb_callinfo_kwarg *kw_arg = NULL;

Expand Down Expand Up @@ -1806,7 +1806,16 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c
flags |= VM_CALL_FCALL;
}

if (rb_is_attrset_id(method_id)) {
ADD_INSN1(ret, &dummy_line_node, setn, INT2FIX(orig_argc + 1));
}

ADD_SEND_R(ret, &dummy_line_node, method_id, INT2FIX(orig_argc), block_iseq, INT2FIX(flags), kw_arg);

if (rb_is_attrset_id(method_id)) {
PM_POP;
}

PM_POP_IF_POPPED;
}

Expand Down Expand Up @@ -2149,13 +2158,18 @@ pm_compile_node(rb_iseq_t *iseq, const pm_node_t *node, LINK_ANCHOR *const ret,
case PM_CALL_NODE: {
pm_call_node_t *call_node = (pm_call_node_t *) node;

ID method_id = pm_constant_id_lookup(scope_node, call_node->name);
if (rb_is_attrset_id(method_id)) {
PM_PUTNIL;
}

if (call_node->receiver == NULL) {
PM_PUTSELF;
} else {
PM_COMPILE_NOT_POPPED(call_node->receiver);
}

pm_compile_call(iseq, call_node, ret, src, popped, scope_node);
pm_compile_call(iseq, call_node, ret, src, popped, scope_node, method_id);
return;
}
case PM_CALL_AND_WRITE_NODE: {
Expand Down
28 changes: 28 additions & 0 deletions test/ruby/test_compile_prism.rb
View file
Expand Up @@ -1301,6 +1301,34 @@ def self.prism_test_call_node_splat(*); end
CODE

assert_prism_eval("prism_test_call_node_splat(*[], 1, 2)")

assert_prism_eval(<<-CODE)
class Foo
def []=(a, b)
1234
end
end
def self.foo(i, j)
tbl = Foo.new
tbl[i] = j
end
foo(1, 2)
CODE

assert_prism_eval(<<-CODE)
class Foo
def i=(a)
1234
end
end
def self.foo(j)
tbl = Foo.new
tbl.i = j
end
foo(1)
CODE
end

def test_CallAndWriteNode
Expand Down

0 comments on commit 1cbe114

Please sign in to comment.