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 8a1d443
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 21 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
4 changes: 2 additions & 2 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static jl_value_t *scm_to_julia_(value_t e, int eo)
jl_cellset(ex->args, i, scm_to_julia_(car_(e), eo));
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, jl_current_module);
resolve_globals(nli->ast, nli);
return (jl_value_t*)nli;
}
Expand Down Expand Up @@ -639,7 +639,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
21 changes: 19 additions & 2 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,10 @@ JL_CALLABLE(jl_f_kwcall)

extern int jl_lineno;

DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex, int delay_warn)
{
static int jl_warn_on_eval = 0;
int last_delay_warn = jl_warn_on_eval;
if (m == NULL)
m = jl_main_module;
if (jl_is_symbol(ex))
Expand All @@ -547,16 +549,31 @@ 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 (!delay_warn && jl_options.incremental && jl_generating_output()) {
if (m != last_m) {
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");
}
else if (jl_warn_on_eval) {
jl_printf(JL_STDERR, "WARNING: eval from staged function in module %s: \n", m->name->name);
jl_static_show(JL_STDERR, ex);
jl_printf(JL_STDERR, "\n ** incremental compilation may be broken for these modules **\n\n");
}
}
JL_TRY {
jl_warn_on_eval = delay_warn && (jl_warn_on_eval || m != last_m); // compute whether a warning was suppressed
jl_current_task->current_module = jl_current_module = m;
v = jl_toplevel_eval(ex);
}
JL_CATCH {
jl_warn_on_eval = last_delay_warn;
jl_lineno = last_lineno;
jl_current_module = last_m;
jl_current_task->current_module = task_last_m;
jl_rethrow();
}
jl_warn_on_eval = last_delay_warn;
jl_lineno = last_lineno;
jl_current_module = last_m;
jl_current_task->current_module = task_last_m;
Expand All @@ -578,7 +595,7 @@ JL_CALLABLE(jl_f_top_eval)
m = (jl_module_t*)args[0];
ex = args[1];
}
return jl_toplevel_eval_in(m, ex);
return jl_toplevel_eval_in(m, ex, 0);
}

JL_CALLABLE(jl_f_isdefined)
Expand Down
5 changes: 2 additions & 3 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,7 +936,7 @@ 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 = (jl_function_t*)jl_toplevel_eval_in(m->func->linfo->module, (jl_value_t*)ex, 1); // need to eval macros in the right module, but not give a warning for the `eval` call unless that results in a call to `eval`
func->linfo->name = m->func->linfo->name;
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
5 changes: 3 additions & 2 deletions 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 Expand Up @@ -1184,7 +1185,7 @@ DLLEXPORT const char *jl_lookup_soname(const char *pfx, size_t n);
// compiler
void jl_compile(jl_function_t *f);
DLLEXPORT jl_value_t *jl_toplevel_eval(jl_value_t *v);
DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex);
DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex, int delay_warn);
jl_value_t *jl_eval_global_var(jl_module_t *m, jl_sym_t *e);
DLLEXPORT jl_value_t *jl_load(const char *fname, size_t len);
jl_value_t *jl_parse_eval_all(const char *fname, size_t len);
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 8a1d443

Please sign in to comment.