Skip to content

Commit 51ffef2

Browse files
peterzhu2118peterzhu2118
committed
Fix memory leak in prism when syntax error in iseq compilation
If there's a syntax error during iseq compilation then prism would leak memory because it would not free the pm_parse_result_t. This commit changes pm_iseq_new_with_opt to have a rb_protect to catch when an error is raised, and return NULL and set error_state to a value that can be raised by calling rb_jump_tag after memory has been freed. For example: 10.times do 10_000.times do eval("/[/=~s") rescue SyntaxError end puts `ps -o rss= -p #{$$}` end Before: 39280 68736 99232 128864 158896 188208 217344 246304 275376 304592 After: 12192 13200 14256 14848 16000 16000 16000 16064 17232 17952
1 parent 72550d2 commit 51ffef2

File tree

8 files changed

+130
-25
lines changed

8 files changed

+130
-25
lines changed

‎iseq.c‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -897,12 +897,12 @@ rb_iseq_new_top(const VALUE ast_value, VALUE name, VALUE path, VALUE realpath, c
897897
* The main entry-point into the prism compiler when a file is required.
898898
*/
899899
rb_iseq_t *
900-
pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent)
900+
pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, int *error_state)
901901
{
902902
iseq_new_setup_coverage(path, (int) (node->parser->newline_list.size - 1));
903903

904904
return pm_iseq_new_with_opt(node, name, path, realpath, 0, parent, 0,
905-
ISEQ_TYPE_TOP, &COMPILE_OPTION_DEFAULT);
905+
ISEQ_TYPE_TOP, &COMPILE_OPTION_DEFAULT, error_state);
906906
}
907907

908908
rb_iseq_t *
@@ -921,13 +921,13 @@ rb_iseq_new_main(const VALUE ast_value, VALUE path, VALUE realpath, const rb_ise
921921
* main file in the program.
922922
*/
923923
rb_iseq_t *
924-
pm_iseq_new_main(pm_scope_node_t *node, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt)
924+
pm_iseq_new_main(pm_scope_node_t *node, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt, int *error_state)
925925
{
926926
iseq_new_setup_coverage(path, (int) (node->parser->newline_list.size - 1));
927927

928928
return pm_iseq_new_with_opt(node, rb_fstring_lit("<main>"),
929929
path, realpath, 0,
930-
parent, 0, ISEQ_TYPE_MAIN, opt ? &COMPILE_OPTION_DEFAULT : &COMPILE_OPTION_FALSE);
930+
parent, 0, ISEQ_TYPE_MAIN, opt ? &COMPILE_OPTION_DEFAULT : &COMPILE_OPTION_FALSE, error_state);
931931
}
932932

933933
rb_iseq_t *
@@ -947,7 +947,7 @@ rb_iseq_new_eval(const VALUE ast_value, VALUE name, VALUE path, VALUE realpath,
947947

948948
rb_iseq_t *
949949
pm_iseq_new_eval(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath,
950-
int first_lineno, const rb_iseq_t *parent, int isolated_depth)
950+
int first_lineno, const rb_iseq_t *parent, int isolated_depth, int *error_state)
951951
{
952952
if (rb_get_coverage_mode() & COVERAGE_TARGET_EVAL) {
953953
VALUE coverages = rb_get_coverages();
@@ -957,7 +957,7 @@ pm_iseq_new_eval(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath,
957957
}
958958

959959
return pm_iseq_new_with_opt(node, name, path, realpath, first_lineno,
960-
parent, isolated_depth, ISEQ_TYPE_EVAL, &COMPILE_OPTION_DEFAULT);
960+
parent, isolated_depth, ISEQ_TYPE_EVAL, &COMPILE_OPTION_DEFAULT, error_state);
961961
}
962962

963963
static inline rb_iseq_t *
@@ -1013,6 +1013,25 @@ rb_iseq_new_with_opt(VALUE ast_value, VALUE name, VALUE path, VALUE realpath,
10131013
return iseq_translate(iseq);
10141014
}
10151015

