Skip to content

Commit

Permalink
print a warning when a module is unlikely to be successful at increme…
Browse files Browse the repository at this point in the history
…ntal compilation (close #12352)

calling eval into another module during compile-time is almost guaranteed to break incremental compilation

the usual symptom would be a segfault during deserialization due to the
existance of a type, module, or other data structure in the 'other'
module that will not be persisted.
it is also possible to 'lose' method definitions because their
lambda_info->module field will be assigned incorrectly
  • Loading branch information
vtjnash committed Jul 31, 2015
1 parent f278077 commit 2149008
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 41 deletions.
2 changes: 1 addition & 1 deletion base/serialize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ function deserialize(s::SerializationState, ::Type{LambdaStaticData})
linfo = known_lambda_data[lnumber]::LambdaStaticData
makenew = false
else
linfo = ccall(:jl_new_lambda_info, Any, (Ptr{Void}, Ptr{Void}), C_NULL, C_NULL)::LambdaStaticData
linfo = ccall(:jl_new_lambda_info, Any, (Ptr{Void}, Ptr{Void}, Ptr{Void}), C_NULL, C_NULL, C_NULL)::LambdaStaticData
makenew = true
end
deserialize_cycle(s, linfo)
Expand Down
4 changes: 2 additions & 2 deletions src/alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ DLLEXPORT jl_function_t *jl_new_closure(jl_fptr_t fptr, jl_value_t *env,
}

DLLEXPORT
jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams)
jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams, jl_module_t *ctx)
{
jl_lambda_info_t *li =
(jl_lambda_info_t*)newobj((jl_value_t*)jl_lambda_info_type,
Expand All @@ -339,7 +339,7 @@ jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams)
li->line = jl_unbox_long(jl_exprarg(body1, 0));
}
}
li->module = jl_current_module;
li->module = ctx;
li->sparams = sparams;
li->tfunc = jl_nothing;
li->fptr = &jl_trampoline;
Expand Down
53 changes: 29 additions & 24 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static value_t fl_error_sym;
static value_t fl_null_sym;
static value_t fl_jlgensym_sym;

static jl_value_t *scm_to_julia(value_t e, int expronly);
static jl_value_t *scm_to_julia(value_t e, int expronly, jl_module_t *mod);
static value_t julia_to_scm(jl_value_t *v);

value_t fl_defined_julia_global(value_t *args, uint32_t nargs)
Expand Down Expand Up @@ -68,11 +68,11 @@ value_t fl_invoke_julia_macro(value_t *args, uint32_t nargs)
jl_value_t **margs;
JL_GC_PUSHARGS(margs, nargs);
int i;
for(i=1; i < nargs; i++) margs[i] = scm_to_julia(args[i], 1);
for(i=1; i < nargs; i++) margs[i] = scm_to_julia(args[i], 1, jl_current_module);
jl_value_t *result = NULL;

JL_TRY {
margs[0] = scm_to_julia(args[0], 1);
margs[0] = scm_to_julia(args[0], 1, jl_current_module);
f = (jl_function_t*)jl_toplevel_eval(margs[0]);
assert(jl_is_func(f));
result = jl_apply(f, &margs[1], nargs-1);
Expand Down Expand Up @@ -168,7 +168,7 @@ static jl_sym_t *scmsym_to_julia(value_t s)
return jl_symbol(symbol_name(s));
}

static jl_value_t *scm_to_julia_(value_t e, int expronly);
static jl_value_t *scm_to_julia_(value_t e, int expronly, jl_module_t *mod);

