From 0ca9146462066500b9329f57f98f5f3db5621be5 Mon Sep 17 00:00:00 2001 From: Chris Foster Date: Tue, 30 Apr 2019 14:12:29 +1000 Subject: [PATCH] Ensure syntax error locations appear in backtraces Errors which are thrown directly inside jl_toplevel_eval_flex do not get an entry in the backtrace. The most prominent of these are syntax errors, but there's other cases too. Fix this by constructing a julia expression throwing the appropriate error, and evaling it to generate a julia-level frame. --- src/julia.h | 2 ++ src/rtutils.c | 4 ++-- src/toplevel.c | 35 ++++++++++++++++++++++++++--------- test/backtrace.jl | 31 +++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/julia.h b/src/julia.h index 0d4f3855c43e0..25b947d81cfec 100644 --- a/src/julia.h +++ b/src/julia.h @@ -1430,6 +1430,8 @@ JL_DLLEXPORT jl_sym_t *jl_get_ARCH(void); JL_DLLEXPORT jl_value_t *jl_environ(int i); // throwing common exceptions +JL_DLLEXPORT jl_value_t *jl_vexceptionf(jl_datatype_t *exception_type, + const char *fmt, va_list args); JL_DLLEXPORT void JL_NORETURN jl_error(const char *str); JL_DLLEXPORT void JL_NORETURN jl_errorf(const char *fmt, ...); JL_DLLEXPORT void JL_NORETURN jl_exceptionf(jl_datatype_t *ty, diff --git a/src/rtutils.c b/src/rtutils.c index ea1eb6b2ea033..2275dbb7bf71b 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -43,8 +43,8 @@ JL_DLLEXPORT void JL_NORETURN jl_error(const char *str) extern int vasprintf(char **str, const char *fmt, va_list ap); -static jl_value_t *jl_vexceptionf(jl_datatype_t *exception_type, - const char *fmt, va_list args) +jl_value_t *jl_vexceptionf(jl_datatype_t *exception_type, + const char *fmt, va_list args) { if (exception_type == NULL) { jl_printf(JL_STDERR, "ERROR: "); diff --git a/src/toplevel.c b/src/toplevel.c index 2be53b2980367..7f59ff55384aa 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -568,6 +568,22 @@ static jl_module_t *eval_import_from(jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_ return NULL; } +// Format msg and eval `throw(ErrorException(msg)))` in module `m`. +// Used in `jl_toplevel_eval_flex` instead of `jl_errorf` so that the error +// location in julia code gets into the backtrace. +static void jl_eval_errorf(jl_module_t *m, const char* fmt, ...) +{ + jl_value_t *throw_ex = (jl_value_t*)jl_exprn(call_sym, 2); + JL_GC_PUSH1(&throw_ex); + jl_exprargset(throw_ex, 0, jl_symbol("throw")); + va_list args; + va_start(args, fmt); + jl_exprargset(throw_ex, 1, jl_vexceptionf(jl_errorexception_type, fmt, args)); + va_end(args); + jl_toplevel_eval_flex(m, throw_ex, 0, 0); + JL_GC_POP(); +} + jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int fast, int expanded) { jl_ptls_t ptls = jl_get_ptls_states(); @@ -585,7 +601,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int char *n = jl_symbol_name((jl_sym_t*)e), *n0 = n; while (*n == '_') ++n; if (*n == 0 && n > n0) - jl_error("all-underscore identifier used as rvalue"); + jl_eval_errorf(m, "all-underscore identifier used as rvalue"); } return jl_interpret_toplevel_expr_in(m, e, NULL, NULL); } @@ -594,7 +610,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int if (ex->head == dot_sym) { if (jl_expr_nargs(ex) != 2) - jl_error("syntax: malformed \".\" expression"); + jl_eval_errorf(m, "syntax: malformed \".\" expression"); jl_value_t *lhs = jl_exprarg(ex, 0); jl_value_t *rhs = jl_exprarg(ex, 1); // only handle `a.b` syntax here, so qualified names can be eval'd in pure contexts @@ -604,7 +620,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int } if (ptls->in_pure_callback) { - jl_error("eval cannot be used in a generated function"); + jl_eval_errorf(m, "eval cannot be used in a generated function"); } jl_method_instance_t *li = NULL; @@ -646,7 +662,8 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int } else { if (!jl_is_module(u)) - jl_errorf("invalid using path: \"%s\" does not name a module", jl_symbol_name(name)); + jl_eval_errorf(m, "invalid using path: \"%s\" does not name a module", + jl_symbol_name(name)); // `using A.B` syntax jl_module_using(m, u); if (m == jl_main_module && name == NULL) { @@ -657,7 +674,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int } } else { - jl_error("syntax: malformed \"using\" statement"); + jl_eval_errorf(m, "syntax: malformed \"using\" statement"); } } JL_GC_POP(); @@ -684,7 +701,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int } } else { - jl_error("syntax: malformed \"import\" statement"); + jl_eval_errorf(m, "syntax: malformed \"import\" statement"); } } JL_GC_POP(); @@ -694,7 +711,7 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int for (size_t i = 0; i < jl_array_len(ex->args); i++) { jl_sym_t *name = (jl_sym_t*)jl_array_ptr_ref(ex->args, i); if (!jl_is_symbol(name)) - jl_error("syntax: malformed \"export\" statement"); + jl_eval_errorf(m, "syntax: malformed \"export\" statement"); jl_module_export(m, name); } JL_GC_POP(); @@ -750,9 +767,9 @@ jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_value_t *e, int } else if (head == error_sym || head == jl_incomplete_sym) { if (jl_expr_nargs(ex) == 0) - jl_errorf("malformed \"%s\" expression", jl_symbol_name(head)); + jl_eval_errorf(m, "malformed \"%s\" expression", jl_symbol_name(head)); if (jl_is_string(jl_exprarg(ex, 0))) - jl_errorf("syntax: %s", jl_string_data(jl_exprarg(ex, 0))); + jl_eval_errorf(m, "syntax: %s", jl_string_data(jl_exprarg(ex, 0))); jl_throw(jl_exprarg(ex, 0)); } else if (jl_is_symbol(ex)) { diff --git a/test/backtrace.jl b/test/backtrace.jl index 9c7847ac34741..e63ae84c56181 100644 --- a/test/backtrace.jl +++ b/test/backtrace.jl @@ -189,3 +189,34 @@ let bt, found = false end @test found end + +# Syntax error locations appear in backtraces +let trace = try + include_string(@__MODULE__, + """ + + ) + + """, "a_filename") + catch + stacktrace(Base.catch_stack()[end-1][2]) # Ignore LoadError + end + @test trace[1].func == Symbol("top-level scope") + @test trace[1].file == :a_filename + @test trace[1].line == 2 +end +let trace = try + include_string(@__MODULE__, + """ + + incomplete_syntax( + + """, "a_filename") + catch + stacktrace(Base.catch_stack()[end-1][2]) # Ignore LoadError + end + @test trace[1].func == Symbol("top-level scope") + @test trace[1].file == :a_filename + @test trace[1].line == 2 +end +