Skip to content

Commit d077df2

Browse files
Earlopainkddnewton
authored andcommitted
[Bug #22002] Never pop when compiling branch predicate
The value is always needed. Now prism emits the same instructions as parse.y for the added test case. `defined?`/`flip-flop` caused `argument stack underflow`, `and`/`or` segfaulted during runtime.
1 parent 75387fd commit d077df2

2 files changed

Lines changed: 31 additions & 16 deletions

File tree

prism_compile.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -964,10 +964,10 @@ pm_code_location(pm_scope_node_t *scope_node, const pm_node_t *node)
964964

965965
static void
966966
pm_compile_branch_condition(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const pm_node_t *cond,
967-
LABEL *then_label, LABEL *else_label, bool popped, pm_scope_node_t *scope_node);
967+
LABEL *then_label, LABEL *else_label, pm_scope_node_t *scope_node);
968968

969969
static void
970-
pm_compile_logical(rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_node_t *cond, LABEL *then_label, LABEL *else_label, bool popped, pm_scope_node_t *scope_node)
970+
pm_compile_logical(rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_node_t *cond, LABEL *then_label, LABEL *else_label, pm_scope_node_t *scope_node)
971971
{
972972
const pm_node_location_t location = PM_NODE_START_LOCATION(cond);
973973

@@ -977,17 +977,14 @@ pm_compile_logical(rb_iseq_t *iseq, LINK_ANCHOR *const ret, pm_node_t *cond, LAB
977977
if (!then_label) then_label = label;
978978
else if (!else_label) else_label = label;
979979

980-
pm_compile_branch_condition(iseq, seq, cond, then_label, else_label, popped, scope_node);
980+
pm_compile_branch_condition(iseq, seq, cond, then_label, else_label, scope_node);
981981

982982
if (LIST_INSN_SIZE_ONE(seq)) {
983983
INSN *insn = (INSN *) ELEM_FIRST_INSN(FIRST_ELEMENT(seq));
984984
if (insn->insn_id == BIN(jump) && (LABEL *)(insn->operands[0]) == label) return;
985985
}
986986

987-
if (!label->refcnt) {
988-
if (popped) PUSH_INSN(ret, location, putnil);
989-
}
990-
else {
987+
if (label->refcnt) {
991988
PUSH_LABEL(seq, label);
992989
}
993990

@@ -1059,22 +1056,22 @@ pm_compile_flip_flop(const pm_flip_flop_node_t *flip_flop_node, LABEL *else_labe
10591056
static void pm_compile_defined_expr(rb_iseq_t *iseq, const pm_node_t *node, const pm_node_location_t *node_location, LINK_ANCHOR *const ret, bool popped, pm_scope_node_t *scope_node, bool in_condition);
10601057

10611058
static void
1062-
pm_compile_branch_condition(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const pm_node_t *cond, LABEL *then_label, LABEL *else_label, bool popped, pm_scope_node_t *scope_node)
1059+
pm_compile_branch_condition(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const pm_node_t *cond, LABEL *then_label, LABEL *else_label, pm_scope_node_t *scope_node)
10631060
{
10641061
const pm_node_location_t location = PM_NODE_START_LOCATION(cond);
10651062

10661063
again:
10671064
switch (PM_NODE_TYPE(cond)) {
10681065
case PM_AND_NODE: {
10691066
const pm_and_node_t *cast = (const pm_and_node_t *) cond;
1070-
pm_compile_logical(iseq, ret, cast->left, NULL, else_label, popped, scope_node);
1067+
pm_compile_logical(iseq, ret, cast->left, NULL, else_label, scope_node);
10711068

10721069
cond = cast->right;
10731070
goto again;
10741071
}
10751072
case PM_OR_NODE: {
10761073
const pm_or_node_t *cast = (const pm_or_node_t *) cond;
1077-
pm_compile_logical(iseq, ret, cast->left, then_label, NULL, popped, scope_node);
1074+
pm_compile_logical(iseq, ret, cast->left, then_label, NULL, scope_node);
10781075

10791076
cond = cast->right;
10801077
goto again;
@@ -1095,11 +1092,11 @@ pm_compile_branch_condition(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const pm_no
10951092
PUSH_INSNL(ret, location, jump, then_label);
10961093
return;
10971094
case PM_FLIP_FLOP_NODE:
1098-
pm_compile_flip_flop((const pm_flip_flop_node_t *) cond, else_label, then_label, iseq, location.line, ret, popped, scope_node);
1095+
pm_compile_flip_flop((const pm_flip_flop_node_t *) cond, else_label, then_label, iseq, location.line, ret, false, scope_node);
10991096
return;
11001097
case PM_DEFINED_NODE: {
11011098
const pm_defined_node_t *cast = (const pm_defined_node_t *) cond;
1102-
pm_compile_defined_expr(iseq, cast->value, &location, ret, popped, scope_node, true);
1099+
pm_compile_defined_expr(iseq, cast->value, &location, ret, false, scope_node, true);
11031100
break;
11041101
}
11051102
default: {
@@ -1143,7 +1140,7 @@ pm_compile_conditional(rb_iseq_t *iseq, const pm_node_location_t *node_location,
11431140
LABEL *end_label = NULL;
11441141

11451142
DECL_ANCHOR(cond_seq);
1146-
pm_compile_branch_condition(iseq, cond_seq, predicate, then_label, else_label, false, scope_node);
1143+
pm_compile_branch_condition(iseq, cond_seq, predicate, then_label, else_label, scope_node);
11471144
PUSH_SEQ(ret, cond_seq);
11481145

11491146
rb_code_location_t conditional_location = { 0 };
@@ -1291,10 +1288,10 @@ pm_compile_loop(rb_iseq_t *iseq, const pm_node_location_t *node_location, pm_nod
12911288
PUSH_LABEL(ret, next_label);
12921289

12931290
if (type == PM_WHILE_NODE) {
1294-
pm_compile_branch_condition(iseq, ret, predicate, redo_label, end_label, popped, scope_node);
1291+
pm_compile_branch_condition(iseq, ret, predicate, redo_label, end_label, scope_node);
12951292
}
12961293
else if (type == PM_UNTIL_NODE) {
1297-
pm_compile_branch_condition(iseq, ret, predicate, end_label, redo_label, popped, scope_node);
1294+
pm_compile_branch_condition(iseq, ret, predicate, end_label, redo_label, scope_node);
12981295
}
12991296

13001297
PUSH_LABEL(ret, end_label);
@@ -7693,7 +7690,7 @@ pm_compile_case_node(rb_iseq_t *iseq, const pm_case_node_t *cast, const pm_node_
76937690
}
76947691
else {
76957692
LABEL *next_label = NEW_LABEL(pm_node_line_number_cached(condition, scope_node));
7696-
pm_compile_branch_condition(iseq, cond_seq, condition, label, next_label, false, scope_node);
7693+
pm_compile_branch_condition(iseq, cond_seq, condition, label, next_label, scope_node);
76977694
PUSH_LABEL(cond_seq, next_label);
76987695
}
76997696
}

test/ruby/test_syntax.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1911,6 +1911,24 @@ def test_classdef_in_cond
19111911
assert_valid_syntax('while class Foo a = tap do end; end; break; end')
19121912
end
19131913

1914+
def test_while_until_conditional_bug_22002
1915+
@foo = 123 until defined?(@foo)
1916+
assert_equal(123, @foo)
1917+
1918+
@bar = 456 while @bar==nil..true
1919+
assert_equal(456, @bar)
1920+
1921+
while false and @baz
1922+
@baz = 789
1923+
end
1924+
assert_equal(nil, @baz)
1925+
1926+
until true || @baz
1927+
@baz = 789
1928+
end
1929+
assert_equal(nil, @baz)
1930+
end
1931+
19141932
def test_command_with_cmd_brace_block
19151933
assert_valid_syntax('obj.foo (1) {}')
19161934
assert_valid_syntax('obj::foo (1) {}')

0 commit comments

Comments
 (0)