Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dap] support non-verified breakpoints #1374

Merged
merged 2 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions apps/els_dap/src/els_dap_breakpoints.erl
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
build_source_breakpoints/1,
get_function_breaks/2,
get_line_breaks/2,
do_line_breakpoints/4,
do_function_breaks/4,
do_line_breakpoints/5,
do_function_breaks/5,
type/3
]).

Expand Down Expand Up @@ -108,32 +108,43 @@ get_function_breaks(Module, Breaks) ->
get_line_breaks(Module, Breaks) ->
case Breaks of
#{Module := #{line := Lines}} -> Lines;
_ -> []
_ -> #{}
end.

-spec do_line_breakpoints(
node(),
module(),
#{line() => line_breaks()},
breakpoints()
breakpoints(),
boolean()
) ->
breakpoints().
do_line_breakpoints(Node, Module, LineBreakPoints, Breaks) ->
maps:map(
fun(Line, _) -> els_dap_rpc:break(Node, Module, Line) end,
LineBreakPoints
),
do_line_breakpoints(Node, Module, LineBreakPoints, Breaks, Set) ->
case Set of
Copy link
Member

Choose a reason for hiding this comment

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

In which case do we get Set=false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the module does not exist, e.g. the debugged node cannot load the module.

true ->
maps:map(
fun(Line, _) -> els_dap_rpc:break(Node, Module, Line) end,
LineBreakPoints
);
false ->
ok
end,
case Breaks of
#{Module := ModBreaks} ->
Breaks#{Module => ModBreaks#{line => LineBreakPoints}};
_ ->
Breaks#{Module => #{line => LineBreakPoints, function => []}}
end.

