intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-26 21:53:12 +08:00
Code Issues Packages Projects Releases Wiki Activity

[lldb-vscode] fix breakpoint result ordering

Browse Source 
Summary:
The DAP specifies the following for the SetBreakpoints request:

  The breakpoints returned are in the same order as the elements of the 'breakpoints' arguments

This was not followed, as lldb-vscode was returning the breakpoints in a different order, because they were first stored into a map, and then traversed. Of course, maps normally don't preserve ordering.

See this log I captured:

  -->
  {"command":"setBreakpoints",
   "arguments":{
     "source":{
       "name":"main.cpp",
       "path":"/Users/wallace/fbsource/xplat/sand/test-projects/buck-cpp/main.cpp"
     },
     "lines":[6,10,11],
     "breakpoints":[{"line":6},{"line":10},{"line":11}],
     "sourceModified":false
   },
   "type":"request",
   "seq":3
  }

  <--
  {"body":{
     "breakpoints":[
       {"id":1, "line":11,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true},
       {"id":2,"line":6,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true},
       {"id":3,"line":10,"source":{"name":"main.cpp","path":"xplat/sand/test-projects/buck-cpp/main.cpp"},"verified":true}]},
     "command":"setBreakpoints",
     "request_seq":3,
     "seq":0,
     "success":true,
     "type":"response"
  }

As you can see, the order was not respected. This was causing the IDE not to be able to disable/enable breakpoints by clicking on them in the breakpoint view in the lower corner of the Debug tab.

This diff fixes the ordering problem. The traversal + querying was done very fast in O(nlogn) time. I'm keeping the same complexity.

I also updated a couple of tests to account for the ordering.

Reviewers: clayborg, aadsm, kusmour, labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76891
This commit is contained in:
Walter Erquinigo
2020-03-26 15:33:12 -07:00
parent e9a2caf34c
commit a515fd01a4
2 changed files with 29 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
View File

@@ -36,7 +36,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
first_line = line_number('main.cpp', 'break 12')
second_line = line_number('main.cpp', 'break 13')
third_line = line_number('main.cpp', 'break 14')
lines = [first_line, second_line, third_line]
lines = [first_line, third_line, second_line]
# Visual Studio Code Debug Adaptors have no way to specify the file
# without launching or attaching to a process, so we must start a
@@ -51,8 +51,9 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), len(lines),
"expect %u source breakpoints" % (len(lines)))
for breakpoint in breakpoints:
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")
@@ -74,8 +75,9 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), len(lines),
"expect %u source breakpoints" % (len(lines)))
for breakpoint in breakpoints:
for (breakpoint, index) in zip(breakpoints, range(len(lines))):
line = breakpoint['line']
self.assertTrue(line, lines[index])
# Verify the same breakpoints are still set within LLDB by
# making sure the breakpoint ID didn't change
self.assertEquals(line_to_id[line], breakpoint['id'],

73
lldb/tools/lldb-vscode/lldb-vscode.cpp
View File

@@ -1733,70 +1733,45 @@ void request_setBreakpoints(const llvm::json::Object &request) {
const auto path = GetString(source, "path");
auto breakpoints = arguments->getArray("breakpoints");
llvm::json::Array response_breakpoints;
// 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] = std::move(src_bp);
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);
continue;
}
}
// At this point the breakpoint is new
src_bp.SetBreakpoint(path.data());
AppendBreakpoint(src_bp.bp, response_breakpoints);
g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}
// See if we already have breakpoints set for this source file from a
// previous "setBreakpoints" request
// Delete any breakpoints in this source file that aren't in the
// request_bps set. There is no call to remove breakpoints other than
// calling this function with a smaller or empty "breakpoints" list.
auto old_src_bp_pos = g_vsc.source_breakpoints.find(path);
if (old_src_bp_pos != g_vsc.source_breakpoints.end()) {
// We have already set breakpoints in this source file and they are giving
// use a new list of lines to set breakpoints on. Some breakpoints might
// already be set, and some might not. We need to remove any breakpoints
// whose lines are not contained in the any breakpoints lines in in the
// "breakpoints" array.
// Delete any breakpoints in this source file that aren't in the
// request_bps set. There is no call to remove breakpoints other than
// calling this function with a smaller or empty "breakpoints" list.
std::vector<uint32_t> remove_lines;
for (auto &pair: old_src_bp_pos->second) {
auto request_pos = request_bps.find(pair.first);
for (auto &old_bp : old_src_bp_pos->second) {
auto request_pos = request_bps.find(old_bp.first);
if (request_pos == request_bps.end()) {
// This breakpoint no longer exists in this source file, delete it
g_vsc.target.BreakpointDelete(pair.second.bp.GetID());
remove_lines.push_back(pair.first);
} else {
pair.second.UpdateBreakpoint(request_pos->second);
// Remove this breakpoint from the request breakpoints since we have
// handled it here and we don't need to set a new breakpoint below.
request_bps.erase(request_pos);
// Add this breakpoint info to the response
AppendBreakpoint(pair.second.bp, response_breakpoints);
g_vsc.target.BreakpointDelete(old_bp.second.bp.GetID());
old_src_bp_pos->second.erase(old_bp.first);
}
}
// Remove any lines from this existing source breakpoint map
for (auto line: remove_lines)
old_src_bp_pos->second.erase(line);
// Now add any breakpoint infos left over in request_bps are the
// breakpoints that weren't set in this source file yet. We need to update
// thread source breakpoint info for the source file in the variable
// "old_src_bp_pos->second" so the info for this source file is up to date.
for (auto &pair : request_bps) {
pair.second.SetBreakpoint(path.data());
// Add this breakpoint info to the response
AppendBreakpoint(pair.second.bp, response_breakpoints);
old_src_bp_pos->second[pair.first] = std::move(pair.second);
}
} else {
// No breakpoints were set for this source file yet. Set all breakpoints
// for each line and add them to the response and create an entry in
// g_vsc.source_breakpoints for this source file.
for (auto &pair : request_bps) {
pair.second.SetBreakpoint(path.data());
// Add this breakpoint info to the response
AppendBreakpoint(pair.second.bp, response_breakpoints);
}
g_vsc.source_breakpoints[path] = std::move(request_bps);
}
llvm::json::Object body;