maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request 

Summary:
When using source maps for a breakpoint, in order to find the actual source that breakpoint has resolved, we
need to use a query similar to what CommandObjectSource::DumpLinesInSymbolContexts does, which is the logic
used by the CLI to display the source line at a given breakpoint. That's necessary because from a breakpoint
location you only have an address, which points to the original source location, not the source mapped one.

in the setBreakpoints request handler, we haven't been doing such query and we were returning the original
source location, which was breaking the UX of VSCode, as many breakpoints were being set but not displayed
in the source file next to each line. Besides, clicking the source path of a breakpoint in the breakpoints
view in the debug tab was not working for this case, as VSCode was trying to open a non-existent file, thus
showing an error to the user.

Ideally, we should do the query mentioned above to find the actual source location to respond to the IDE,
however, that query is expensive and users can have an arbitrary number of breakpoints set. As a simpler fix,
the request sent by VSCode already contains the full source path, which exists because the user set it from
the IDE itself, so we can simply reuse it instead of querying from the SB API.

I wrote a test for this, and found out that I had to move SetSourceMapFromArguments after RunInitCommands in
lldb-vscode.cpp, because an init command used in all tests is `settings clear -all`, which would clear the
source map unless specified after initCommands. And in any case, I think it makes sense to have initCommands
run before anything the debugger would do.

Reviewers: clayborg, kusmour, labath, aadsm

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D76968
This commit is contained in:
Walter Erquinigo 2020-03-27 19:31:14 -07:00
parent ae8ebeca51
commit e796c77b26
9 changed files with 215 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
View File

@ -266,8 +266,8 @@ class VSCodeTestCaseBase(TestBase):
stopOnEntry=False, disableASLR=True,
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None,sourcePath= None,
debuggerRoot=None, launchCommands=None):
stopCommands=None, exitCommands=None,sourcePath=None,
debuggerRoot=None, launchCommands=None, sourceMap=None):
'''Sending launch request to vscode
'''
@ -298,7 +298,8 @@ class VSCodeTestCaseBase(TestBase):
exitCommands=exitCommands,
sourcePath=sourcePath,
debuggerRoot=debuggerRoot,
launchCommands=launchCommands)
launchCommands=launchCommands,
sourceMap=sourceMap)
if not (response and response['success']):
self.assertTrue(response['success'],
'launch failed (%s)' % (response['message']))

4
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
View File

