Skip to content

Commit

Permalink
Ensure syntax error locations appear in backtraces (#31881)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@c42f
c42f authored May 2, 2019
1 parent 7e1eb6a commit 5b637df
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 11 deletions.
2 changes: 2 additions & 0 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/rtutils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: ");
Expand Down
36 changes: 27 additions & 9 deletions src/toplevel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "uv.h"
#include "julia_assert.h"
#include "intrinsics.h"
#include "builtin_proto.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -568,6 +569,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_builtin_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();
Expand All @@ -585,7 +602,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);
}
Expand All @@ -594,7 +611,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
Expand All @@ -604,7 +621,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;
Expand Down Expand Up @@ -646,7 +663,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) {
Expand All @@ -657,7 +675,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();
Expand All @@ -684,7 +702,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();
Expand All @@ -694,7 +712,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();
Expand Down Expand Up @@ -750,9 +768,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)) {
Expand Down
31 changes: 31 additions & 0 deletions test/backtrace.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 comments on commit 5b637df

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.