wallace updated this revision to Diff 254280.
wallace marked 2 inline comments as done.
wallace added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76968/new/
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/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
lldb/test/API/tools/lldb-vscode/breakpoint/other.c
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
@@ -418,7 +418,16 @@
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 @@
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()) {
@@ -1748,13 +1757,14 @@
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);
}
}
Index: lldb/tools/lldb-vscode/JSONUtils.h
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -184,28 +184,58 @@
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 @@
/// 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
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -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 @@
// },
// "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 @@
// 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 @@
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 @@
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 @@
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();
Index: lldb/test/API/tools/lldb-vscode/breakpoint/other.c
===================================================================
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint/other.c
@@ -0,0 +1,5 @@
+extern int foo(int x) {
+ int y = x + 42; // break other
+ int z = y + 42;
+ return z;
+}
Index: lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
===================================================================
--- lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
+++ lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp
@@ -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 @@
}
}
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
}
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
@@ -16,6 +17,76 @@
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 @@
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 @@
self.build_and_launch(program)
# Set 3 breakoints 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 @@
lines.remove(second_line)
# Set 2 breakoints and verify that the previous breakoints 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 @@
# 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 @@
# 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 @@
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 @@
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 @@
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")
Index: lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
===================================================================
--- lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
+++ lldb/test/API/tools/lldb-vscode/breakpoint/Makefile
@@ -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
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