@ -570,7 +570,7 @@ class DebugCommunication(object):
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, sourcePath=None,
debuggerRoot=None, launchCommands=None):
debuggerRoot=None, launchCommands=None, sourceMap=None):
args_dict = {
'program': program
}
@ -605,6 +605,8 @@ class DebugCommunication(object):
args_dict['debuggerRoot'] = debuggerRoot
if launchCommands:
args_dict['launchCommands'] = launchCommands
if sourceMap:
args_dict['sourceMap'] = sourceMap
command_dict = {
'command': 'launch',
'type': 'request',

18
lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
View File

@ -1,3 +1,19 @@
CXX_SOURCES := main.cpp
CXX_SOURCES := main-copy.cpp
LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
USE_LIBDL :=1
a.out: libother
include Makefile.rules
# We copy the source files to move them to test source mapping
other-copy.c: other.c
cp -f $< $@
main-copy.cpp: main.cpp
cp -f $< $@
# The following shared library will be used to test breakpoints under dynamic loading
libother: other-copy.c
$(MAKE) -f $(MAKEFILE_RULES) \
DYLIB_ONLY=YES DYLIB_C_SOURCES=other-copy.c DYLIB_NAME=other

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

@ -5,6 +5,7 @@ Test lldb-vscode setBreakpoints request
import unittest2
import vscode
import shutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
@ -16,6 +17,76 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
mydir = TestBase.compute_mydir(__file__)
def setUp(self):
lldbvscode_testcase.VSCodeTestCaseBase.setUp(self)
self.main_basename = 'main-copy.cpp'
self.main_path = self.getBuildArtifact(self.main_basename)
@skipIfWindows
@skipIfRemote
def test_source_map(self):
self.build_and_create_debug_adaptor()
other_basename = 'other-copy.c'
other_path = self.getBuildArtifact(other_basename)
source_folder = os.path.dirname(self.main_path)
new_main_folder = os.path.join(source_folder, 'moved_main')
new_other_folder = os.path.join(source_folder, 'moved_other')
new_main_path = os.path.join(new_main_folder, self.main_basename)
new_other_path = os.path.join(new_other_folder, other_basename)
# move the sources
os.mkdir(new_main_folder)
os.mkdir(new_other_folder)
shutil.move(self.main_path, new_main_path)
shutil.move(other_path, new_other_path)
main_line = line_number('main.cpp', 'break 12')
other_line = line_number('other.c', 'break other')
program = self.getBuildArtifact("a.out")
source_map = [
[source_folder, new_main_folder],
[source_folder, new_other_folder],
]
self.launch(program, sourceMap=source_map)
# breakpoint in main.cpp
response = self.vscode.request_setBreakpoints(new_main_path, [main_line])
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), 1)
breakpoint = breakpoints[0]
self.assertEqual(breakpoint['line'], main_line)
self.assertTrue(breakpoint['verified'])
self.assertEqual(self.main_basename, breakpoint['source']['name'])
self.assertEqual(new_main_path, breakpoint['source']['path'])
# 2nd breakpoint, which is from a dynamically loaded library
response = self.vscode.request_setBreakpoints(new_other_path, [other_line])
breakpoints = response['body']['breakpoints']
breakpoint = breakpoints[0]
self.assertEqual(breakpoint['line'], other_line)
self.assertFalse(breakpoint['verified'])
self.assertEqual(other_basename, breakpoint['source']['name'])
self.assertEqual(new_other_path, breakpoint['source']['path'])
other_breakpoint_id = breakpoint['id']
self.vscode.request_continue()
self.verify_breakpoint_hit([other_breakpoint_id])
# 2nd breakpoint again, which should be valid at this point
response = self.vscode.request_setBreakpoints(new_other_path, [other_line])
breakpoints = response['body']['breakpoints']
breakpoint = breakpoints[0]
self.assertEqual(breakpoint['line'], other_line)
self.assertTrue(breakpoint['verified'])
self.assertEqual(other_basename, breakpoint['source']['name'])
self.assertEqual(new_other_path, breakpoint['source']['path'])
@skipIfWindows
@skipIfRemote
def test_set_and_clear(self):
@ -31,8 +102,6 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
and makes sure things happen correctly. It doesn't test hitting
breakpoints and the functionality of each breakpoint, like
'conditions' and 'hitCondition' settings.'''
source_basename = 'main.cpp'
source_path = os.path.join(os.getcwd(), source_basename)
first_line = line_number('main.cpp', 'break 12')
second_line = line_number('main.cpp', 'break 13')
third_line = line_number('main.cpp', 'break 14')
@ -45,7 +114,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
self.build_and_launch(program)
# Set 3 breakpoints and verify that they got set correctly
response = self.vscode.request_setBreakpoints(source_path, lines)
response = self.vscode.request_setBreakpoints(self.main_path, lines)
line_to_id = {}
if response:
breakpoints = response['body']['breakpoints']
@ -70,7 +139,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
lines.remove(second_line)
# Set 2 breakpoints and verify that the previous breakpoints that were
# set above are still set.
response = self.vscode.request_setBreakpoints(source_path, lines)
response = self.vscode.request_setBreakpoints(self.main_path, lines)
if response:
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), len(lines),
@ -108,7 +177,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
# Now clear all breakpoints for the source file by passing down an
# empty lines array
lines = []
response = self.vscode.request_setBreakpoints(source_path, lines)
response = self.vscode.request_setBreakpoints(self.main_path, lines)
if response:
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), len(lines),
@ -124,7 +193,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
# Now set a breakpoint again in the same source file and verify it
# was added.
lines = [second_line]
response = self.vscode.request_setBreakpoints(source_path, lines)
response = self.vscode.request_setBreakpoints(self.main_path, lines)
if response:
breakpoints = response['body']['breakpoints']
self.assertEquals(len(breakpoints), len(lines),
@ -155,15 +224,13 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
def test_functionality(self):
'''Tests hitting breakpoints and the functionality of a single
breakpoint, like 'conditions' and 'hitCondition' settings.'''
source_basename = 'main.cpp'
source_path = os.path.join(os.getcwd(), source_basename)
loop_line = line_number('main.cpp', '// break loop')
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
# Set a breakpoint at the loop line with no condition and no
# hitCondition
breakpoint_ids = self.set_source_breakpoints(source_path, [loop_line])
breakpoint_ids = self.set_source_breakpoints(self.main_path, [loop_line])
self.assertEquals(len(breakpoint_ids), 1, "expect one breakpoint")
self.vscode.request_continue()
@ -175,7 +242,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
self.assertEquals(i, 0, 'i != 0 after hitting breakpoint')
# Update the condition on our breakpoint
new_breakpoint_ids = self.set_source_breakpoints(source_path,
new_breakpoint_ids = self.set_source_breakpoints(self.main_path,
[loop_line],
condition="i==4")
self.assertEquals(breakpoint_ids, new_breakpoint_ids,
@ -187,7 +254,7 @@ class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
self.assertEquals(i, 4,
'i != 4 showing conditional works')
new_breakpoint_ids = self.set_source_breakpoints(source_path,
new_breakpoint_ids = self.set_source_breakpoints(self.main_path,
[loop_line],
hitCondition="2")

22
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
View File

@ -1,5 +1,6 @@
#include <stdio.h>
#include <dlfcn.h>
#include <stdexcept>
#include <stdio.h>
int twelve(int i) {
return 12 + i; // break 12
@ -15,6 +16,25 @@ namespace a {
}
}
int main(int argc, char const *argv[]) {
#if defined(__APPLE__)
const char *libother_name = "libother.dylib";
#else
const char *libother_name = "libother.so";
#endif
void *handle = dlopen(libother_name, RTLD_NOW);
if (handle == nullptr) {
fprintf(stderr, "%s\n", dlerror());
exit(1);
}
int (*foo)(int) = (int (*)(int))dlsym(handle, "foo");
if (foo == nullptr) {
fprintf(stderr, "%s\n", dlerror());
exit(2);
}
foo(12);
for (int i=0; i<10; ++i) {
int x = twelve(i) + thirteen(i) + a::fourteen(i); // break loop
}

5
lldb/test/API/tools/lldb-vscode/breakpoint/other.c Normal file
View File

@ -0,0 +1,5 @@
extern int foo(int x) {
int y = x + 42; // break other
int z = y + 42;
return z;
}

30
lldb/tools/lldb-vscode/JSONUtils.cpp
View File

@ -8,7 +8,10 @@
#include <algorithm>
#include "llvm/ADT/Optional.h"
#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/Path.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBBreakpointLocation.h"
@ -281,7 +284,9 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
// },
// "required": [ "verified" ]
// }
llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp,
llvm::Optional<llvm::StringRef> request_path,
llvm::Optional<uint32_t> request_line) {
// Each breakpoint location is treated as a separate breakpoint for VS code.
// They don't have the notion of a single breakpoint with multiple locations.
llvm::json::Object object;
@ -300,7 +305,7 @@ llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
// that is at least loaded in the current process.
lldb::SBBreakpointLocation bp_loc;
const auto num_locs = bp.GetNumLocations();
for (size_t i=0; i<num_locs; ++i) {
for (size_t i = 0; i < num_locs; ++i) {
bp_loc = bp.GetLocationAtIndex(i);
if (bp_loc.IsResolved())
break;
@ -309,6 +314,10 @@ llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
if (!bp_loc.IsResolved())
bp_loc = bp.GetLocationAtIndex(0);
auto bp_addr = bp_loc.GetAddress();
if (request_path)
object.try_emplace("source", CreateSource(*request_path));
if (bp_addr.IsValid()) {
auto line_entry = bp_addr.GetLineEntry();
const auto line = line_entry.GetLine();
@ -316,11 +325,16 @@ llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
object.try_emplace("line", line);
object.try_emplace("source", CreateSource(line_entry));
}
// We try to add request_line as a fallback
if (request_line)
object.try_emplace("line", *request_line);
return llvm::json::Value(std::move(object));
}
void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints) {
breakpoints.emplace_back(CreateBreakpoint(bp));
void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
llvm::Optional<llvm::StringRef> request_path,
llvm::Optional<uint32_t> request_line) {
breakpoints.emplace_back(CreateBreakpoint(bp, request_path, request_line));
}
// "Event": {
@ -481,6 +495,14 @@ llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
return llvm::json::Value(std::move(object));
}
llvm::json::Value CreateSource(llvm::StringRef source_path) {
llvm::json::Object source;
llvm::StringRef name = llvm::sys::path::filename(source_path);
EmplaceSafeString(source, "name", name);
EmplaceSafeString(source, "path", source_path);
return llvm::json::Value(std::move(source));
}
llvm::json::Value CreateSource(lldb::SBFrame &frame, int64_t &disasm_line) {
disasm_line = 0;
auto line_entry = frame.GetLineEntry();

52
lldb/tools/lldb-vscode/JSONUtils.h
View File

@ -184,28 +184,58 @@ void FillResponse(const llvm::json::Object &request,
void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
llvm::StringRef key);
/// Converts \a bp to a JSON value and appends all locations to the
/// Converts \a bp to a JSON value and appends the first valid location to the
/// \a breakpoints array.
///
/// \param[in] bp
/// A LLDB breakpoint object which will get all locations extracted
/// and converted into a JSON objects in the \a breakpoints array
/// A LLDB breakpoint object which will get the first valid location
/// extracted and converted into a JSON object in the \a breakpoints array
///
/// \param[in] breakpoints
/// A JSON array that will get a llvm::json::Value for \a bp
/// appended to it.
void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints);
///
/// \param[in] request_path
/// An optional source path to use when creating the "Source" object of this
/// breakpoint. If not specified, the "Source" object is created from the
/// breakpoint's address' LineEntry. It is useful to ensure the same source
/// paths provided by the setBreakpoints request are returned to the IDE.
///
/// \param[in] request_line
/// An optional line to use when creating the "Breakpoint" object to append.
/// It is used if the breakpoint has no valid locations.
/// It is useful to ensure the same line
/// provided by the setBreakpoints request are returned to the IDE as a
/// fallback.
void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
llvm::Optional<llvm::StringRef> request_path = llvm::None,
llvm::Optional<uint32_t> request_line = llvm::None);
/// Converts breakpoint location to a Visual Studio Code "Breakpoint"
/// JSON object and appends it to the \a breakpoints array.
///
/// \param[in] bp
/// A LLDB breakpoint object to convert into a JSON value
///
/// \param[in] request_path
/// An optional source path to use when creating the "Source" object of this
/// breakpoint. If not specified, the "Source" object is created from the
/// breakpoint's address' LineEntry. It is useful to ensure the same source
/// paths provided by the setBreakpoints request are returned to the IDE.
///
/// \param[in] request_line
/// An optional line to use when creating the resulting "Breakpoint" object.
/// It is used if the breakpoint has no valid locations.
/// It is useful to ensure the same line
/// provided by the setBreakpoints request are returned to the IDE as a
/// fallback.
///
/// \return
/// A "Breakpoint" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp);
llvm::json::Value
CreateBreakpoint(lldb::SBBreakpoint &bp,
llvm::Optional<llvm::StringRef> request_path = llvm::None,
llvm::Optional<uint32_t> request_line = llvm::None);
/// Create a "Event" JSON object using \a event_name as the event name
///
@ -263,6 +293,16 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
/// definition outlined by Microsoft.
llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry);
/// Create a "Source" object for a given source path.
///
/// \param[in] source_path
/// The path to the source to use when creating the "Source" object.
///
/// \return
/// A "Source" JSON object that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateSource(llvm::StringRef source_path);
/// Create a "Source" object for a given frame.
///
/// When there is no source file information for a stack frame, we will

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

@ -418,7 +418,16 @@ void EventThreadFunction() {
bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
auto bp_event = CreateEventObject("breakpoint");
llvm::json::Object body;
body.try_emplace("breakpoint", CreateBreakpoint(bp));
// As VSCode already knows the path of this breakpoint, we don't
// need to send it back as part of a "changed" event. This
// prevent us from sending to VSCode paths that should be source
// mapped. Note that CreateBreakpoint doesn't apply source mapping.
// Besides, the current implementation of VSCode ignores the
// "source" element of breakpoint events.
llvm::json::Value source_bp = CreateBreakpoint(bp);
source_bp.getAsObject()->erase("source");
body.try_emplace("breakpoint", source_bp);
body.try_emplace("reason", "changed");
bp_event.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
@ -1364,13 +1373,13 @@ void request_launch(const llvm::json::Object &request) {
llvm::sys::fs::set_current_path(debuggerRoot.data());
}
SetSourceMapFromArguments(*arguments);
// Run any initialize LLDB commands the user specified in the launch.json.
// This is run before target is created, so commands can't do anything with
// the targets - preRunCommands are run with the target.
g_vsc.RunInitCommands();
SetSourceMapFromArguments(*arguments);
lldb::SBError status;
g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
if (status.Fail()) {
@ -1766,13 +1775,14 @@ void request_setBreakpoints(const llvm::json::Object &request) {
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);
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);
AppendBreakpoint(src_bp.bp, response_breakpoints, path, src_bp.line);
g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
}
}