[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
This commit is contained in:
Jordan Rupprecht 2020-09-30 11:24:10 -07:00
parent 655af658c9
commit ad865d9d10
3 changed files with 77 additions and 28 deletions

View File

@ -728,6 +728,16 @@ class DebugCommunication(object):
def request_setBreakpoints(self, file_path, line_array, condition=None,
hitCondition=None):
(dir, base) = os.path.split(file_path)
source_dict = {
'name': base,
'path': file_path
}
args_dict = {
'source': source_dict,
'sourceModified': False,
}
if line_array is not None:
args_dict['lines'] = '%s' % line_array
breakpoints = []
for line in line_array:
bp = {'line': line}
@ -736,16 +746,8 @@ class DebugCommunication(object):
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,
}
args_dict['breakpoints'] = breakpoints
command_dict = {
'command': 'setBreakpoints',
'type': 'request',

View File

@ -219,6 +219,48 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
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):

View File

@ -1936,6 +1936,9 @@ void request_setBreakpoints(const llvm::json::Object &request) {
// Decode the source breakpoint infos for this "setBreakpoints" request
SourceBreakpointMap request_bps;
// "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) {
@ -1945,7 +1948,8 @@ void request_setBreakpoints(const llvm::json::Object &request) {
// 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);
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,
@ -1959,6 +1963,7 @@ void request_setBreakpoints(const llvm::json::Object &request) {
g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}
}
// Delete any breakpoints in this source file that aren't in the
// request_bps set. There is no call to remove breakpoints other than