-spec do_function_breaks(node(), module(), [function_break()], breakpoints()) ->
-spec do_function_breaks(node(), module(), [function_break()], breakpoints(), boolean()) ->
breakpoints().
do_function_breaks(Node, Module, FBreaks, Breaks) ->
[els_dap_rpc:break_in(Node, Module, Func, Arity) || {Func, Arity} <- FBreaks],
do_function_breaks(Node, Module, FBreaks, Breaks, Set) ->
case Set of
true ->
[els_dap_rpc:break_in(Node, Module, Func, Arity) || {Func, Arity} <- FBreaks];
false ->
ok
end,
case Breaks of
#{Module := ModBreaks} ->
Breaks#{Module => ModBreaks#{function => FBreaks}};
Expand Down
113 changes: 71 additions & 42 deletions apps/els_dap/src/els_dap_general_provider.erl
Original file line number Diff line number Diff line change
Expand Up @@ -192,20 +192,20 @@ handle_request(
ensure_connected(ProjectNode, Timeout),
{Module, LineBreaks} = els_dap_breakpoints:build_source_breakpoints(Params),

{module, Module} = els_dap_rpc:i(ProjectNode, Module),
{IsModuleAvailable, Message} = maybe_interpret_and_clear_module(ProjectNode, Module),

%% purge all breakpoints from the module
els_dap_rpc:no_break(ProjectNode, Module),
Breakpoints1 =
els_dap_breakpoints:do_line_breakpoints(
ProjectNode,
Module,
LineBreaks,
Breakpoints0
Breakpoints0,
IsModuleAvailable
),

BreakpointsRsps = [
#{<<"verified">> => true, <<"line">> => Line}
|| {{_, Line}, _} <- els_dap_rpc:all_breaks(ProjectNode, Module)
#{<<"verified">> => IsModuleAvailable, <<"line">> => Line, message => Message}
Copy link
Member

Choose a reason for hiding this comment

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

Should message be a binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, message should be a binary.

|| Line <- maps:keys(LineBreaks)
],

FunctionBreaks =
Expand All @@ -215,7 +215,8 @@ handle_request(
ProjectNode,
Module,
FunctionBreaks,
Breakpoints1
Breakpoints1,
IsModuleAvailable
),

{#{<<"breakpoints">> => BreakpointsRsps}, State#{breakpoints => Breakpoints2}};
Expand All @@ -232,11 +233,7 @@ handle_request(
ensure_connected(ProjectNode, Timeout),
FunctionBreakPoints = maps:get(<<"breakpoints">>, Params, []),
MFAs = [
begin
Spec = {Mod, _, _} = parse_mfa(MFA),
els_dap_rpc:i(ProjectNode, Mod),
Spec
end
parse_mfa(MFA)
|| #{<<"name">> := MFA, <<"enabled">> := Enabled} <- FunctionBreakPoints,
Enabled andalso parse_mfa(MFA) =/= error
],
Expand All @@ -252,54 +249,64 @@ handle_request(
MFAs
),

%% we need to really purge all break points here
els_dap_rpc:no_break(ProjectNode),
Breakpoints1 = maps:fold(
fun(Mod, Breaks, Acc) ->
Acc#{Mod => Breaks#{function => []}}
end,
#{},
Breakpoints0
),

Breakpoints2 = maps:fold(
fun(Module, FunctionBreaks, Acc) ->
els_dap_breakpoints:do_function_breaks(
ProjectNode,
Module,
FunctionBreaks,
Acc
)
end,
Breakpoints1,
ModFuncBreaks
),
{Breakpoints1, VerifiedMessage} =
maps:fold(
fun(Module, FunctionBreaks, {AccBP, AccVerified}) ->
{IsModuleAvailable, Message} =
maybe_interpret_and_clear_module(ProjectNode, Module),
{
els_dap_breakpoints:do_function_breaks(
ProjectNode,
Module,
FunctionBreaks,
AccBP#{
Module => #{function => []}
},
IsModuleAvailable
),
AccVerified#{Module => {IsModuleAvailable, Message}}
}
end,
{Breakpoints0, #{}},
ModFuncBreaks
),

BreakpointsRsps = [
#{
<<"verified">> => true,
<<"line">> => Line,
<<"source">> => #{<<"path">> => source(Module, ProjectNode)}
<<"verified">> => Verified,
<<"message">> => Message,
<<"source">> => #{
<<"path">> =>
case Verified of
true -> source(Module, ProjectNode);
false -> <<"">>
end
}
}
|| {{Module, Line}, [Status, _, _, _]} <-
els_dap_rpc:all_breaks(ProjectNode),
Status =:= active
|| {Module, {Verified, Message}} <- maps:to_list(VerifiedMessage)
],

%% replay line breaks
Breakpoints3 = maps:fold(
Breakpoints2 = maps:fold(
fun(Module, _, Acc) ->
Set = true =:= els_dap_rpc:interpretable(ProjectNode, Module),
Lines = els_dap_breakpoints:get_line_breaks(Module, Acc),
els_dap_breakpoints:do_line_breakpoints(
ProjectNode,
Module,
Lines,
Acc
Acc,
Set
)
end,
Breakpoints2,
Breakpoints2
Breakpoints1,
Breakpoints1
),

{#{<<"breakpoints">> => BreakpointsRsps}, State#{breakpoints => Breakpoints3}};
{#{<<"breakpoints">> => BreakpointsRsps}, State#{breakpoints => Breakpoints2}};
handle_request({<<"threads">>, _Params}, #{threads := Threads0} = State) ->
Threads =
[
Expand Down Expand Up @@ -1084,3 +1091,25 @@ distribution_error(Error) ->
io_lib:format("Could not start Erlang distribution. ~p", [Error])
)
).

-spec maybe_interpret_and_clear_module(node(), module()) -> {boolean(), binary()}.
maybe_interpret_and_clear_module(ProjectNode, Module) ->
case els_dap_rpc:interpretable(ProjectNode, Module) of
true ->
{module, Module} = els_dap_rpc:i(ProjectNode, Module),

%% purge all breakpoints from the module
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want to purge them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just easier to code this way. The DAP protocol gives the following information on changes to breakpoints:

  • complete set of line breakpoints per module
  • complete set of function breakpoints in total

So we delete all breakpoints and set them again.

We should probably calculate a delta and then use delete_break and del_break_in. This PR doesn't change the general way we set breakpoints, just makes sure we don't crash on invalid ones.

els_dap_rpc:no_break(ProjectNode, Module),
{true, <<"">>};
{error, Reason} ->
Msg = unicode:characters_to_binary(
io_lib:format(
<<
"module not available (~p) in the debugged node, "
"reset the breakpoint when the module is availalbe"
>>,
[Reason]
)
),
{false, Msg}
end.
7 changes: 7 additions & 0 deletions apps/els_dap/src/els_dap_rpc.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
get_meta/2,
halt/1,
i/2,
interpretable/2,
load_binary/4,
meta/4,
meta_eval/3,
Expand Down Expand Up @@ -106,6 +107,12 @@ halt(Node) ->
i(Node, Module) ->
rpc:call(Node, int, i, [Module]).

-spec interpretable(node(), module() | string()) ->
true
| {error, no_src | no_beam | no_debug_info | badarg | {app, kernel | stdlib | gs | debugger}}.
interpretable(Node, AbsModule) ->
rpc:call(Node, int, interpretable, [AbsModule]).

-spec load_binary(node(), module(), string(), binary()) -> any().
load_binary(Node, Module, File, Bin) ->
rpc:call(Node, code, load_binary, [Module, File, Bin]).
Expand Down
21 changes: 20 additions & 1 deletion apps/els_dap/test/els_dap_general_provider_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,16 @@ breakpoints(Config) ->
)
),
?assertMatch([{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)),

els_dap_provider:handle_request(
Provider,
request_set_breakpoints(
path_to_test_module(DataDir, i_dont_exist),
[42]
)
),
?assertMatch([{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)),

els_dap_provider:handle_request(
Provider,
request_set_function_breakpoints([<<"els_dap_test_module:entry/1">>])
Expand Down Expand Up @@ -508,7 +518,16 @@ breakpoints(Config) ->
Provider,
request_set_function_breakpoints([])
),
?assertMatch([{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)),
?assertMatch(
[{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)
),
els_dap_provider:handle_request(
Provider,
request_set_function_breakpoints([<<"i_dont:exist/42">>])
),
?assertMatch(
[{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)
),
ok.

-spec project_node_exit(config()) -> ok.
Expand Down