Make breakpoint stop reason more accurate

Author: satyajanga

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

@@ -171,3 +171,35 @@ 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_breakpoint_reason(self):
+        """Tests setting data breakpoints on variable. Verify that the breakpoint has the correct reason and description in the stopped event."""
+        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()
+        self.dap_server.get_local_variables()
+         # Test write watchpoints on x, arr[2]
+        response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+                # Test response from dataBreakpointInfo request.
+        self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4")
+        self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
+        dataBreakpoints = [
+            {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+        ]
+        set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+        self.assertEqual(
+            set_response["body"]["breakpoints"],
+            [{"verified": True}],
+        )
+
+        stopped_events = self.continue_to_next_stop()
+        self.assertEqual(len(stopped_events), 1)
+        stopped_event = stopped_events[0]
+        self.assertEqual(stopped_event["body"]["reason"], "data breakpoint")
+        self.assertEqual(stopped_event["body"]["description"], "data breakpoint 1.1")
+        x_val = self.dap_server.get_local_variable_value("x")
+        self.assertEqual(x_val, "2")

lldb/tools/lldb-dap/DAP.cpp

@@ -183,17 +183,6 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
   return nullptr;
 }
 
-ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
-  // See comment in the other GetExceptionBreakpoint().
-  PopulateExceptionBreakpoints();
-
-  for (auto &bp : *exception_breakpoints) {
-    if (bp.bp.GetID() == bp_id)
-      return &bp;
-  }
-  return nullptr;
-}
-
 llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
   in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true);
 
@@ -444,27 +433,6 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) {
       o, llvm::StringRef(buffer, std::min<int>(actual_length, sizeof(buffer))));
 }
 
-ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
-  const auto num = thread.GetStopReasonDataCount();
-  // Check to see if have hit an exception breakpoint and change the
-  // reason to "exception", but only do so if all breakpoints that were
-  // hit are exception breakpoints.
-  ExceptionBreakpoint *exc_bp = nullptr;
-  for (size_t i = 0; i < num; i += 2) {
-    // thread.GetStopReasonDataAtIndex(i) will return the bp ID and
-    // thread.GetStopReasonDataAtIndex(i+1) will return the location
-    // within that breakpoint. We only care about the bp ID so we can
-    // see if this is an exception breakpoint that is getting hit.
-    lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
-    exc_bp = GetExceptionBreakpoint(bp_id);
-    // If any breakpoint is not an exception breakpoint, then stop and
-    // report this as a normal breakpoint
-    if (exc_bp == nullptr)
-      return nullptr;
-  }
-  return exc_bp;
-}
-
 lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
   auto tid = GetInteger<int64_t>(arguments, "threadId")
                  .value_or(LLDB_INVALID_THREAD_ID);
@@ -1074,32 +1042,6 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) {
   return inst_bp;
 }
 
