wallace created this revision. wallace added reviewers: clayborg, kusmour, labath, aadsm. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. wallace updated this revision to Diff 253283. wallace added a comment.
cleanup 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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76968 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py lldb/tools/lldb-vscode/JSONUtils.cpp lldb/tools/lldb-vscode/JSONUtils.h lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp =================================================================== --- lldb/tools/lldb-vscode/lldb-vscode.cpp +++ lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -1364,15 +1364,17 @@ 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()) { response["success"] = llvm::json::Value(false); EmplaceSafeString(response, "message", status.GetCString()); @@ -1748,13 +1750,15 @@ 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); + AppendSourceBreakpoint(existing_bp->second.bp, path, src_bp.line, + response_breakpoints); continue; } } // At this point the breakpoint is new src_bp.SetBreakpoint(path.data()); - AppendBreakpoint(src_bp.bp, response_breakpoints); + AppendSourceBreakpoint(src_bp.bp, path, src_bp.line, + response_breakpoints); g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp); } } Index: lldb/tools/lldb-vscode/JSONUtils.h =================================================================== --- lldb/tools/lldb-vscode/JSONUtils.h +++ lldb/tools/lldb-vscode/JSONUtils.h @@ -196,6 +196,26 @@ /// appended to it. void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints); +/// Converts \a bp to a JSON value using the provided source line information to +/// the \a breakpoints array. +/// +/// \param[in] bp +/// A LLDB breakpoint object which will provide basic information as ID and +/// resolved status. +/// +/// \param[in] sourcePath +/// The full path to the source to store in the JSON value. +/// +/// \param[in] line +/// The line to store in the JSON value. +/// +/// \param[in] breakpoints +/// A JSON array that will get a llvm::json::Value for \a bp +/// appended to it. +void AppendSourceBreakpoint(lldb::SBBreakpoint &bp, + const llvm::StringRef &sourcePath, int line, + llvm::json::Array &breakpoints); + /// Converts breakpoint location to a Visual Studio Code "Breakpoint" /// JSON object and appends it to the \a breakpoints array. /// Index: lldb/tools/lldb-vscode/JSONUtils.cpp =================================================================== --- lldb/tools/lldb-vscode/JSONUtils.cpp +++ lldb/tools/lldb-vscode/JSONUtils.cpp @@ -12,6 +12,7 @@ #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBBreakpointLocation.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBValue.h" #include "lldb/Host/PosixApi.h" @@ -319,10 +320,37 @@ return llvm::json::Value(std::move(object)); } +llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint &bp, + const llvm::StringRef &sourcePath, + int line) { + llvm::json::Object object; + if (!bp.IsValid()) + return llvm::json::Value(std::move(object)); + + object.try_emplace("verified", bp.GetNumResolvedLocations() > 0); + object.try_emplace("id", bp.GetID()); + + object.try_emplace("line", line); + + llvm::json::Object source; + lldb::SBFileSpec file(sourcePath.str().c_str()); + const char *name = file.GetFilename(); + EmplaceSafeString(source, "name", name); + EmplaceSafeString(source, "path", sourcePath); + object.try_emplace("source", llvm::json::Value(std::move(source))); + return llvm::json::Value(std::move(object)); +} + void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints) { breakpoints.emplace_back(CreateBreakpoint(bp)); } +void AppendSourceBreakpoint(lldb::SBBreakpoint &bp, + const llvm::StringRef &sourcePath, int line, + llvm::json::Array &breakpoints) { + breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line)); +} + // "Event": { // "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, { // "type": "object", Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py =================================================================== --- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py +++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py @@ -5,6 +5,7 @@ import unittest2 import vscode +import shutil from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil @@ -18,6 +19,45 @@ @skipIfWindows @skipIfRemote + def test_source_map(self): + self.build_and_create_debug_adaptor() + + source_basename = 'main.cpp' + source_path = os.path.join(os.getcwd(), source_basename) + + new_source_folder = os.path.join(os.path.dirname(os.getcwd()), 'moved_location') + new_source_path = os.path.join(new_source_folder, source_basename) + + def cleanup(): + shutil.move(new_source_path, source_path) + os.rmdir(new_source_folder) + + self.addTearDownHook(cleanup) + + lines = [line_number('main.cpp', 'break 12')] + + os.mkdir(new_source_folder) + shutil.move(source_path, new_source_path) + + program = self.getBuildArtifact("a.out") + self.launch(program, sourceMap=[[os.getcwd(), new_source_folder]]) + + response = self.vscode.request_setBreakpoints(new_source_path, lines) + if response: + 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]) + self.assertTrue(line in lines, "line expected in lines array") + self.assertTrue(breakpoint['verified'], + "expect breakpoint verified") + self.assertEqual('main.cpp', breakpoint['source']['name']) + self.assertEqual(new_source_path, breakpoint['source']['path']) + + @skipIfWindows + @skipIfRemote def test_set_and_clear(self): '''Tests setting and clearing source file and line breakpoints. This packet is a bit tricky on the debug adaptor side since there Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py =================================================================== --- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py +++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py @@ -570,7 +570,7 @@ 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 @@ args_dict['debuggerRoot'] = debuggerRoot if launchCommands: args_dict['launchCommands'] = launchCommands + if sourceMap: + args_dict['sourceMap'] = sourceMap command_dict = { 'command': 'launch', 'type': 'request', Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py =================================================================== --- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -266,8 +266,8 @@ 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 @@ 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']))
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits