Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,14 @@ def get_local_variable_child(
return child
return None

def get_source_for_source_reference(self, source_reference):
return {"sourceReference": source_reference}

def get_source_for_path(self, file_path):
(dir, base) = os.path.split(file_path)
source_dict = {"name": base, "path": file_path}
return source_dict

def replay_packets(self, replay_file_path):
f = open(replay_file_path, "r")
mode = "invalid"
Expand Down Expand Up @@ -948,13 +956,11 @@ def request_scopes(self, frameId):
command_dict = {"command": "scopes", "type": "request", "arguments": args_dict}
return self.send_recv(command_dict)

def request_setBreakpoints(self, file_path, line_array, data=None):
def request_setBreakpoints(self, source_dict, line_array, data=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

from typing import TypedDict, Optional

class Source(TypedDict, total=False): # At runtime, this is just a dict with well known keys
  name: str
  path: str
  sourceReference: int


class SourceBreakpoint(TypedDict, total=False):
  line: int
  column: str
  condition: str
  hitCondition: str
  logMessage: str


def request_setBreakpoints(
   self,
   # For backwards compatibility, leaving these as positional args
   sourcePath: Optional[str] = None,
   lines: Optional[list[int]] = None,
   breakpoints: Optional[list[SourceBreakpoint]] = None,
   *,
   source: Optional[Source] = None,
   sourceReference: Optional[int] = None,
):
  source_dict = {}
  if source is not None:
     source_dict = source
  elif sourcePath is not None:
    source_dict["name"] = os.path.basename(sourcePath)
    source_dict["pah"] = sourcePath
  elif sourceReference is not None:
    source_dict["sourceReference"] = sourceReference
  else:
    raise ValueError("'source', 'sourcePath' or 'sourceReference' must be set')

  args_dict = {
    "source": source_dict,
    "sourceModified": False,
  }
  ...

Then we wouldn't need the get_source_for_path and get_source_for_source_reference helpers, since those would be built based on the args of the request.

I could also look at adding this as a follow up to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea, I added a simple Source class for this, let me know what you think

"""data is array of parameters for breakpoints in line_array.
Each parameter object is 1:1 mapping with entries in line_entry.
It contains optional location/hitCondition/logMessage parameters.
"""
(dir, base) = os.path.split(file_path)
source_dict = {"name": base, "path": file_path}
args_dict = {
"source": source_dict,
"sourceModified": False,
Expand Down Expand Up @@ -1381,7 +1387,9 @@ def run_vscode(dbg, args, options):
else:
source_to_lines[path] = [int(line)]
for source in source_to_lines:
dbg.request_setBreakpoints(source, source_to_lines[source])
dbg.request_setBreakpoints(
dbg.get_source_for_path(source), source_to_lines[source]
)
if options.funcBreakpoints:
dbg.request_setFunctionBreakpoints(options.funcBreakpoints)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def set_source_breakpoints(self, source_path, lines, data=None):
Each object in data is 1:1 mapping with the entry in lines.
It contains optional location/hitCondition/logMessage parameters.
"""
response = self.dap_server.request_setBreakpoints(source_path, lines, data)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(source_path), lines, data
)
if response is None or not response["success"]:
return []
breakpoints = response["body"]["breakpoints"]
Expand All @@ -64,6 +66,20 @@ def set_source_breakpoints(self, source_path, lines, data=None):
breakpoint_ids.append("%i" % (breakpoint["id"]))
return breakpoint_ids

def set_source_breakpoints_assembly(self, source_reference, lines, data=None):
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_source_reference(source_reference),
lines,
data,
)
if response is None:
return []
breakpoints = response["body"]["breakpoints"]
breakpoint_ids = []
for breakpoint in breakpoints:
breakpoint_ids.append("%i" % (breakpoint["id"]))
return breakpoint_ids

def set_function_breakpoints(self, functions, condition=None, hitCondition=None):
"""Sets breakpoints by function name given an array of function names
and returns an array of strings containing the breakpoint IDs
Expand Down
3 changes: 3 additions & 0 deletions lldb/test/API/tools/lldb-dap/breakpoint-assembly/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
C_SOURCES := main.c

include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
"""
Test lldb-dap setBreakpoints request in assembly source references.
"""


from lldbsuite.test.decorators import *
import lldbdap_testcase


# @skipIfWindows
class TestDAP_setBreakpointsAssembly(lldbdap_testcase.DAPTestCaseBase):
def test_can_break_in_source_references(self):
"""Tests hitting assembly source breakpoints"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)

assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"])
self.continue_to_breakpoints(assmebly_func_breakpoints)

assembly_func_frame = self.get_stackFrames()[0]
self.assertIn(
"sourceReference",
assembly_func_frame.get("source"),
"Expected assembly source frame",
)

line = assembly_func_frame["line"]

# Set an assembly breakpoint in the next line and check that it's hit
source_reference = assembly_func_frame["source"]["sourceReference"]
assembly_breakpoint_ids = self.set_source_breakpoints_assembly(
source_reference, [line + 1]
)
self.continue_to_breakpoints(assembly_breakpoint_ids)

# Continue again and verify it hits in the next function call
self.continue_to_breakpoints(assmebly_func_breakpoints)
self.continue_to_breakpoints(assembly_breakpoint_ids)

# Clear the breakpoint and then check that the assembly breakpoint does not hit next time
self.set_source_breakpoints_assembly(source_reference, [])
self.continue_to_breakpoints(assmebly_func_breakpoints)
self.continue_to_exit()

def test_break_on_invalid_source_reference(self):
"""Tests hitting assembly source breakpoints"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)

# Verify that setting a breakpoint on an invalid source reference fails
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_source_reference(-1), [1]
)
self.assertIsNotNone(response)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(len(breakpoints), 1)
breakpoint = breakpoints[0]
self.assertFalse(
breakpoint["verified"], "Expected breakpoint to not be verified"
)
self.assertIn("message", breakpoint, "Expected message to be present")
self.assertEqual(
breakpoint["message"],
"Invalid sourceReference.",
)

# Verify that setting a breakpoint on a source reference without a symbol also fails
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_source_reference(0), [1]
)
self.assertIsNotNone(response)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(len(breakpoints), 1)
breakpoint = breakpoints[0]
self.assertFalse(
breakpoint["verified"], "Expected breakpoint to not be verified"
)
self.assertIn("message", breakpoint, "Expected message to be present")
self.assertEqual(
breakpoint["message"],
"Breakpoints in assembly without a valid symbol are not supported yet.",
)
14 changes: 14 additions & 0 deletions lldb/test/API/tools/lldb-dap/breakpoint-assembly/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
__attribute__((nodebug)) int assembly_func(int n) {
n += 1;
n += 2;
n += 3;

return n;
}

int main(int argc, char const *argv[]) {
assembly_func(10);
assembly_func(20);
assembly_func(30);
return 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test_breakpoint_events(self):
# Set breakpoints and verify that they got set correctly
dap_breakpoint_ids = []
response = self.dap_server.request_setBreakpoints(
main_source_path, [main_bp_line]
self.dap_server.get_source_for_path(main_source_path), [main_bp_line]
)
self.assertTrue(response["success"])
breakpoints = response["body"]["breakpoints"]
Expand All @@ -70,7 +70,7 @@ def test_breakpoint_events(self):
)

response = self.dap_server.request_setBreakpoints(
foo_source_path, [foo_bp1_line]
self.dap_server.get_source_for_path(foo_source_path), [foo_bp1_line]
)
self.assertTrue(response["success"])
breakpoints = response["body"]["breakpoints"]
Expand Down
40 changes: 30 additions & 10 deletions lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ def test_source_map(self):
self.launch(program, sourceMap=source_map)

# breakpoint in main.cpp
response = self.dap_server.request_setBreakpoints(new_main_path, [main_line])
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(new_main_path), [main_line]
)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(len(breakpoints), 1)
breakpoint = breakpoints[0]
Expand All @@ -68,7 +70,9 @@ def test_source_map(self):
self.assertEqual(new_main_path, breakpoint["source"]["path"])

# 2nd breakpoint, which is from a dynamically loaded library
response = self.dap_server.request_setBreakpoints(new_other_path, [other_line])
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(new_other_path), [other_line]
)
breakpoints = response["body"]["breakpoints"]
breakpoint = breakpoints[0]
self.assertEqual(breakpoint["line"], other_line)
Expand All @@ -81,7 +85,9 @@ def test_source_map(self):
self.verify_breakpoint_hit([other_breakpoint_id])

