llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Sergei Druzhkov (DrSergei) <details> <summary>Changes</summary> This patch adds support for `dataBreakpointInfoBytes` capability from DAP. You can test this feature in VSCode (`Add data breakpoint at address` button in breakpoints tab). --- Full diff: https://github.com/llvm/llvm-project/pull/167237.diff 4 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+14-6) - (modified) lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py (+52) - (modified) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp (+31-13) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+3) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index ac550962cfb85..aaa6f76001aa3 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -1234,16 +1234,24 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non return response def request_dataBreakpointInfo( - self, variablesReference, name, frameIndex=0, threadId=None + self, variablesReference, name, size=None, frameIndex=0, threadId=None ): stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId) if stackFrame is None: return [] - args_dict = { - "variablesReference": variablesReference, - "name": name, - "frameId": stackFrame["id"], - } + args_dict = ( + { + "variablesReference": variablesReference, + "name": name, + "frameId": stackFrame["id"], + } + if size is None + else { + "bytes": size, + "name": name, + "asAddress": True, + } + ) command_dict = { "command": "dataBreakpointInfo", "type": "request", diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index a542a318050dd..7afc42f89ce55 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -171,3 +171,55 @@ def test_functionality(self): self.continue_to_next_stop() x_val = self.dap_server.get_local_variable_value("x") self.assertEqual(x_val, "10") + + @skipIfWindows + def test_bytes(self): + """Tests setting data breakpoints on memory range.""" + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.cpp" + first_loop_break_line = line_number(source, "// first loop breakpoint") + self.set_source_breakpoints(source, [first_loop_break_line]) + self.continue_to_next_stop() + # Test write watchpoints on x, arr[2] + x = self.dap_server.get_local_variable("x") + response_x = self.dap_server.request_dataBreakpointInfo( + 0, x["memoryReference"], 4 + ) + arr_2 = self.dap_server.get_local_variable_child("arr", "[2]") + response_arr_2 = self.dap_server.request_dataBreakpointInfo( + 0, arr_2["memoryReference"], 4 + ) + + # Test response from dataBreakpointInfo request. + self.assertEqual( + response_x["body"]["dataId"].split("/"), [x["memoryReference"][2:], "4"] + ) + self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes) + self.assertEqual( + response_arr_2["body"]["dataId"].split("/"), + [arr_2["memoryReference"][2:], "4"], + ) + self.assertEqual(response_arr_2["body"]["accessTypes"], self.accessTypes) + dataBreakpoints = [ + {"dataId": response_x["body"]["dataId"], "accessType": "write"}, + {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"}, + ] + set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints) + self.assertEqual( + set_response["body"]["breakpoints"], + [{"verified": True}, {"verified": True}], + ) + + self.continue_to_next_stop() + x_val = self.dap_server.get_local_variable_value("x") + i_val = self.dap_server.get_local_variable_value("i") + self.assertEqual(x_val, "2") + self.assertEqual(i_val, "1") + + self.continue_to_next_stop() + arr_2 = self.dap_server.get_local_variable_child("arr", "[2]") + i_val = self.dap_server.get_local_variable_value("i") + self.assertEqual(arr_2["value"], "42") + self.assertEqual(i_val, "2") + self.dap_server.request_setDataBreakpoint([]) diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp index 87b93fc999ecd..c55b165c6b783 100644 --- a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp @@ -16,6 +16,20 @@ namespace lldb_dap { +static bool CheckAddress(DAP &dap, lldb::addr_t load_addr) { + lldb::SBMemoryRegionInfo region; + lldb::SBError err = + dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region); + // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this + // request if SBProcess::GetMemoryRegionInfo returns error. + if (err.Success()) { + if (!(region.IsReadable() || region.IsWritable())) { + return false; + } + } + return true; +} + /// Obtains information on a possible data breakpoint that could be set on an /// expression or variable. Clients should only call this request if the /// corresponding capability supportsDataBreakpoints is true. @@ -23,7 +37,6 @@ llvm::Expected<protocol::DataBreakpointInfoResponseBody> DataBreakpointInfoRequestHandler::Run( const protocol::DataBreakpointInfoArguments &args) const { protocol::DataBreakpointInfoResponseBody response; - lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId); lldb::SBValue variable = dap.variables.FindVariable( args.variablesReference.value_or(0), args.name); std::string addr, size; @@ -43,7 +56,8 @@ DataBreakpointInfoRequestHandler::Run( addr = llvm::utohexstr(load_addr); size = llvm::utostr(byte_size); } - } else if (args.variablesReference.value_or(0) == 0 && frame.IsValid()) { + } else if (lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId); + args.variablesReference.value_or(0) == 0 && frame.IsValid()) { lldb::SBValue value = frame.EvaluateExpression(args.name.c_str()); if (value.GetError().Fail()) { lldb::SBError error = value.GetError(); @@ -58,17 +72,10 @@ DataBreakpointInfoRequestHandler::Run( if (data.IsValid()) { size = llvm::utostr(data.GetByteSize()); addr = llvm::utohexstr(load_addr); - lldb::SBMemoryRegionInfo region; - lldb::SBError err = - dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region); - // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this - // request if SBProcess::GetMemoryRegionInfo returns error. - if (err.Success()) { - if (!(region.IsReadable() || region.IsWritable())) { - is_data_ok = false; - response.description = "memory region for address " + addr + - " has no read or write permissions"; - } + if (!CheckAddress(dap, load_addr)) { + is_data_ok = false; + response.description = "memory region for address " + addr + + " has no read or write permissions"; } } else { is_data_ok = false; @@ -76,6 +83,17 @@ DataBreakpointInfoRequestHandler::Run( "unable to get byte size for expression: " + args.name; } } + } else if (args.asAddress) { + size = llvm::utostr(args.bytes.value_or(1)); + if (llvm::StringRef(args.name).starts_with("0x")) + addr = args.name.substr(2); + else + addr = llvm::utohexstr(std::stoull(args.name)); + if (!CheckAddress(dap, std::stoull(addr, 0, 16))) { + is_data_ok = false; + response.description = "memory region for address " + addr + + " has no read or write permissions"; + } } else { is_data_ok = false; response.description = "variable not found: " + args.name; diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index bc22133d92453..c9cafd33327a8 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -432,6 +432,9 @@ class DataBreakpointInfoRequestHandler public: using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "dataBreakpointInfo"; } + FeatureSet GetSupportedFeatures() const override { + return {protocol::eAdapterFeatureDataBreakpointBytes}; + } llvm::Expected<protocol::DataBreakpointInfoResponseBody> Run(const protocol::DataBreakpointInfoArguments &args) const override; }; `````````` </details> https://github.com/llvm/llvm-project/pull/167237 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
