Skip to content

Commit

Permalink
Change the first argument of rb_iseq_new functions to be `rb_ast_bo…
Browse files Browse the repository at this point in the history 
…dy_t *`

Some functions like `new_child_iseq` need `VALUE vast` because
`rb_iseq_new` functions first argument is `VALUE vast`.
However `new_child_iseq` doesn’t have `rb_ast_t *ast`
then it needs to allocate `rb_ast_t` by `rb_ruby_ast_new`.
This is the cause of memory leak which e3bfd25 tried to resolve.

As a first step to solve the problem, this commit change the first argument of
`rb_iseq_new` functions so that there is no need to allocate `VALUE vast`
for `new_child_iseq` functions.
  • Loading branch information
@yui-knk
yui-knk committed Apr 30, 2024
1 parent 528c450 commit 9abad3b
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 57 deletions.
10 changes: 6 additions & 4 deletions compile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1479,11 +1479,12 @@ new_child_iseq(rb_iseq_t *iseq, const NODE *const node,
VALUE name, const rb_iseq_t *parent, enum rb_iseq_type type, int line_no)
{
rb_iseq_t *ret_iseq;
VALUE vast = rb_ruby_ast_new(node);
rb_ast_body_t body;
rb_ast_body_initialize(&body, node);

debugs("[new_child_iseq]> ---------------------------------------\n");
int isolated_depth = ISEQ_COMPILE_DATA(iseq)->isolated_depth;
ret_iseq = rb_iseq_new_with_opt(vast, name,
ret_iseq = rb_iseq_new_with_opt(&body, name,
rb_iseq_path(iseq), rb_iseq_realpath(iseq),
line_no, parent,
isolated_depth ? isolated_depth + 1 : 0,
Expand Down Expand Up @@ -8771,10 +8772,11 @@ compile_builtin_mandatory_only_method(rb_iseq_t *iseq, const NODE *node, const N
scope_node.nd_body = mandatory_node(iseq, node);
scope_node.nd_args = &args_node;

VALUE vast = rb_ruby_ast_new(RNODE(&scope_node));
rb_ast_body_t body;
rb_ast_body_initialize(&body, RNODE(&scope_node));

ISEQ_BODY(iseq)->mandatory_only_iseq =
rb_iseq_new_with_opt(vast, rb_iseq_base_label(iseq),
rb_iseq_new_with_opt(&body, rb_iseq_base_label(iseq),
rb_iseq_path(iseq), rb_iseq_realpath(iseq),
nd_line(line_node), NULL, 0,
ISEQ_TYPE_METHOD, ISEQ_COMPILE_DATA(iseq)->option,
Expand Down
2 changes: 1 addition & 1 deletion internal/ruby_parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ enum lex_state_e {
EXPR_NONE = 0
};

VALUE rb_ruby_ast_new(const NODE *const root);
void rb_ast_body_initialize(rb_ast_body_t *body, const NODE *const root);
rb_ast_t *rb_ruby_ast_data_get(VALUE vast);

#endif /* INTERNAL_RUBY_PARSE_H */
39 changes: 15 additions & 24 deletions iseq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -839,21 +839,14 @@ make_compile_option_value(rb_compile_option_t *option)
}

rb_iseq_t *
rb_iseq_new(const VALUE vast, VALUE name, VALUE path, VALUE realpath,
rb_iseq_new(const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath,
const rb_iseq_t *parent, enum rb_iseq_type type)
{
return rb_iseq_new_with_opt(vast, name, path, realpath, 0, parent,
return rb_iseq_new_with_opt(body, name, path, realpath, 0, parent,
0, type, &COMPILE_OPTION_DEFAULT,
Qnil);
}

static int
ast_line_count(const VALUE vast)
{
rb_ast_t *ast = rb_ruby_ast_data_get(vast);
return ast->body.line_count;
}

static VALUE
iseq_setup_coverage(VALUE coverages, VALUE path, int line_count)
{
Expand All @@ -880,11 +873,11 @@ iseq_new_setup_coverage(VALUE path, int line_count)
}

rb_iseq_t *
rb_iseq_new_top(const VALUE vast, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent)
rb_iseq_new_top(const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent)
{
iseq_new_setup_coverage(path, ast_line_count(vast));
iseq_new_setup_coverage(path, body->line_count);

return rb_iseq_new_with_opt(vast, name, path, realpath, 0, parent, 0,
return rb_iseq_new_with_opt(body, name, path, realpath, 0, parent, 0,
ISEQ_TYPE_TOP, &COMPILE_OPTION_DEFAULT,
Qnil);
}
Expand All @@ -902,11 +895,11 @@ pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, c
}

rb_iseq_t *
rb_iseq_new_main(const VALUE vast, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt)
rb_iseq_new_main(const rb_ast_body_t *body, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt)
{
iseq_new_setup_coverage(path, ast_line_count(vast));
iseq_new_setup_coverage(path, body->line_count);

return rb_iseq_new_with_opt(vast, rb_fstring_lit("<main>"),
return rb_iseq_new_with_opt(body, rb_fstring_lit("<main>"),
path, realpath, 0,
parent, 0, ISEQ_TYPE_MAIN, opt ? &COMPILE_OPTION_DEFAULT : &COMPILE_OPTION_FALSE,
Qnil);
Expand All @@ -927,16 +920,16 @@ pm_iseq_new_main(pm_scope_node_t *node, VALUE path, VALUE realpath, const rb_ise
}

rb_iseq_t *
rb_iseq_new_eval(const VALUE vast, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth)
rb_iseq_new_eval(const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth)
{
if (rb_get_coverage_mode() & COVERAGE_TARGET_EVAL) {
VALUE coverages = rb_get_coverages();
if (RTEST(coverages) && RTEST(path) && !RTEST(rb_hash_has_key(coverages, path))) {
iseq_setup_coverage(coverages, path, ast_line_count(vast) + first_lineno - 1);
iseq_setup_coverage(coverages, path, body->line_count + first_lineno - 1);
}
}

return rb_iseq_new_with_opt(vast, name, path, realpath, first_lineno,
return rb_iseq_new_with_opt(body, name, path, realpath, first_lineno,
parent, isolated_depth, ISEQ_TYPE_EVAL, &COMPILE_OPTION_DEFAULT,
Qnil);
}
Expand Down Expand Up @@ -971,13 +964,11 @@ iseq_translate(rb_iseq_t *iseq)
}

rb_iseq_t *
rb_iseq_new_with_opt(const VALUE vast, VALUE name, VALUE path, VALUE realpath,
rb_iseq_new_with_opt(const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath,
int first_lineno, const rb_iseq_t *parent, int isolated_depth,
enum rb_iseq_type type, const rb_compile_option_t *option,
VALUE script_lines)
{
rb_ast_t *ast = rb_ruby_ast_data_get(vast);
rb_ast_body_t *body = ast ? &ast->body : NULL;
const NODE *node = body ? body->root : 0;
/* TODO: argument check */
rb_iseq_t *iseq = iseq_alloc();
Expand Down Expand Up @@ -1234,7 +1225,7 @@ rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V
}
{
const VALUE parser = rb_parser_new();
const rb_iseq_t *outer_scope = rb_iseq_new(Qnil, name, name, Qnil, 0, ISEQ_TYPE_TOP);
const rb_iseq_t *outer_scope = rb_iseq_new(NULL, name, name, Qnil, 0, ISEQ_TYPE_TOP);
VALUE outer_scope_v = (VALUE)outer_scope;
rb_parser_set_context(parser, outer_scope, FALSE);
if (ruby_vm_keep_script_lines) rb_parser_set_script_lines(parser);
Expand All @@ -1249,7 +1240,7 @@ rb_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V
rb_exc_raise(GET_EC()->errinfo);
}
else {
iseq = rb_iseq_new_with_opt(vast, name, file, realpath, ln,
iseq = rb_iseq_new_with_opt(&ast->body, name, file, realpath, ln,
NULL, 0, ISEQ_TYPE_TOP, &option,
Qnil);
rb_ast_dispose(ast);
Expand Down Expand Up @@ -1641,7 +1632,7 @@ iseqw_s_compile_file(int argc, VALUE *argv, VALUE self)

make_compile_option(&option, opt);

ret = iseqw_new(rb_iseq_new_with_opt(vast, rb_fstring_lit("<main>"),
ret = iseqw_new(rb_iseq_new_with_opt(&ast->body, rb_fstring_lit("<main>"),
file,
rb_realpath_internal(Qnil, file, 1),
1, NULL, 0, ISEQ_TYPE_TOP, &option,
Expand Down
2 changes: 1 addition & 1 deletion load.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ load_iseq_eval(rb_execution_context_t *ec, VALUE fname)
vast = rb_parser_load_file(parser, fname);
ast = rb_ruby_ast_data_get(vast);

iseq = rb_iseq_new_top(vast, rb_fstring_lit("<top (required)>"),
iseq = rb_iseq_new_top(&ast->body, rb_fstring_lit("<top (required)>"),
fname, realpath_internal_cached(realpath_map, fname), NULL);
rb_ast_dispose(ast);
}
Expand Down
2 changes: 1 addition & 1 deletion mini_builtin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ builtin_iseq_load(const char *feature_name, const struct rb_builtin_function *ta
.debug_level = 0,
};
ast = rb_ruby_ast_data_get(vast);
const rb_iseq_t *iseq = rb_iseq_new_with_opt(vast, name_str, name_str, Qnil, 0, NULL, 0, ISEQ_TYPE_TOP, &optimization, Qnil);
const rb_iseq_t *iseq = rb_iseq_new_with_opt(&ast->body, name_str, name_str, Qnil, 0, NULL, 0, ISEQ_TYPE_TOP, &optimization, Qnil);
GET_VM()->builtin_function_table = NULL;

rb_ast_dispose(ast);
Expand Down
2 changes: 1 addition & 1 deletion proc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3467,7 +3467,7 @@ proc_binding(VALUE self)
env = VM_ENV_ENVVAL_PTR(block->as.captured.ep);
env = env_clone(env, method_cref(method));
/* set empty iseq */
empty = rb_iseq_new(Qnil, name, name, Qnil, 0, ISEQ_TYPE_TOP);
empty = rb_iseq_new(NULL, name, name, Qnil, 0, ISEQ_TYPE_TOP);
RB_OBJ_WRITE(env, &env->iseq, empty);
break;
}
Expand Down
2 changes: 1 addition & 1 deletion ruby.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2557,7 +2557,7 @@ process_options(int argc, char **argv, ruby_cmdline_options_t *opt)
}
else {
rb_ast_t *ast = result.ast;
iseq = rb_iseq_new_main(vast, opt->script_name, path, parent, optimize);
iseq = rb_iseq_new_main(&ast->body, opt->script_name, path, parent, optimize);
rb_ast_dispose(ast);
}
}
Expand Down
21 changes: 8 additions & 13 deletions ruby_parser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,19 +1133,14 @@ parser_aset_script_lines_for(VALUE path, rb_parser_ary_t *lines)
rb_hash_aset(hash, path, script_lines);
}

VALUE
rb_ruby_ast_new(const NODE *const root)
{
rb_ast_t *ast = ruby_xcalloc(1, sizeof(rb_ast_t));
VALUE vast = ast_alloc(ast);
ast->body = (rb_ast_body_t){
.root = root,
.frozen_string_literal = -1,
.coverage_enabled = -1,
.script_lines = NULL,
.line_count = 0,
};
return vast;
void
rb_ast_body_initialize(rb_ast_body_t *body, const NODE *const root)
{
body->root = root;
body->frozen_string_literal = -1;
body->coverage_enabled = -1;
body->script_lines = NULL;
body->line_count = 0;
}

rb_ast_t *
Expand Down
11 changes: 6 additions & 5 deletions vm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,7 @@ rb_binding_add_dynavars(VALUE bindval, rb_binding_t *bind, int dyncount, const I
rb_execution_context_t *ec = GET_EC();
const rb_iseq_t *base_iseq, *iseq;
rb_node_scope_t tmp_node;
rb_ast_body_t body;

if (dyncount < 0) return 0;

Expand All @@ -1480,14 +1481,14 @@ rb_binding_add_dynavars(VALUE bindval, rb_binding_t *bind, int dyncount, const I
tmp_node.nd_body = 0;
tmp_node.nd_args = 0;

VALUE vast = rb_ruby_ast_new(RNODE(&tmp_node));
rb_ast_body_initialize(&body, RNODE(&tmp_node));

if (base_iseq) {
iseq = rb_iseq_new(vast, ISEQ_BODY(base_iseq)->location.label, path, realpath, base_iseq, ISEQ_TYPE_EVAL);
iseq = rb_iseq_new(&body, ISEQ_BODY(base_iseq)->location.label, path, realpath, base_iseq, ISEQ_TYPE_EVAL);
}
else {
VALUE tempstr = rb_fstring_lit("<temp>");
iseq = rb_iseq_new_top(vast, tempstr, tempstr, tempstr, NULL);
iseq = rb_iseq_new_top(&body, tempstr, tempstr, tempstr, NULL);
}
tmp_node.nd_tbl = 0; /* reset table */
ALLOCV_END(idtmp);
Expand Down Expand Up @@ -2860,7 +2861,7 @@ rb_vm_call_cfunc(VALUE recv, VALUE (*func)(VALUE), VALUE arg,
{
rb_execution_context_t *ec = GET_EC();
const rb_control_frame_t *reg_cfp = ec->cfp;
const rb_iseq_t *iseq = rb_iseq_new(Qnil, filename, filename, Qnil, 0, ISEQ_TYPE_TOP);
const rb_iseq_t *iseq = rb_iseq_new(NULL, filename, filename, Qnil, 0, ISEQ_TYPE_TOP);
VALUE val;

vm_push_frame(ec, iseq, VM_FRAME_MAGIC_TOP | VM_ENV_FLAG_LOCAL | VM_FRAME_FLAG_FINISH,
Expand Down Expand Up @@ -4175,7 +4176,7 @@ Init_VM(void)
rb_vm_t *vm = ruby_current_vm_ptr;
rb_thread_t *th = GET_THREAD();
VALUE filename = rb_fstring_lit("<main>");
const rb_iseq_t *iseq = rb_iseq_new(Qnil, filename, filename, Qnil, 0, ISEQ_TYPE_TOP);
const rb_iseq_t *iseq = rb_iseq_new(NULL, filename, filename, Qnil, 0, ISEQ_TYPE_TOP);

// Ractor setup
rb_ractor_main_setup(vm, th->ractor, th);
Expand Down
10 changes: 5 additions & 5 deletions vm_core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1209,11 +1209,11 @@ typedef enum {
RUBY_SYMBOL_EXPORT_BEGIN

/* node -> iseq */
rb_iseq_t *rb_iseq_new (const VALUE vast, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type);
rb_iseq_t *rb_iseq_new_top (const VALUE vast, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent);
rb_iseq_t *rb_iseq_new_main (const VALUE vast, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt);
rb_iseq_t *rb_iseq_new_eval (const VALUE vast, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth);
rb_iseq_t *rb_iseq_new_with_opt(const VALUE vast, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth,
rb_iseq_t *rb_iseq_new (const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type);
rb_iseq_t *rb_iseq_new_top (const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent);
rb_iseq_t *rb_iseq_new_main (const rb_ast_body_t *body, VALUE path, VALUE realpath, const rb_iseq_t *parent, int opt);
rb_iseq_t *rb_iseq_new_eval (const rb_ast_body_t *body, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_iseq_t *parent, int isolated_depth);
rb_iseq_t *rb_iseq_new_with_opt(const rb_ast_body_t *body, 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*,
VALUE script_lines);

Expand Down
2 changes: 1 addition & 1 deletion vm_eval.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ eval_make_iseq(VALUE src, VALUE fname, int line,

if (ast->body.root) {
ast->body.coverage_enabled = coverage_enabled;
iseq = rb_iseq_new_eval(vast,
iseq = rb_iseq_new_eval(&ast->body,
ISEQ_BODY(parent)->location.label,
fname, Qnil, line,
parent, isolated_depth);
Expand Down

0 comments on commit 9abad3b

Please sign in to comment.