1016+
struct pm_iseq_new_with_opt_data {
1017+
rb_iseq_t *iseq;
1018+
pm_scope_node_t *node;
1019+
};
1020+
1021+
VALUE
1022+
pm_iseq_new_with_opt_try(VALUE d)
1023+
{
1024+
struct pm_iseq_new_with_opt_data *data = (struct pm_iseq_new_with_opt_data *)d;
1025+
1026+
// This can compile child iseqs, which can raise syntax errors
1027+
pm_iseq_compile_node(data->iseq, data->node);
1028+
1029+
// This raises an exception if there is a syntax error
1030+
finish_iseq_build(data->iseq);
1031+
1032+
return Qundef;
1033+
}
1034+
10161035
/**
10171036
* This is a step in the prism compiler that is called once all of the various
10181037
* options have been established. It is called from one of the pm_iseq_new_*
@@ -1028,7 +1047,7 @@ rb_iseq_new_with_opt(VALUE ast_value, VALUE name, VALUE path, VALUE realpath,
10281047
rb_iseq_t *
10291048
pm_iseq_new_with_opt(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath,
10301049
int first_lineno, const rb_iseq_t *parent, int isolated_depth,
1031-
enum rb_iseq_type type, const rb_compile_option_t *option)
1050+
enum rb_iseq_type type, const rb_compile_option_t *option, int *error_state)
10321051
{
10331052
rb_iseq_t *iseq = iseq_alloc();
10341053
ISEQ_BODY(iseq)->prism = true;
@@ -1054,8 +1073,13 @@ pm_iseq_new_with_opt(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpa
10541073
prepare_iseq_build(iseq, name, path, realpath, first_lineno, &code_location, -1,
10551074
parent, isolated_depth, type, node->script_lines == NULL ? Qnil : *node->script_lines, option);
10561075

1057-
pm_iseq_compile_node(iseq, node);
1058-
finish_iseq_build(iseq);
1076+
struct pm_iseq_new_with_opt_data data = {
1077+
.iseq = iseq,
1078+
.node = node
1079+
};
1080+
rb_protect(pm_iseq_new_with_opt_try, (VALUE)&data, error_state);
1081+
1082+
if (*error_state) return NULL;
10591083

10601084
return iseq_translate(iseq);
10611085
}
@@ -1313,8 +1337,15 @@ pm_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V
13131337
}
13141338

13151339
if (error == Qnil) {
1316-
iseq = pm_iseq_new_with_opt(&result.node, name, file, realpath, ln, NULL, 0, ISEQ_TYPE_TOP, &option);
1340+
int error_state;
1341+
iseq = pm_iseq_new_with_opt(&result.node, name, file, realpath, ln, NULL, 0, ISEQ_TYPE_TOP, &option, &error_state);
1342+
13171343
pm_parse_result_free(&result);
1344+
1345+
if (error_state) {
1346+
RUBY_ASSERT(iseq == NULL);
1347+
rb_jump_tag(error_state);
1348+
}
13181349
}
13191350
else {
13201351
pm_parse_result_free(&result);
@@ -1771,11 +1802,20 @@ iseqw_s_compile_file_prism(int argc, VALUE *argv, VALUE self)
17711802
if (error == Qnil) {
17721803
make_compile_option(&option, opt);
17731804

1774-
ret = iseqw_new(pm_iseq_new_with_opt(&result.node, rb_fstring_lit("<main>"),
1775-
file,
1776-
rb_realpath_internal(Qnil, file, 1),
1777-
1, NULL, 0, ISEQ_TYPE_TOP, &option));
1805+
int error_state;
1806+
rb_iseq_t *iseq = pm_iseq_new_with_opt(&result.node, rb_fstring_lit("<main>"),
1807+
file,
1808+
rb_realpath_internal(Qnil, file, 1),
1809+
1, NULL, 0, ISEQ_TYPE_TOP, &option, &error_state);
1810+
17781811
pm_parse_result_free(&result);
1812+
1813+
if (error_state) {
1814+
RUBY_ASSERT(iseq == NULL);
1815+
rb_jump_tag(error_state);
1816+
}
1817+
1818+
ret = iseqw_new(iseq);
17791819
rb_vm_pop_frame(ec);
17801820
RB_GC_GUARD(v);
17811821
return ret;

‎load.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,8 +752,15 @@ load_iseq_eval(rb_execution_context_t *ec, VALUE fname)
752752
VALUE error = pm_load_parse_file(&result, fname, NULL);
753753

754754
if (error == Qnil) {
755-
iseq = pm_iseq_new_top(&result.node, rb_fstring_lit("<top (required)>"), fname, realpath_internal_cached(realpath_map, fname), NULL);
755+
int error_state;
756+
iseq = pm_iseq_new_top(&result.node, rb_fstring_lit("<top (required)>"), fname, realpath_internal_cached(realpath_map, fname), NULL, &error_state);
757+
756758
pm_parse_result_free(&result);
759+
760+
if (error_state) {
761+
RUBY_ASSERT(iseq == NULL);
762+
rb_jump_tag(error_state);
763+
}
757764
}
758765
else {
759766
rb_vm_pop_frame(ec);

‎mini_builtin.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,16 @@ builtin_iseq_load(const char *feature_name, const struct rb_builtin_function *ta
6363
pm_prelude_load(&result, name_str, code, start_line);
6464

6565
vm->builtin_function_table = table;
66-
iseq = pm_iseq_new_with_opt(&result.node, name_str, name_str, Qnil, 0, NULL, 0, ISEQ_TYPE_TOP, &optimization);
66+
int error_state;
67+
iseq = pm_iseq_new_with_opt(&result.node, name_str, name_str, Qnil, 0, NULL, 0, ISEQ_TYPE_TOP, &optimization, &error_state);
6768

6869
vm->builtin_function_table = NULL;
6970
pm_parse_result_free(&result);
71+
72+
if (error_state) {
73+
RUBY_ASSERT(iseq == NULL);
74+
rb_jump_tag(error_state);
75+
}
7076
}
7177
else {
7278
VALUE ast_value = prelude_ast_value(name_str, code, start_line);

‎prism_compile.c‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,11 +1268,17 @@ pm_new_child_iseq(rb_iseq_t *iseq, pm_scope_node_t *node, VALUE name, const rb_i
12681268
{
12691269
debugs("[new_child_iseq]> ---------------------------------------\n");
12701270
int isolated_depth = ISEQ_COMPILE_DATA(iseq)->isolated_depth;
1271+
int error_state;
12711272
rb_iseq_t *ret_iseq = pm_iseq_new_with_opt(node, name,
12721273
rb_iseq_path(iseq), rb_iseq_realpath(iseq),
12731274
line_no, parent,
12741275
isolated_depth ? isolated_depth + 1 : 0,
1275-
type, ISEQ_COMPILE_DATA(iseq)->option);
1276+
type, ISEQ_COMPILE_DATA(iseq)->option, &error_state);
1277+
1278+
if (error_state) {
1279+
RUBY_ASSERT(ret_iseq == NULL);
1280+
rb_jump_tag(error_state);
1281+
}
12761282
debugs("[new_child_iseq]< ---------------------------------------\n");
12771283
return ret_iseq;
12781284
}
@@ -3479,6 +3485,7 @@ pm_compile_builtin_mandatory_only_method(rb_iseq_t *iseq, pm_scope_node_t *scope
34793485
pm_scope_node_t next_scope_node;
34803486
pm_scope_node_init(&def.base, &next_scope_node, scope_node);
34813487

3488+
int error_state;
34823489
ISEQ_BODY(iseq)->mandatory_only_iseq = pm_iseq_new_with_opt(
34833490
&next_scope_node,
34843491
rb_iseq_base_label(iseq),
@@ -3488,9 +3495,15 @@ pm_compile_builtin_mandatory_only_method(rb_iseq_t *iseq, pm_scope_node_t *scope
34883495
NULL,
34893496
0,
34903497
ISEQ_TYPE_METHOD,
3491-
ISEQ_COMPILE_DATA(iseq)->option
3498+
ISEQ_COMPILE_DATA(iseq)->option,
3499+
&error_state
34923500
);
34933501

3502+
if (error_state) {
3503+
RUBY_ASSERT(ISEQ_BODY(iseq)->mandatory_only_iseq == NULL);
3504+
rb_jump_tag(error_state);
3505+
}
3506+
34943507
pm_scope_node_destroy(&next_scope_node);
34953508
return COMPILE_OK;
34963509
}

‎prism_compile.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,10 @@ VALUE pm_parse_string(pm_parse_result_t *result, VALUE source, VALUE filepath, V
9090
VALUE pm_parse_stdin(pm_parse_result_t *result);
9191
void pm_parse_result_free(pm_parse_result_t *result);
9292

93-
rb_iseq_t *pm_iseq_new(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type);
94-
rb_iseq_t *pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent);
95-
rb_iseq_t *pm_iseq_new_main(pm_scope_node_t *node, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt);
96-
rb_iseq_t *pm_iseq_new_eval(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth);
97-
rb_iseq_t *pm_iseq_new_with_opt(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth, enum rb_iseq_type, const rb_compile_option_t*);
93+
rb_iseq_t *pm_iseq_new(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type, int *error_state);
94+
rb_iseq_t *pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, int *error_state);
95+
rb_iseq_t *pm_iseq_new_main(pm_scope_node_t *node, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt, int *error_state);
96+
rb_iseq_t *pm_iseq_new_eval(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth, int *error_state);
97+
rb_iseq_t *pm_iseq_new_with_opt(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth, enum rb_iseq_type, const rb_compile_option_t *option, int *error_state);
9898

9999
VALUE pm_iseq_compile_node(rb_iseq_t *iseq, pm_scope_node_t *node);

‎ruby.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2609,8 +2609,15 @@ process_options(int argc, char **argv, ruby_cmdline_options_t *opt)
26092609

26102610
if (!result.ast) {
26112611
pm_parse_result_t *pm = &result.prism;
2612-
iseq = pm_iseq_new_main(&pm->node, opt->script_name, path, parent, optimize);
2612+
int error_state;
2613+
iseq = pm_iseq_new_main(&pm->node, opt->script_name, path, parent, optimize, &error_state);
2614+
26132615
pm_parse_result_free(pm);
2616+
2617+
if (error_state) {
2618+
RUBY_ASSERT(iseq == NULL);
2619+
rb_jump_tag(error_state);
2620+
}
26142621
}
26152622
else {
26162623
rb_ast_t *ast = result.ast;

‎test/ruby/test_eval.rb‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,4 +612,28 @@ def test_return_in_eval_lambda
612612
x = orphan_lambda
613613
assert_equal(:ok, x.call)
614614
end
615+
616+
def test_syntax_error_no_memory_leak
617+
assert_no_memory_leak([], "#{<<~'begin;'}", "#{<<~'end;'}", rss: true)
618+
begin;
619+
100_000.times do
620+
eval("/[/=~s")
621+
rescue SyntaxError
622+
else
623+
raise "Expected SyntaxError to be raised"
624+
end
625+
end;
626+
627+
assert_no_memory_leak([], "#{<<~'begin;'}", "#{<<~'end;'}", rss: true)
628+
begin;
629+
a = 1
630+
631+
100_000.times do
632+
eval("if a in [0, 0] | [0, a]; end")
633+
rescue SyntaxError
634+
else
635+
raise "Expected SyntaxError to be raised"
636+
end
637+
end;
638+
end
615639
end

‎vm_eval.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1766,7 +1766,8 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line,
17661766
iseq = ISEQ_BODY(iseq)->parent_iseq;
17671767
}
17681768

1769-
iseq = pm_iseq_new_eval(&result.node, name, fname, Qnil, line, parent, 0);
1769+
int error_state;
1770+
iseq = pm_iseq_new_eval(&result.node, name, fname, Qnil, line, parent, 0, &error_state);
17701771

17711772
pm_scope_node_t *prev = result.node.previous;
17721773
while (prev) {
@@ -1778,6 +1779,13 @@ pm_eval_make_iseq(VALUE src, VALUE fname, int line,
17781779
}
17791780

17801781
pm_parse_result_free(&result);
1782+
1783+
// If there was an error, raise it after memory has been cleaned up
1784+
if (error_state) {
1785+
RUBY_ASSERT(iseq == NULL);
1786+
rb_jump_tag(error_state);
1787+
}
1788+
17811789
rb_exec_event_hook_script_compiled(GET_EC(), iseq, src);
17821790

17831791
return iseq;

0 commit comments

Comments
 (0)