Skip to content

Commit

Permalink
Handle errors in setting breakpoints
Browse files Browse the repository at this point in the history 
Disallow setting breakpoints in Inspect protocol implementations
Disallow setting breakpoints in builtin protocols and JasonV.Encoder protocol
Fixes #900
Fixes #903
Fixes #942
  • Loading branch information
@lukaszsamson
lukaszsamson committed Jul 11, 2023
1 parent 6bc501c commit 5c1182b
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 13 deletions.
72 changes: 59 additions & 13 deletions apps/elixir_ls_debugger/lib/debugger/server.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ defmodule ElixirLS.Debugger.Server do
result =
case current[{m, f, a}] do
nil ->
case :int.ni(m) do
case interpret(m, false) do
{:module, _} ->
ModuleInfoCache.store(m)
breaks_before = :int.all_breaks(m)
Expand All @@ -344,16 +344,21 @@ defmodule ElixirLS.Debugger.Server do
"Setting function breakpoint in #{inspect(m)}.#{f}/#{a}"
)

:ok = :int.break_in(m, f, a)
breaks_after = :int.all_breaks(m)
lines = for {{^m, line}, _} <- breaks_after -- breaks_before, do: line
case :int.break_in(m, f, a) do
:ok ->
breaks_after = :int.all_breaks(m)
lines = for {{^m, line}, _} <- breaks_after -- breaks_before, do: line

# pass nil as log_message - not supported on function breakpoints as of DAP 1.51
update_break_condition(m, lines, condition, nil, hit_count)
# pass nil as log_message - not supported on function breakpoints as of DAP 1.51
update_break_condition(m, lines, condition, nil, hit_count)

{:ok, lines}
{:ok, lines}

_ ->
{:error, :function_not_found} ->
{:error, "Function #{inspect(m)}.#{f}/#{a} not found"}
end

:error ->
{:error, "Cannot interpret module #{inspect(m)}"}
end

Expand Down Expand Up @@ -1153,8 +1158,7 @@ defmodule ElixirLS.Debugger.Server do
defp save_and_reload(module, beam_bin) do
:ok = File.write(Path.join(@temp_beam_dir, to_string(module) <> ".beam"), beam_bin)
true = :code.delete(module)
Output.debugger_console("Interpreting module #{inspect(module)}")
{:module, _} = :int.ni(module)
{:module, _} = interpret(module)
ModuleInfoCache.store(module)
end

Expand Down Expand Up @@ -1193,6 +1197,9 @@ defmodule ElixirLS.Debugger.Server do
set_breakpoint(modules_to_break, path, line)
end
end
rescue
error ->
for _line <- lines, do: {:error, Exception.format_exit(error)}
end

defp set_breakpoint([], path, {line, _}) do
Expand All @@ -1202,10 +1209,11 @@ defmodule ElixirLS.Debugger.Server do
defp set_breakpoint(modules, path, {line, {condition, log_message, hit_count}}) do
modules_with_breakpoints =
Enum.reduce(modules, [], fn module, added ->
case :int.ni(module) do
case interpret(module, false) do
{:module, _} ->
ModuleInfoCache.store(module)
Output.debugger_console("Setting breakpoint in #{inspect(module)} #{path}:#{line}")
# no need to handle errors here, it can fail only with {:error, :break_exists}
:int.break(module, line)
update_break_condition(module, line, condition, log_message, hit_count)

Expand Down Expand Up @@ -1237,8 +1245,7 @@ defmodule ElixirLS.Debugger.Server do

defp interpret_module(mod) do
try do
Output.debugger_console("Interpreting module #{inspect(mod)}")
{:module, _} = :int.ni(mod)
{:module, _} = interpret(mod)
ModuleInfoCache.store(mod)
catch
_, _ ->
Expand Down Expand Up @@ -1397,4 +1404,43 @@ defmodule ElixirLS.Debugger.Server do
"step"
end
end

@exclude_protocols_from_interpreting [
Enumerable,
Collectable,
List.Chars,
String.Chars,
Inspect,
IEx.Info,
JasonV.Encoder
]

@exclude_implementations_from_interpreting [Inspect]

defp interpret(module, print_message? \\ true)

defp interpret(module, _print_message?) when module in @exclude_protocols_from_interpreting do
:error
end

defp interpret(module, print_message?) do
if Code.ensure_loaded?(module) do
module_behaviours =
module.module_info(:attributes) |> Keyword.get_values(:behaviour) |> Enum.concat()

if Enum.any?(@exclude_implementations_from_interpreting, &(&1 in module_behaviours)) do
# debugger uses Inspect protocol and setting breakpoints in implementations leads to deadlocks
# https://github.com/elixir-lsp/elixir-ls/issues/903
:error
else
if print_message? do
Output.debugger_console("Interpreting module #{inspect(module)}")
end

:int.ni(module)
end
else
:error
end
end
end
92 changes: 92 additions & 0 deletions apps/elixir_ls_debugger/test/debugger_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,54 @@ defmodule ElixirLS.Debugger.ServerTest do
end)
end