-FunctionBreakpoint *DAP::GetFunctionBPFromStopReason(lldb::SBThread &thread) {
-  const auto num = thread.GetStopReasonDataCount();
-  FunctionBreakpoint *func_bp = nullptr;
-  for (size_t i = 0; i < num; i += 2) {
-    // thread.GetStopReasonDataAtIndex(i) will return the bp ID and
-    // thread.GetStopReasonDataAtIndex(i+1) will return the location
-    // within that breakpoint. We only care about the bp ID so we can
-    // see if this is an function breakpoint that is getting hit.
-    lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
-    func_bp = GetFunctionBreakPoint(bp_id);
-    // If any breakpoint is not an function breakpoint, then stop and
-    // report this as a normal breakpoint
-    if (func_bp == nullptr)
-      return nullptr;
-  }
-  return func_bp;
-}
-
-FunctionBreakpoint *DAP::GetFunctionBreakPoint(const lldb::break_id_t bp_id) {
-  for (auto &bp : function_breakpoints) {
-    if (bp.second.bp.GetID() == bp_id)
-      return &bp.second;
-  }
-  return nullptr;
-}
-
 lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
   switch (variablesReference) {
   case VARREF_LOCALS:

lldb/tools/lldb-dap/DAP.h

@@ -392,6 +392,66 @@ struct DAP {
 
   void SetThreadFormat(llvm::StringRef format);
 
+  template <typename BreakpointType>
+  BreakpointType *GetBreakpointFromStopReason(lldb::SBThread &thread) {
+    // Check to see if have hit the <BreakpointType> breakpoint and change the
+    // reason accordingly, but only do so if all breakpoints that were
+    // hit are of <BreakpointType>.
+    const auto num = thread.GetStopReasonDataCount();
+    BreakpointType *bp = nullptr;
+    for (size_t i = 0; i < num; i += 2) {
+      lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
+      // If any breakpoint is not the <BreakpointType>, then stop and
+      // report this as a normal breakpoint
+      bp = GetBreakpoint<BreakpointType>(bp_id);
+      if (bp == nullptr)
+        return nullptr;
+    }
+    return bp;
+  }
+
+  template <typename BreakpointType>
+  BreakpointType *GetBreakpoint(const lldb::break_id_t bp_id);
+
+  template <>
+  FunctionBreakpoint *
+  GetBreakpoint<FunctionBreakpoint>(const lldb::break_id_t bp_id) {
+    for (auto &bp : function_breakpoints) {
+      if (bp.second.bp.GetID() == bp_id)
+        return &bp.second;
+    }
+    return nullptr;
+  }
+
+  template <>
+  InstructionBreakpoint *
+  GetBreakpoint<InstructionBreakpoint>(const lldb::break_id_t bp_id) {
+    for (auto &bp : instruction_breakpoints) {
+      if (bp.second.bp.GetID() == bp_id)
+        return &bp.second;
+    }
+    return nullptr;
+  }
+
+  template <>
+  ExceptionBreakpoint *
+  GetBreakpoint<ExceptionBreakpoint>(const lldb::break_id_t bp_id) {
+    // See comment in the other GetExceptionBreakpoint().
+    PopulateExceptionBreakpoints();
+
+    for (auto &bp : *exception_breakpoints) {
+      if (bp.bp.GetID() == bp_id)
+        return &bp;
+    }
+    return nullptr;
+  }
+
+  FunctionBreakpoint *GetFunctionBPFromStopReason(lldb::SBThread &thread);
+
+  FunctionBreakpoint *GetFunctionBreakPoint(const lldb::break_id_t bp_id);
+
+  void WaitWorkerThreadsToExit();
+
 private:
   // Send the JSON in "json_str" to the "out" stream. Correctly send the
   // "Content-Length:" field followed by the length, followed by the raw

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

@@ -123,7 +123,8 @@ void ExceptionInfoRequestHandler::operator()(
     if (stopReason == lldb::eStopReasonSignal)
       body.try_emplace("exceptionId", "signal");
     else if (stopReason == lldb::eStopReasonBreakpoint) {
-      ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
+      ExceptionBreakpoint *exc_bp =
+          dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread);
       if (exc_bp) {
         EmplaceSafeString(body, "exceptionId", exc_bp->filter);
         EmplaceSafeString(body, "description", exc_bp->label);

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -962,19 +962,20 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
     body.try_emplace("reason", "step");
     break;
   case lldb::eStopReasonBreakpoint: {
-    ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
+    ExceptionBreakpoint *exc_bp =
+        dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread);
     if (exc_bp) {
       body.try_emplace("reason", "exception");
       EmplaceSafeString(body, "description", exc_bp->label);
     } else {
-      std::string reason = "breakpoint";
+      llvm::StringRef reason = "breakpoint";
       InstructionBreakpoint *inst_bp =
-          dap.GetInstructionBPFromStopReason(thread);
+          dap.GetBreakpointFromStopReason<InstructionBreakpoint>(thread);
       if (inst_bp) {
         reason = "instruction breakpoint";
       } else {
         FunctionBreakpoint *function_bp =
-            dap.GetFunctionBPFromStopReason(thread);
+            dap.GetBreakpointFromStopReason<FunctionBreakpoint>(thread);
         if (function_bp) {
           reason = "function breakpoint";
         }
@@ -989,7 +990,15 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
       EmplaceSafeString(body, "description", desc_str);
     }
   } break;
-  case lldb::eStopReasonWatchpoint:
+  case lldb::eStopReasonWatchpoint: {
+    // Assuming that all watch points are data breakpoints.
+    body.try_emplace("reason", "data breakpoint");
+    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);
+    EmplaceSafeString(body, "description", desc_str);
+  } break;
   case lldb::eStopReasonInstrumentation:
     body.try_emplace("reason", "breakpoint");
     break;