static jl_value_t *full_list(value_t e, int expronly)
{
Expand All @@ -177,7 +177,7 @@ static jl_value_t *full_list(value_t e, int expronly)
jl_array_t *ar = jl_alloc_cell_1d(ln);
size_t i=0;
while (iscons(e)) {
jl_cellset(ar, i, scm_to_julia_(car_(e), expronly));
jl_cellset(ar, i, scm_to_julia_(car_(e), expronly, jl_current_module));
e = cdr_(e);
i++;
}
Expand All @@ -200,12 +200,12 @@ static jl_value_t *full_list_of_lists(value_t e, int expronly)

static jl_value_t *resolve_globals(jl_value_t *expr, jl_lambda_info_t *lam);

static jl_value_t *scm_to_julia(value_t e, int expronly)
static jl_value_t *scm_to_julia(value_t e, int expronly, jl_module_t *mod)
{
int en = jl_gc_enable(0);
jl_value_t *v;
JL_TRY {
v = scm_to_julia_(e, expronly);
v = scm_to_julia_(e, expronly, mod);
}
JL_CATCH {
// if expression cannot be converted, replace with error expr
Expand All @@ -219,7 +219,7 @@ static jl_value_t *scm_to_julia(value_t e, int expronly)

extern int64_t conv_to_int64(void *data, numerictype_t tag);

static jl_value_t *scm_to_julia_(value_t e, int eo)
static jl_value_t *scm_to_julia_(value_t e, int eo, jl_module_t *mod)
{
if (fl_isnumber(e)) {
int64_t i64;
Expand Down Expand Up @@ -316,10 +316,10 @@ static jl_value_t *scm_to_julia_(value_t e, int eo)

for(i=2; i < n; i++) {
assert(iscons(e));
jl_cellset(ex->args, i, scm_to_julia_(car_(e), eo));
jl_cellset(ex->args, i, scm_to_julia_(car_(e), eo, mod));
e = cdr_(e);
}
jl_lambda_info_t *nli = jl_new_lambda_info((jl_value_t*)ex, jl_emptysvec);
jl_lambda_info_t *nli = jl_new_lambda_info((jl_value_t*)ex, jl_emptysvec, mod);
resolve_globals(nli->ast, nli);
return (jl_value_t*)nli;
}
Expand All @@ -328,27 +328,27 @@ static jl_value_t *scm_to_julia_(value_t e, int eo)
if (!eo) {
if (sym == line_sym && n==1) {
return jl_new_struct(jl_linenumbernode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
if (sym == label_sym) {
return jl_new_struct(jl_labelnode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
if (sym == goto_sym) {
return jl_new_struct(jl_gotonode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
if (sym == inert_sym || (sym == quote_sym && (!iscons(car_(e))))) {
return jl_new_struct(jl_quotenode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
if (sym == top_sym) {
return jl_new_struct(jl_topnode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
if (sym == newvar_sym) {
return jl_new_struct(jl_newvarnode_type,
scm_to_julia_(car_(e),0));
scm_to_julia_(car_(e),0,mod));
}
}
else if (sym == inert_sym && !iscons(car_(e))) {
Expand All @@ -360,7 +360,7 @@ static jl_value_t *scm_to_julia_(value_t e, int eo)
ex->args = jl_alloc_cell_1d(0);
for(i=0; i < n; i++) {
assert(iscons(e));
jl_cellset(ex->args, i, scm_to_julia_(car_(e),eo));
jl_cellset(ex->args, i, scm_to_julia_(car_(e),eo,mod));
e = cdr_(e);
}
return (jl_value_t*)ex;
Expand Down Expand Up @@ -485,7 +485,7 @@ DLLEXPORT jl_value_t *jl_parse_input_line(const char *str, size_t len)
value_t e = fl_applyn(1, symbol_value(symbol("jl-parse-string")), s);
if (e == FL_EOF)
return jl_nothing;
return scm_to_julia(e,0);
return scm_to_julia(e,0,jl_current_module);
}

// this is for parsing one expression out of a string, keeping track of
Expand All @@ -503,7 +503,7 @@ DLLEXPORT jl_value_t *jl_parse_string(const char *str, size_t len,
if (e == FL_EOF)
expr = jl_nothing;
else
expr = scm_to_julia(e,0);
expr = scm_to_julia(e,0,jl_current_module);

pos1 = jl_box_long(tosize(cdr_(p),"parse"));
jl_value_t *result = (jl_value_t*)jl_svec2(expr, pos1);
Expand Down Expand Up @@ -558,7 +558,7 @@ jl_value_t *jl_parse_next(void)
// for error, get most recent line number
if (iscons(c) && car_(c) == fl_error_sym)
jl_lineno = numval(fl_applyn(0, symbol_value(symbol("jl-parser-current-lineno"))));
return scm_to_julia(c,0);
return scm_to_julia(c,0,jl_current_module);
}

jl_value_t *jl_load_file_string(const char *text, size_t len,
Expand All @@ -574,25 +574,30 @@ jl_value_t *jl_load_file_string(const char *text, size_t len,
}

// returns either an expression or a thunk
jl_value_t *jl_expand(jl_value_t *expr)
DLLEXPORT jl_value_t *jl_expand_in(jl_module_t *mod, jl_value_t *expr)
{
int np = jl_gc_n_preserved_values();
value_t arg = julia_to_scm(expr);
value_t e = fl_applyn(1, symbol_value(symbol("jl-expand-to-thunk")), arg);
jl_value_t *result = scm_to_julia(e,0);
jl_value_t *result = scm_to_julia(e,0,mod);
while (jl_gc_n_preserved_values() > np) {
jl_gc_unpreserve();
}
return result;
}

DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr)
{
return jl_expand_in(jl_current_module, expr);
}

DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr)
{
int np = jl_gc_n_preserved_values();
value_t arg = julia_to_scm(expr);
value_t e = fl_applyn(1, symbol_value(symbol("jl-macroexpand")), arg);
jl_value_t *result;
result = scm_to_julia(e,0);
result = scm_to_julia(e,0,jl_current_module);
while (jl_gc_n_preserved_values() > np) {
jl_gc_unpreserve();
}
Expand Down Expand Up @@ -639,7 +644,7 @@ jl_lambda_info_t *jl_wrap_expr(jl_value_t *expr)
expr = (jl_value_t*)bo;
}
jl_cellset(le->args, 2, expr);
jl_lambda_info_t *li = jl_new_lambda_info((jl_value_t*)le, jl_emptysvec);
jl_lambda_info_t *li = jl_new_lambda_info((jl_value_t*)le, jl_emptysvec, jl_current_module);
JL_GC_POP();
return li;
}
Expand Down
5 changes: 5 additions & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,11 @@ DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
int last_lineno = jl_lineno;
jl_module_t *last_m = jl_current_module;
jl_module_t *task_last_m = jl_current_task->current_module;
if (m != last_m && jl_generating_output() && jl_options.incremental) {
jl_printf(JL_STDERR, "WARNING: eval from module %s to %s: \n", m->name->name, last_m->name->name);
jl_static_show(JL_STDERR, ex);
jl_printf(JL_STDERR, "\n ** incremental compilation may be broken for this module **\n\n");
}
JL_TRY {
jl_current_task->current_module = jl_current_module = m;
v = jl_toplevel_eval(ex);
Expand Down
9 changes: 5 additions & 4 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,9 @@ jl_lambda_info_t *jl_add_static_parameters(jl_lambda_info_t *l, jl_svec_t *sp)
JL_GC_PUSH1(&sp);
if (jl_svec_len(l->sparams) > 0)
sp = jl_svec_append(sp, l->sparams);
jl_lambda_info_t *nli = jl_new_lambda_info(l->ast, sp);
jl_lambda_info_t *nli = jl_new_lambda_info(l->ast, sp, l->module);
nli->name = l->name;
nli->fptr = l->fptr;
nli->module = l->module;
nli->file = l->file;
nli->line = l->line;
nli->def = l->def;
Expand Down Expand Up @@ -937,8 +936,10 @@ DLLEXPORT jl_function_t *jl_instantiate_staged(jl_methlist_t *m, jl_tupletype_t
}
ex = oldast;
}
func = (jl_function_t*)jl_toplevel_eval_in(m->func->linfo->module, (jl_value_t*)ex);
func->linfo->name = m->func->linfo->name;
jl_lambda_info_t *li = (jl_lambda_info_t*)jl_expand_in(m->func->linfo->module, (jl_value_t*)ex);
assert(jl_is_lambda_info(li));
li->name = m->func->linfo->name;
func = jl_new_closure(NULL, (jl_value_t*)jl_emptysvec, li);
JL_GC_POP();
return func;
}
Expand Down
8 changes: 1 addition & 7 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,12 @@
(pair? (cadr e)) (eq? (caadr e) '=) (symbol? (cadadr e))
(eq? (cadr (caddr e)) (cadadr e))))

(define (lambda-ex? e)
(and (pair? e) (eq? (car e) 'lambda)))

(define (expand-toplevel-expr- e)
(let ((ex (expand-toplevel-expr-- e)))
(cond ((contains (lambda (x) (equal? x '(top ccall))) ex) ex)
((simple-assignment? ex) (cadr ex))
((and (length= ex 2) (eq? (car ex) 'body)
(not (lambda-ex? (cadadr ex))))
((and (length= ex 2) (eq? (car ex) 'body))
;; (body (return x)) => x
;; if x is not a lambda expr, so we don't think it is a thunk
;; to be called immediately.
(cadadr ex))
(else ex))))

Expand Down
3 changes: 2 additions & 1 deletion src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ DLLEXPORT jl_value_t *jl_new_structv(jl_datatype_t *type, jl_value_t **args, uin
DLLEXPORT jl_value_t *jl_new_struct_uninit(jl_datatype_t *type);
DLLEXPORT jl_function_t *jl_new_closure(jl_fptr_t proc, jl_value_t *env,
jl_lambda_info_t *li);
DLLEXPORT jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams);
DLLEXPORT jl_lambda_info_t *jl_new_lambda_info(jl_value_t *ast, jl_svec_t *sparams, jl_module_t *ctx);
DLLEXPORT jl_svec_t *jl_svec(size_t n, ...);
DLLEXPORT jl_svec_t *jl_svec1(void *a);
DLLEXPORT jl_svec_t *jl_svec2(void *a, void *b);
Expand Down Expand Up @@ -1149,6 +1149,7 @@ jl_value_t *jl_parse_next(void);
DLLEXPORT jl_value_t *jl_load_file_string(const char *text, size_t len,
char *filename, size_t namelen);
DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr);
DLLEXPORT jl_value_t *jl_expand_in(jl_module_t *module, jl_value_t *expr);
jl_lambda_info_t *jl_wrap_expr(jl_value_t *expr);
DLLEXPORT void *jl_eval_string(const char *str);

Expand Down
5 changes: 3 additions & 2 deletions src/toplevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ jl_module_t *jl_new_main_module(void)

jl_main_module = jl_new_module(jl_symbol("Main"));
jl_main_module->parent = jl_main_module;
if (old_main) // don't block continued loading of incremental caches
jl_main_module->uuid = old_main->uuid;
jl_current_module = jl_main_module;

jl_core_module->parent = jl_main_module;
Expand Down Expand Up @@ -700,14 +702,13 @@ DLLEXPORT jl_value_t *jl_generic_function_def(jl_sym_t *name, jl_value_t **bp, j
static jl_lambda_info_t *jl_copy_lambda_info(jl_lambda_info_t *linfo)
{
jl_lambda_info_t *new_linfo =
jl_new_lambda_info(linfo->ast, linfo->sparams);
jl_new_lambda_info(linfo->ast, linfo->sparams, linfo->module);
new_linfo->tfunc = linfo->tfunc;
new_linfo->name = linfo->name;
new_linfo->roots = linfo->roots;
new_linfo->specTypes = linfo->specTypes;
new_linfo->unspecialized = linfo->unspecialized;
new_linfo->specializations = linfo->specializations;
new_linfo->module = linfo->module;
new_linfo->def = linfo->def;
new_linfo->capt = linfo->capt;
new_linfo->file = linfo->file;
Expand Down

0 comments on commit 2149008

Please sign in to comment.