Skip to content

Commit

Permalink
Fix many GC related bugs
Browse files Browse the repository at this point in the history
* Make `jl_linenode_line` non-allocating
* Fix missing GC root when calling `jl_fieldref` and `jl_get_nth_field`
  that can possibly allocate
* Add missing write barrier when using heap allocated array as stack buffer
* Remove a few allocation from `jl_static_show_x` (the general case still
  allocate and should be fixed)
* Fix missing and extra `JL_GC_POP`
* Fix misplaced `JL_GC_PUSH*`
  • Loading branch information
yuyichao committed Sep 27, 2015
1 parent 16b1e87 commit 34c9332
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams, jl_mod
if (ast != NULL && jl_is_expr(ast)) {
jl_value_t *body1 = skip_meta(jl_lam_body((jl_expr_t*)ast)->args);
if (jl_is_linenode(body1)) {
li->file = (jl_sym_t*)jl_fieldref(body1, 0);
li->line = jl_unbox_long(jl_fieldref(body1, 1));
li->file = jl_linenode_file(body1);
li->line = jl_linenode_line(body1);
} else if (jl_is_expr(body1) && ((jl_expr_t*)body1)->head == line_sym) {
li->file = (jl_sym_t*)jl_exprarg(body1, 1);
li->line = jl_unbox_long(jl_exprarg(body1, 0));
Expand Down
3 changes: 3 additions & 0 deletions src/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,12 @@ jl_array_t *jl_ptr_to_array(jl_value_t *atype, void *data, jl_value_t *dims,
}
else {
size_t *adims = &a->nrows;
// jl_fieldref can allocate
JL_GC_PUSH1(&a);
for(i=0; i < ndims; i++) {
adims[i] = jl_unbox_long(jl_fieldref(dims, i));
}
JL_GC_POP();
}
return a;
}
Expand Down
5 changes: 5 additions & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,13 +482,18 @@ static value_t julia_to_scm_(jl_value_t *v)
return scmv;
}
if (jl_typeis(v, jl_linenumbernode_type)) {
// GC Note: jl_fieldref(v, 1) allocates but neither jl_fieldref(v, 0)
// or julia_to_list2 should allocate here
value_t args = julia_to_list2(jl_fieldref(v,1), jl_fieldref(v,0));
fl_gc_handle(&args);
value_t hd = julia_to_scm_((jl_value_t*)line_sym);
value_t scmv = fl_cons(hd, args);
fl_free_gc_handles(1);
return scmv;
}
// GC Note: jl_fieldref(v, 0) allocate for LabelNode, GotoNode
// but we don't need a GC root here because julia_to_list2
// shouldn't allocate in this case.
if (jl_typeis(v, jl_labelnode_type))
return julia_to_list2((jl_value_t*)label_sym, jl_fieldref(v,0));
if (jl_typeis(v, jl_gotonode_type))
Expand Down
28 changes: 20 additions & 8 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -452,12 +452,17 @@ JL_CALLABLE(jl_f_apply)
jl_value_t **newargs;
int onstack = (n < jl_page_size/sizeof(jl_value_t*));
JL_GC_PUSHARGS(newargs, onstack ? n : 1);
jl_svec_t *arg_heap = NULL;
if (!onstack) {
// put arguments on the heap if there are too many
jl_value_t *argarr = (jl_value_t*)jl_alloc_cell_1d(n);
newargs[0] = argarr;
newargs = jl_cell_data(argarr);
}
arg_heap = jl_alloc_svec(n);
newargs[0] = (jl_value_t*)arg_heap;
newargs = jl_svec_data(arg_heap);
}
// GC Note: here we assume that the the return value of `jl_svecref`,
// `jl_cellref` will not be young if `arg_heap` becomes old
// since they are allocated before `arg_heap`. Otherwise,
// we need to add write barrier for !onstack
n = 0;
for(i=1; i < nargs; i++) {
if (jl_is_svec(args[i])) {
Expand All @@ -468,8 +473,13 @@ JL_CALLABLE(jl_f_apply)
}
else if (jl_is_tuple(args[i])) {
size_t al = jl_nfields(args[i]);
for(j=0; j < al; j++)
for(j=0; j < al; j++) {
// jl_fieldref may allocate.
newargs[n++] = jl_fieldref(args[i], j);
if (arg_heap) {
jl_gc_wb(arg_heap, newargs[n - 1]);
}
}
}
else {
size_t al = jl_array_len(args[i]);
Expand Down Expand Up @@ -1502,7 +1512,8 @@ size_t jl_static_show_x(JL_STREAM *out, jl_value_t *v, int depth)
n += jl_printf(out, ")");
}
else if (jl_is_linenode(v)) {
n += jl_printf(out, "# line %"PRIuPTR" %s", jl_linenode_line(v), jl_linenode_file(v)->name);
n += jl_printf(out, "# line %"PRIuPTR" %s",
jl_linenode_line(v), jl_linenode_file(v)->name);
}
else if (jl_is_expr(v)) {
jl_expr_t *e = (jl_expr_t*)v;
Expand Down Expand Up @@ -1545,8 +1556,8 @@ size_t jl_static_show_x(JL_STREAM *out, jl_value_t *v, int depth)
else if (jl_typeis(v,jl_loaderror_type)) {
n += jl_printf(out, "LoadError(at ");
n += jl_static_show_x(out, jl_fieldref(v, 0), depth);
n += jl_printf(out, " line ");
n += jl_static_show_x(out, jl_fieldref(v, 1), depth);
// Access the field directly to avoid allocation
n += jl_printf(out, " line %" PRIdPTR, ((intptr_t*)v)[1]);
n += jl_printf(out, ": ");
n += jl_static_show_x(out, jl_fieldref(v, 2), depth);
n += jl_printf(out, ")");
Expand Down Expand Up @@ -1579,6 +1590,7 @@ size_t jl_static_show_x(JL_STREAM *out, jl_value_t *v, int depth)
//jl_fielddesc_t f = t->fields[i];
n += jl_printf(out, "=");
}
// FIXME: this line can allocate
fldval = jl_get_nth_field(v, i);
n += jl_static_show_x(out, fldval, depth);
if (istuple && tlen==1)
Expand Down
2 changes: 1 addition & 1 deletion src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,6 @@ static std::string generate_func_sig(

t = julia_struct_to_llvm(tti);
if (t == NULL || t == T_void) {
JL_GC_POP();
std::stringstream msg;
msg << "ccall: the type of argument ";
msg << i+1;
Expand Down Expand Up @@ -1283,6 +1282,7 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
bool needroot = false;
if (jl_is_abstract_ref_type(jargty)) {
if (addressOf) {
JL_GC_POP();
emit_error("ccall: & on a Ref{T} argument is invalid", ctx);
return jl_cgval_t();
}
Expand Down
4 changes: 2 additions & 2 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -1027,8 +1027,8 @@ void jl_serialize_dependency_list(ios_t *s)
unique_func = jl_get_global(jl_base_module, jl_symbol("unique"));
jl_array_t *udeps = deps && unique_func ? (jl_array_t *) jl_apply((jl_function_t*)unique_func, (jl_value_t**)&deps, 1) : NULL;

JL_GC_PUSH1(&udeps);
if (udeps) {
JL_GC_PUSH1(&udeps);
size_t l = jl_array_len(udeps);
for (size_t i=0; i < l; i++) {
jl_value_t *dep = jl_fieldref(jl_cellref(udeps, i), 0);
Expand All @@ -1051,8 +1051,8 @@ void jl_serialize_dependency_list(ios_t *s)
write_float64(s, jl_unbox_float64(jl_fieldref(deptuple, 1)));
}
write_int32(s, 0); // terminator, for ease of reading
JL_GC_POP();
}
JL_GC_POP();
}

// --- deserialize ---
Expand Down
9 changes: 4 additions & 5 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,9 @@ static jl_value_t *eval(jl_value_t *e, jl_value_t **locals, size_t nl, size_t ng
return (jl_value_t*)jl_nothing;
}

static int label_idx(jl_value_t *tgt, jl_array_t *stmts)
static int label_idx(long ltgt, jl_array_t *stmts)
{
size_t j;
long ltgt = jl_unbox_long(tgt);
for(j=0; j < stmts->nrows; j++) {
jl_value_t *l = jl_cellref(stmts,j);
if (jl_is_labelnode(l) && jl_labelnode_label(l)==ltgt)
Expand Down Expand Up @@ -538,15 +537,15 @@ static jl_value_t *eval_body(jl_array_t *stmts, jl_value_t **locals, size_t nl,
while (1) {
jl_value_t *stmt = jl_cellref(stmts,i);
if (jl_is_gotonode(stmt)) {
i = label_idx(jl_fieldref(stmt,0), stmts);
i = label_idx(jl_gotonode_label(stmt), stmts);
continue;
}
if (jl_is_expr(stmt)) {
jl_sym_t *head = ((jl_expr_t*)stmt)->head;
if (head == goto_ifnot_sym) {
jl_value_t *cond = eval(jl_exprarg(stmt,0), locals, nl, ngensym);
if (cond == jl_false) {
i = label_idx(jl_exprarg(stmt,1), stmts);
i = label_idx(jl_unbox_long(jl_exprarg(stmt, 1)), stmts);
continue;
}
else if (cond != jl_true) {
Expand All @@ -571,7 +570,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, jl_value_t **locals, size_t nl,
if (jl_exception_in_transit == jl_stackovf_exception)
_resetstkoflw();
#endif
i = label_idx(jl_exprarg(stmt,0), stmts);
i = label_idx(jl_unbox_long(jl_exprarg(stmt,0)), stmts);
continue;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,8 @@ static jl_value_t *inst_tuple_w_(jl_value_t *t, jl_value_t **env, size_t n,
for(i=0; i < ntp; i++) {
jl_value_t *elt = jl_svecref(tp, i);
iparams[i] = (jl_value_t*)inst_type_w_(elt, env, n, stack, 0);
if (ip_heap)
jl_gc_wb(ip_heap, iparams[i]);
jl_value_t *pi = iparams[i];
check_tuple_parameter(pi, i, ntp);
if (!isabstract && !jl_is_leaf_type(pi)) {
Expand Down
2 changes: 1 addition & 1 deletion src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ STATIC_INLINE jl_value_t *jl_cellset(void *a, size_t i, void *x)
#define jl_symbolnode_sym(s) ((jl_sym_t*)jl_fieldref(s,0))
#define jl_symbolnode_type(s) (jl_fieldref(s,1))
#define jl_linenode_file(x) ((jl_sym_t*)jl_fieldref(x,0))
#define jl_linenode_line(x) (((ptrint_t*)jl_fieldref(x,1))[0])
#define jl_linenode_line(x) (((ptrint_t*)x)[1])
#define jl_labelnode_label(x) (((ptrint_t*)x)[0])
#define jl_gotonode_label(x) (((ptrint_t*)x)[0])
#define jl_globalref_mod(s) ((jl_module_t*)jl_fieldref(s,0))
Expand Down

0 comments on commit 34c9332

Please sign in to comment.