From: Jordan Rupprecht Date: Wed, 30 Sep 2020 18:24:10 +0000 (-0700) Subject: [lldb-vscode] Allow an empty 'breakpoints' field to clear breakpoints. X-Git-Tag: llvmorg-13-init~10478 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ad865d9d10b8cf93738470175aae1be7a4a3eb6b;p=platform%2Fupstream%2Fllvm.git [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 --- 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 834e33e..70f29cd 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 @@ class DebugCommunication(object): 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 c08c4d7..23f4ad2 100644 --- a/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py @@ -221,6 +221,48 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase): @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): '''Tests hitting breakpoints and the functionality of a single breakpoint, like 'conditions' and 'hitCondition' settings.''' diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index 3b0817c..b648294 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); } }