[Lldb-commits] [lldb] ad865d9 - [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints.
Jordan Rupprecht via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 30 11:33:37 PDT 2020
Author: Jordan Rupprecht
Date: 2020-09-30T11:32:06-07:00
New Revision: ad865d9d10b8cf93738470175aae1be7a4a3eb6b
URL: https://github.com/llvm/llvm-project/commit/ad865d9d10b8cf93738470175aae1be7a4a3eb6b
DIFF: https://github.com/llvm/llvm-project/commit/ad865d9d10b8cf93738470175aae1be7a4a3eb6b.diff
LOG: [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints.
Per the DAP spec for SetBreakpoints [1], the way to clear breakpoints is: `To clear all breakpoint for a source, specify an empty array.`
However, leaving the breakpoints field unset is also a well formed request (note the `breakpoints?:` in the `SetBreakpointsArguments` definition). If it's unset, we have a couple choices:
1. Crash (current behavior)
2. Clear breakpoints
3. Return an error response that the breakpoints field is missing.
I propose we do (2) instead of (1), and treat an unset breakpoints field the same as an empty breakpoints field.
[1] https://microsoft.github.io/debug-adapter-protocol/specification#Requests_SetBreakpoints
Reviewed By: wallace, labath
Differential Revision: https://reviews.llvm.org/D88513
Added:
Modified:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 834e33ef5c3d..70f29cdd3d75 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -728,24 +728,26 @@ def request_scopes(self, frameId):
def request_setBreakpoints(self, file_path, line_array, condition=None,
hitCondition=None):
(dir, base) = os.path.split(file_path)
- breakpoints = []
- for line in line_array:
- bp = {'line': line}
- if condition is not None:
- bp['condition'] = condition
- if hitCondition is not None:
- bp['hitCondition'] = hitCondition
- breakpoints.append(bp)
source_dict = {
'name': base,
'path': file_path
}
args_dict = {
'source': source_dict,
- 'breakpoints': breakpoints,
- 'lines': '%s' % (line_array),
'sourceModified': False,
}
+ if line_array is not None:
+ args_dict['lines'] = '%s' % line_array
+ breakpoints = []
+ for line in line_array:
+ bp = {'line': line}
+ if condition is not None:
+ bp['condition'] = condition
+ if hitCondition is not None:
+ bp['hitCondition'] = hitCondition
+ breakpoints.append(bp)
+ args_dict['breakpoints'] = breakpoints
+
command_dict = {
'command': 'setBreakpoints',
'type': 'request',
diff --git a/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py b/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
index c08c4d70489f..23f4ad216ea2 100644
--- a/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ b/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -219,6 +219,48 @@ def test_set_and_clear(self):
self.assertTrue(breakpoint['verified'],
"expect breakpoint still verified")
+ @skipIfWindows
+ @skipIfRemote
+ def test_clear_breakpoints_unset_breakpoints(self):
+ '''Test clearing breakpoints like test_set_and_clear, but clear
+ breakpoints by omitting the breakpoints array instead of sending an
+ empty one.'''
+ lines = [line_number('main.cpp', 'break 12'),
+ line_number('main.cpp', 'break 13')]
+
+ # Visual Studio Code Debug Adaptors have no way to specify the file
+ # without launching or attaching to a process, so we must start a
+ # process in order to be able to set breakpoints.
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+
+ # Set one breakpoint and verify that it got set correctly.
+ response = self.vscode.request_setBreakpoints(self.main_path, lines)
+ line_to_id = {}
+ breakpoints = response['body']['breakpoints']
+ self.assertEquals(len(breakpoints), len(lines),
+ "expect %u source breakpoints" % (len(lines)))
+ for (breakpoint, index) in zip(breakpoints, range(len(lines))):
+ line = breakpoint['line']
+ self.assertTrue(line, lines[index])
+ # Store the "id" of the breakpoint that was set for later
+ line_to_id[line] = breakpoint['id']
+ self.assertTrue(line in lines, "line expected in lines array")
+ self.assertTrue(breakpoint['verified'],
+ "expect breakpoint verified")
+
+ # Now clear all breakpoints for the source file by not setting the
+ # lines array.
+ lines = None
+ response = self.vscode.request_setBreakpoints(self.main_path, lines)
+ breakpoints = response['body']['breakpoints']
+ self.assertEquals(len(breakpoints), 0, "expect no source breakpoints")
+
+ # Verify with the target that all breakpoints have been cleared.
+ response = self.vscode.request_testGetTargetBreakpoints()
+ breakpoints = response['body']['breakpoints']
+ self.assertEquals(len(breakpoints), 0, "expect no source breakpoints")
+
@skipIfWindows
@skipIfRemote
def test_functionality(self):
diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 3b0817c71e62..b64829423e30 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1936,27 +1936,32 @@ void request_setBreakpoints(const llvm::json::Object &request) {
// Decode the source breakpoint infos for this "setBreakpoints" request
SourceBreakpointMap request_bps;
- for (const auto &bp : *breakpoints) {
- auto bp_obj = bp.getAsObject();
- if (bp_obj) {
- SourceBreakpoint src_bp(*bp_obj);
- request_bps[src_bp.line] = src_bp;
-
- // We check if this breakpoint already exists to update it
- auto existing_source_bps = g_vsc.source_breakpoints.find(path);
- if (existing_source_bps != g_vsc.source_breakpoints.end()) {
- const auto &existing_bp = existing_source_bps->second.find(src_bp.line);
- if (existing_bp != existing_source_bps->second.end()) {
- existing_bp->second.UpdateBreakpoint(src_bp);
- AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
- src_bp.line);
- continue;
+ // "breakpoints" may be unset, in which case we treat it the same as being set
+ // to an empty array.
+ if (breakpoints) {
+ for (const auto &bp : *breakpoints) {
+ auto bp_obj = bp.getAsObject();
+ if (bp_obj) {
+ SourceBreakpoint src_bp(*bp_obj);
+ request_bps[src_bp.line] = src_bp;
+
+ // We check if this breakpoint already exists to update it
+ auto existing_source_bps = g_vsc.source_breakpoints.find(path);
+ if (existing_source_bps != g_vsc.source_breakpoints.end()) {
+ const auto &existing_bp =
+ existing_source_bps->second.find(src_bp.line);
+ if (existing_bp != existing_source_bps->second.end()) {
+ existing_bp->second.UpdateBreakpoint(src_bp);
+ AppendBreakpoint(existing_bp->second.bp, response_breakpoints, path,
+ src_bp.line);
+ continue;
+ }
}
+ // At this point the breakpoint is new
+ src_bp.SetBreakpoint(path.data());
+ AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
+ g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
- // At this point the breakpoint is new
- src_bp.SetBreakpoint(path.data());
- AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
- g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}
More information about the lldb-commits
mailing list