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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
