[lldb][lldb-dap] review changes.

[lldb][lldb-dap] add review changes

Author: da-viper

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -1044,10 +1044,10 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non
     def request_dataBreakpointInfo(
         self,
         name: str,
-        variablesReference: int = None,
-        frameIndex: int = 0,
-        bytes_: int = None,
-        asAddress: bool = None,
+            variablesReference: Optional[int] | None = None,
+            frameIndex: Optional[int] = 0,
+            bytes_: Optional[int] = None,
+            asAddress: Optional[bool] = None,
     ):
         args_dict = {}
         if asAddress is not None:
@@ -1057,9 +1057,12 @@ def request_dataBreakpointInfo(
                 "bytes": bytes_,
             }
         else:
-            stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=None)
+            thread_id = self.get_thread_id()
+            stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=thread_id)
             if stackFrame is None:
-                return []
+                raise ValueError(
+                    f"could not get stackframe for frameIndex: {frameIndex} and threadId: {thread_id}"
+                )
             args_dict = {
                 "variablesReference": variablesReference,
                 "name": name,

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -105,7 +105,7 @@ def waitUntil(self, condition_callback):
         return False
 
     def verify_breakpoint_hit(
-        self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, is_watchpoint=False
+            self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
     ):
         """Wait for the process we are debugging to stop, and verify we hit
         any breakpoint location in the "breakpoint_ids" array.
@@ -118,9 +118,10 @@ def verify_breakpoint_hit(
                 body = stopped_event["body"]
                 if "reason" not in body:
                     continue
-                if (
-                    body["reason"] != "breakpoint"
-                    and body["reason"] != "instruction breakpoint"
+                if body["reason"] not in (
+                        "breakpoint",
+                        "instruction breakpoint",
+                        "data breakpoint",
                 ):
                     continue
                 if "description" not in body:
@@ -133,7 +134,7 @@ def verify_breakpoint_hit(
                 # So when looking at the description we just want to make sure
                 # the right breakpoint matches and not worry about the actual
                 # location.
-                type_name = "watchpoint" if is_watchpoint else "breakpoint"
+                type_name = reason or "breakpoint"
                 description = body["description"]
                 for breakpoint_id in breakpoint_ids:
                     match_desc = f"{type_name} {breakpoint_id}"
@@ -333,15 +334,15 @@ def continue_to_next_stop(self, timeout=DEFAULT_TIMEOUT):
         return self.dap_server.wait_for_stopped(timeout)
 
     def continue_to_breakpoint(
-        self, breakpoint_id: str, timeout=DEFAULT_TIMEOUT, is_watchpoint=False
+            self, breakpoint_id: str, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
     ):
-        self.continue_to_breakpoints([breakpoint_id], timeout, is_watchpoint)
+        self.continue_to_breakpoints([breakpoint_id], timeout, reason)
 
     def continue_to_breakpoints(
-        self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, is_watchpoint=False
+            self, breakpoint_ids, timeout=DEFAULT_TIMEOUT, reason: Optional[str] = None
     ):
         self.do_continue()
-        self.verify_breakpoint_hit(breakpoint_ids, timeout, is_watchpoint)
+        self.verify_breakpoint_hit(breakpoint_ids, timeout, reason)
 
     def continue_to_exception_breakpoint(self, filter_label, timeout=DEFAULT_TIMEOUT):
         self.do_continue()

lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py

@@ -77,16 +77,16 @@ def test_breakpoint_info_bytes(self):
         bp_resp = self.dap_server.request_dataBreakpointInfo(
             var_address, asAddress=True, bytes_=var_byte_watch_size
         )
-        resp_data_id = bp_resp["body"]["dataId"]
         self.assertTrue(
             bp_resp["success"], f"dataBreakpointInfo request failed: {bp_resp}"
         )
+        resp_data_id = bp_resp["body"]["dataId"]
         self.assertEqual(resp_data_id.split("/")[1], str(var_byte_watch_size))
 
         data_breakpoints = [{"dataId": resp_data_id, "accessType": "write"}]
         self.dap_server.request_setDataBreakpoint(data_breakpoints)
 
-        self.continue_to_breakpoint(breakpoint_id=1, is_watchpoint=True)
+        self.continue_to_breakpoint(breakpoint_id=1, reason="data breakpoint")
         eval_response = self.dap_server.request_evaluate("var", context="watch")
         self.assertTrue(eval_response["success"])
         var_value = eval_response["body"]["result"]

lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp

@@ -18,16 +18,15 @@ namespace lldb_dap {
 static llvm::Expected<protocol::DataBreakpointInfoResponseBody>
 HandleDataBreakpointBytes(DAP &dap,
                           const protocol::DataBreakpointInfoArguments &args) {
-  llvm::StringRef address = args.name;
+  llvm::StringRef raw_address = args.name;
 
-  unsigned long long load_addr = LLDB_INVALID_ADDRESS;
-  if (llvm::getAsUnsignedInteger(address, 0, load_addr)) {
+  lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
+  if (raw_address.getAsInteger<lldb::addr_t>(0, load_addr)) {
     return llvm::make_error<DAPError>(llvm::formatv("invalid address"),
                                       llvm::inconvertibleErrorCode(), false);
   }
 
-  lldb::SBAddress sb_addr(load_addr, dap.target);
-  if (!sb_addr.IsValid()) {
+  if (lldb::SBAddress address(load_addr, dap.target); !address.IsValid()) {
     return llvm::make_error<DAPError>(
         llvm::formatv("address {:x} does not exist in the debuggee", load_addr),
         llvm::inconvertibleErrorCode(), false);

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -893,7 +893,16 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
       EmplaceSafeString(body, "description", desc_str);
     }
   } break;
-  case lldb::eStopReasonWatchpoint:
+  case lldb::eStopReasonWatchpoint: {
+    lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0);
+    lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
+    std::string desc_str =
+        llvm::formatv("data breakpoint {0}.{1}", bp_id, bp_loc_id);
+    body.try_emplace("hitBreakpointIds",
+                     llvm::json::Array{llvm::json::Value(bp_id)});
+    body.try_emplace("reason", "data breakpoint");
+    EmplaceSafeString(body, "description", desc_str);
+  } break;
   case lldb::eStopReasonInstrumentation:
     body.try_emplace("reason", "breakpoint");
     break;