# 2nd breakpoint again, which should be valid at this point
response = self.dap_server.request_setBreakpoints(new_other_path, [other_line])
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(new_other_path), [other_line]
)
breakpoints = response["body"]["breakpoints"]
breakpoint = breakpoints[0]
self.assertEqual(breakpoint["line"], other_line)
Expand Down Expand Up @@ -124,7 +130,9 @@ def test_set_and_clear(self):
self.build_and_launch(program)

# Set 3 breakpoints and verify that they got set correctly
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
line_to_id = {}
breakpoints = response["body"]["breakpoints"]
self.assertEqual(
Expand All @@ -149,7 +157,9 @@ def test_set_and_clear(self):
lines.remove(second_line)
# Set 2 breakpoints and verify that the previous breakpoints that were
# set above are still set.
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(
len(breakpoints),
Expand Down Expand Up @@ -194,7 +204,9 @@ def test_set_and_clear(self):
# Now clear all breakpoints for the source file by passing down an
# empty lines array
lines = []
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(
len(breakpoints),
Expand All @@ -214,7 +226,9 @@ def test_set_and_clear(self):
# Now set a breakpoint again in the same source file and verify it
# was added.
lines = [second_line]
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
if response:
breakpoints = response["body"]["breakpoints"]
self.assertEqual(
Expand Down Expand Up @@ -265,7 +279,9 @@ def test_clear_breakpoints_unset_breakpoints(self):
self.build_and_launch(program)

# Set one breakpoint and verify that it got set correctly.
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
line_to_id = {}
breakpoints = response["body"]["breakpoints"]
self.assertEqual(
Expand All @@ -281,7 +297,9 @@ def test_clear_breakpoints_unset_breakpoints(self):
# Now clear all breakpoints for the source file by not setting the
# lines array.
lines = None
response = self.dap_server.request_setBreakpoints(self.main_path, lines)
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), lines
)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(len(breakpoints), 0, "expect no source breakpoints")

Expand Down Expand Up @@ -357,7 +375,9 @@ def test_column_breakpoints(self):
# Set two breakpoints on the loop line at different columns.
columns = [13, 39]
response = self.dap_server.request_setBreakpoints(
self.main_path, [loop_line, loop_line], list({"column": c} for c in columns)
self.dap_server.get_source_for_path(self.main_path),
[loop_line, loop_line],
list({"column": c} for c in columns),
)

# Verify the breakpoints were set correctly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ def instruction_breakpoint_test(self):
self.build_and_launch(program)

# Set source breakpoint 1
response = self.dap_server.request_setBreakpoints(self.main_path, [main_line])
response = self.dap_server.request_setBreakpoints(
self.dap_server.get_source_for_path(self.main_path), [main_line]
)
breakpoints = response["body"]["breakpoints"]
self.assertEqual(len(breakpoints), 1)
breakpoint = breakpoints[0]
Expand Down
33 changes: 26 additions & 7 deletions lldb/tools/lldb-dap/Breakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
#include "Breakpoint.h"
#include "DAP.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "lldb/API/SBAddress.h"
#include "lldb/API/SBBreakpointLocation.h"
#include "lldb/API/SBLineEntry.h"
#include "lldb/API/SBMutex.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringExtras.h"
#include <cstddef>
#include <cstdint>
Expand Down Expand Up @@ -63,14 +65,31 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
std::string formatted_addr =
"0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
breakpoint.instructionReference = formatted_addr;

lldb::StopDisassemblyType stop_disassembly_display =
GetStopDisassemblyDisplay(m_dap.debugger);
auto line_entry = bp_addr.GetLineEntry();
const auto line = line_entry.GetLine();
if (line != UINT32_MAX)
breakpoint.line = line;
const auto column = line_entry.GetColumn();
if (column != 0)
breakpoint.column = column;
breakpoint.source = CreateSource(line_entry);
if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) {
const auto line = line_entry.GetLine();
if (line != LLDB_INVALID_LINE_NUMBER)
breakpoint.line = line;
const auto column = line_entry.GetColumn();
if (column != LLDB_INVALID_COLUMN_NUMBER)
breakpoint.column = column;
breakpoint.source = CreateSource(line_entry);
} else {
// Breakpoint made by assembly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Breakpoint made by assembly
// Assembly breakpoint.

auto symbol = bp_addr.GetSymbol();
if (symbol.IsValid()) {
breakpoint.line =
m_bp.GetTarget()
.ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr)
.GetSize() +
1;

breakpoint.source = CreateAssemblySource(m_dap.target, bp_addr);
}
}
}

return breakpoint;
Expand Down
Loading
Loading