From 87d692a5c71e9e7087435ef609a1b69d8aec7ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 24 Feb 2022 18:44:48 +0100 Subject: [PATCH] Improve eval stacktraces on Erlang/OTP 25+ --- lib/elixir/src/elixir.erl | 98 ++++++++++++++-------------- lib/elixir/test/elixir/code_test.exs | 36 ++++++---- lib/iex/lib/iex/evaluator.ex | 45 ++++++++----- 3 files changed, 97 insertions(+), 82 deletions(-) diff --git a/lib/elixir/src/elixir.erl b/lib/elixir/src/elixir.erl index e8b3c77469e..70b16019d29 100644 --- a/lib/elixir/src/elixir.erl +++ b/lib/elixir/src/elixir.erl @@ -282,7 +282,8 @@ eval_forms(Tree, RawBinding, OrigE) -> end, ErlBinding = elixir_erl_var:load_binding(Binding, E, S), - {value, Value, NewBinding} = recur_eval(Exprs, ErlBinding, NewE), + ExternalHandler = eval_external_handler(NewE), + {value, Value, NewBinding} = erl_eval:exprs(Exprs, ErlBinding, none, ExternalHandler), {Value, elixir_erl_var:dump_binding(NewBinding, NewExS, NewErlS), NewE} end. @@ -300,56 +301,53 @@ normalize_binding([], VarsMap, Normalized, _Counter) -> normalize_pair({Key, Value}) when is_atom(Key) -> {{Key, nil}, Value}; normalize_pair({Pair, Value}) -> {Pair, Value}. -recur_eval([Expr | Exprs], Binding, Env) -> - {value, Value, NewBinding} = - % Below must be all one line for locations to be the same - % when the stacktrace is extended to the full stacktrace. - try erl_eval:expr(Expr, Binding, none, none, none) catch Class:Exception:Stacktrace -> erlang:raise(Class, rewrite_exception(Exception, Stacktrace, Expr, Env), rewrite_stacktrace(Stacktrace)) end, - - case Exprs of - [] -> {value, Value, NewBinding}; - _ -> recur_eval(Exprs, NewBinding, Env) - end. - -rewrite_exception(badarg, [{Mod, _, _, _} | _], Erl, #{file := File}) when Mod == erl_eval; Mod == eval_bits -> - {Min, Max} = - erl_parse:fold_anno(fun(Anno, {Min, Max}) -> - case erl_anno:line(Anno) of - Line when Line > 0 -> {min(Min, Line), max(Max, Line)}; - _ -> {Min, Max} +%% TODO: Remove conditional once we require Erlang/OTP 25+. +-if(?OTP_RELEASE >= 25). +eval_external_handler(Env) -> + Fun = fun(Ann, FunOrModFun, Args) -> + try + case FunOrModFun of + {Mod, Fun} -> apply(Mod, Fun, Args); + Fun -> apply(Fun, Args) end - end, {999999, -999999}, Erl), - - 'Elixir.ArgumentError':exception( - erlang:iolist_to_binary( - ["argument error while evaluating", badarg_file(File), badarg_line(Min, Max)] - ) - ); -rewrite_exception(Other, _, _, _) -> - Other. - -badarg_file(<<"nofile">>) -> ""; -badarg_file(Path) -> [$\s, 'Elixir.Path':relative_to_cwd(Path)]. - -badarg_line(999999, -999999) -> []; -badarg_line(Line, Line) -> [" at line ", integer_to_binary(Line)]; -badarg_line(Min, Max) -> [" between lines ", integer_to_binary(Min), " and ", integer_to_binary(Max)]. - -rewrite_stacktrace(Stacktrace) -> - % eval_eval and eval_bits can call :erlang.raise/3 without the full - % stacktrace. When this occurs re-add the current stacktrace so that no - % stack information is lost. - {current_stacktrace, CurrentStack} = erlang:process_info(self(), current_stacktrace), - merge_stacktrace(Stacktrace, tl(CurrentStack)). - -% The stacktrace did not include the current stack, re-add it. -merge_stacktrace([], CurrentStack) -> - CurrentStack; -% The stacktrace includes the current stack. -merge_stacktrace(CurrentStack, CurrentStack) -> - CurrentStack; -merge_stacktrace([StackItem | Stacktrace], CurrentStack) -> - [StackItem | merge_stacktrace(Stacktrace, CurrentStack)]. + catch + Kind:Reason:Stacktrace -> + %% Take everything up to the Elixir module + Pruned = + lists:takewhile(fun + ({elixir,_,_,_}) -> false; + (_) -> true + end, Stacktrace), + + Caller = + lists:dropwhile(fun + ({elixir,_,_,_}) -> false; + (_) -> true + end, Stacktrace), + + %% Now we prune any shared code path from erl_eval + {current_stacktrace, Current} = + erlang:process_info(self(), current_stacktrace), + + Reversed = drop_common(lists:reverse(Current), lists:reverse(Pruned)), + File = elixir_utils:characters_to_list(?key(Env, file)), + Location = [{file, File}, {line, erl_anno:line(Ann)}], + + %% Add file+line information at the bottom + Custom = lists:reverse([{elixir_eval, '__FILE__', 1, Location} | Reversed], Caller), + erlang:raise(Kind, Reason, Custom) + end + end, + {value, Fun}. + +drop_common([H | T1], [H | T2]) -> drop_common(T1, T2); +drop_common([_ | T1], T2) -> drop_common(T1, T2); +drop_common([], [{?MODULE, _, _, _} | T2]) -> T2; +drop_common([], T2) -> T2. +-else. +eval_external_handler(_Env) -> + none. +-endif. %% Converts a quoted expression to Erlang abstract format diff --git a/lib/elixir/test/elixir/code_test.exs b/lib/elixir/test/elixir/code_test.exs index 7e859c86eed..7ed4a38edf7 100644 --- a/lib/elixir/test/elixir/code_test.exs +++ b/lib/elixir/test/elixir/code_test.exs @@ -69,27 +69,35 @@ defmodule CodeTest do assert Code.eval_string(code, [], options) == {true, []} end - test "yields the correct stacktrace" do + test "keeps caller in stacktrace" do try do - Code.eval_string("<>", a: :a, b: :b) + Code.eval_string("<>", [a: :a, b: :b], file: "myfile") rescue _ -> assert Enum.any?(__STACKTRACE__, &(elem(&1, 0) == __MODULE__)) end end - test "raises streamlined argument errors" do - assert_raise ArgumentError, - ~r"argument error while evaluating at line 1", - fn -> Code.eval_string("a <> b", a: :a, b: :b) end - - assert_raise ArgumentError, - ~r"argument error while evaluating example.ex at line 1", - fn -> Code.eval_string("a <> b", [a: :a, b: :b], file: "example.ex") end - - assert_raise ArgumentError, - ~r"argument error while evaluating example.ex between lines 1 and 2", - fn -> Code.eval_string("a <>\nb", [a: :a, b: :b], file: "example.ex") end + if System.otp_release() >= "25" do + test "includes eval file in stacktrace" do + try do + Code.eval_string("<>", [a: :a, b: :b], file: "myfile") + rescue + _ -> + assert Exception.format_stacktrace(__STACKTRACE__) =~ "myfile:1" + end + + try do + Code.eval_string( + "Enum.map([a: :a, b: :b], fn {a, b} -> <> end)", + [], + file: "myfile" + ) + rescue + _ -> + assert Exception.format_stacktrace(__STACKTRACE__) =~ "myfile:1" + end + end end test "warns when lexical tracker process is dead" do diff --git a/lib/iex/lib/iex/evaluator.ex b/lib/iex/lib/iex/evaluator.ex index af8cc5e83d0..b3bffa2773f 100644 --- a/lib/iex/lib/iex/evaluator.ex +++ b/lib/iex/lib/iex/evaluator.ex @@ -398,23 +398,32 @@ defmodule IEx.Evaluator do end end - @elixir_internals [:elixir, :elixir_expand, :elixir_compiler, :elixir_module] ++ - [:elixir_clauses, :elixir_lexical, :elixir_def, :elixir_map] ++ - [:elixir_erl, :elixir_erl_clauses, :elixir_erl_pass] ++ - [Kernel.ErrorHandler, Module.ParallelChecker] - - defp prune_stacktrace(stacktrace) do - # The order in which each drop_while is listed is important. - # For example, the user may call Code.eval_string/2 in IEx - # and if there is an error we should not remove erl_eval - # and eval_bits information from the user stacktrace. - stacktrace - |> Enum.reverse() - |> Enum.drop_while(&(elem(&1, 0) == :proc_lib)) - |> Enum.drop_while(&(elem(&1, 0) == __MODULE__)) - |> Enum.drop_while(&(elem(&1, 0) in [Code, Module.ParallelChecker, :elixir])) - |> Enum.drop_while(&(elem(&1, 0) in [:erl_eval, :eval_bits])) - |> Enum.reverse() - |> Enum.reject(&(elem(&1, 0) in @elixir_internals)) + if System.otp_release() >= "25" do + defp prune_stacktrace(stack) do + stack + |> Enum.reverse() + |> Enum.drop_while(&(elem(&1, 0) != :elixir_eval)) + |> Enum.reverse() + end + else + @elixir_internals [:elixir, :elixir_expand, :elixir_compiler, :elixir_module] ++ + [:elixir_clauses, :elixir_lexical, :elixir_def, :elixir_map] ++ + [:elixir_erl, :elixir_erl_clauses, :elixir_erl_pass] ++ + [Kernel.ErrorHandler, Module.ParallelChecker] + + defp prune_stacktrace(stacktrace) do + # The order in which each drop_while is listed is important. + # For example, the user may call Code.eval_string/2 in IEx + # and if there is an error we should not remove erl_eval + # and eval_bits information from the user stacktrace. + stacktrace + |> Enum.reverse() + |> Enum.drop_while(&(elem(&1, 0) == :proc_lib)) + |> Enum.drop_while(&(elem(&1, 0) == __MODULE__)) + |> Enum.drop_while(&(elem(&1, 0) in [Code, Module.ParallelChecker, :elixir])) + |> Enum.drop_while(&(elem(&1, 0) in [:erl_eval, :eval_bits])) + |> Enum.reverse() + |> Enum.reject(&(elem(&1, 0) in @elixir_internals)) + end end end