@tag :fixture
test "handles invalid request", %{server: server} do
in_fixture(__DIR__, "mix_project", fn ->
Server.receive_packet(server, initialize_req(1, %{}))
assert_receive(response(_, 1, "initialize", _))

Server.receive_packet(
server,
launch_req(2, %{
"request" => "launch",
"type" => "mix_task",
"task" => "test",
"projectDir" => File.cwd!(),
# disable auto interpret
"debugAutoInterpretAllModules" => false
})
)

assert_receive(response(_, 2, "launch", _), 3000)
assert_receive(event(_, "initialized", %{}), 5000)

refute :hello in :int.interpreted()
abs_path = Path.absname("src/hello.erl1")

Server.receive_packet(
server,
set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 5}])
)

assert_receive(
response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => false}]}),
3000
)

abs_path = Path.absname("lib/mix_project.ex1")

Server.receive_packet(
server,
set_breakpoints_req(4, %{"path" => abs_path}, [%{"line" => 3}])
)

assert_receive(
response(_, 4, "setBreakpoints", %{"breakpoints" => [%{"verified" => false}]}),
3000
)
end)
end

@tag :fixture
test "sets and unsets breakpoints in elixir modules", %{server: server} do
in_fixture(__DIR__, "mix_project", fn ->
Expand Down Expand Up @@ -1671,6 +1719,50 @@ defmodule ElixirLS.Debugger.ServerTest do
end)
end

test "handles invalid requests", %{server: server} do
in_fixture(__DIR__, "mix_project", fn ->
Server.receive_packet(server, initialize_req(1, %{}))
assert_receive(response(_, 1, "initialize", _))

Server.receive_packet(
server,
launch_req(2, %{
"request" => "launch",
"type" => "mix_task",
"task" => "test",
"projectDir" => File.cwd!(),
# disable auto interpret
"debugAutoInterpretAllModules" => false
})
)

assert_receive(response(_, 2, "launch", _), 3000)
assert_receive(event(_, "initialized", %{}), 5000)

refute :hello in :int.interpreted()

Server.receive_packet(
server,
set_function_breakpoints_req(3, [%{"name" => ":hello1.hello_world/0"}])
)

assert_receive(
response(_, 3, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => false}]}),
3000
)

Server.receive_packet(
server,
set_function_breakpoints_req(4, [%{"name" => ":hello.hello_world1/0"}])
)

assert_receive(
response(_, 4, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => false}]}),
3000
)
end)
end

test "sets, modifies and unsets conditional function breakpoints", %{server: server} do
in_fixture(__DIR__, "mix_project", fn ->
Server.receive_packet(server, initialize_req(1, %{}))
Expand Down

0 comments on commit 5c1182b

Please sign in to comment.