Make breakpoint stop reason more accurate

Author: satyajanga

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

@@ -174,17 +174,19 @@ def test_functionality(self):
 
     @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."""
+        """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]
+        # Test write watchpoints on x
         response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
-                # Test response from dataBreakpointInfo request.
+        # Test response from dataBreakpointInfo request.
         self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4")
         self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
         dataBreakpoints = [

lldb/tools/lldb-dap/DAP.cpp

@@ -1014,34 +1014,6 @@ void DAP::SetThreadFormat(llvm::StringRef format) {
   }
 }
 
-InstructionBreakpoint *
-DAP::GetInstructionBreakpoint(const lldb::break_id_t bp_id) {
-  for (auto &bp : instruction_breakpoints) {
-    if (bp.second.bp.GetID() == bp_id)
-      return &bp.second;
-  }
-  return nullptr;
-}
-
-InstructionBreakpoint *
-DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) {
-  const auto num = thread.GetStopReasonDataCount();
-  InstructionBreakpoint *inst_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 instruction breakpoint that is getting hit.
-    lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
-    inst_bp = GetInstructionBreakpoint(bp_id);
-    // If any breakpoint is not an instruction breakpoint, then stop and
-    // report this as a normal breakpoint
-    if (inst_bp == nullptr)
-      return nullptr;
-  }
-  return inst_bp;
-}
-
 lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
   switch (variablesReference) {
   case VARREF_LOCALS:

lldb/tools/lldb-dap/DAP.h

@@ -233,7 +233,6 @@ struct DAP {
   /// @}
 
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
-  ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
 
   /// Redirect stdout and stderr fo the IDE's console output.
   ///
@@ -392,11 +391,20 @@ struct DAP {
 
   void SetThreadFormat(llvm::StringRef format);
 
+  // Check to see if we have hit the  <BreakpointType> breakpoint and return the
+  // last breakpoint. but only do so if all breakpoints that were hit are of
+  // <BreakpointType>.
+  // Caller uses this to set the reason accordingly. i.e. If all the breakpoints
+  // are exception breakpoints then reason is 'exception'; if all the
+  // breakpoints are function breakpoints then reason is 'function breakpoint';
+  // if all the breakpoints are instruction breakpoints then reason =
+  // 'instruction breakpoint'; if there are combination of one more breakpoints,
+  // then the reason will be 'breakpoint'.
   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>.
+    // Check to see if we 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) {
@@ -416,47 +424,31 @@ struct DAP {
   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;
+    auto it = std::find_if(
+        function_breakpoints.begin(), function_breakpoints.end(),
+        [bp_id](const auto &bp) { return bp.second.bp.GetID() == bp_id; });
+    return it != function_breakpoints.end() ? &it->second : 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;
+    auto it = std::find_if(
+        instruction_breakpoints.begin(), instruction_breakpoints.end(),
+        [bp_id](const auto &bp) { return bp.second.bp.GetID() == bp_id; });
+    return it != instruction_breakpoints.end() ? &it->second : 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;
+    auto it = std::find_if(
+        exception_breakpoints->begin(), exception_breakpoints->end(),
+        [bp_id](const auto &bp) { return bp.bp.GetID() == bp_id; });
+    return it != exception_breakpoints->end() ? &*it : 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
-  // JSON bytes.
-  void SendJSON(const std::string &json_str);
 };
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -962,19 +962,21 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
     body.try_emplace("reason", "step");
     break;
   case lldb::eStopReasonBreakpoint: {
-    ExceptionBreakpoint *exc_bp =
+    // Setting the reason as per DAP protocol spec.
+    // https://microsoft.github.io/debug-adapter-protocol/specification#Events_Stopped
+    const auto *exc_bp =
         dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread);
     if (exc_bp) {
       body.try_emplace("reason", "exception");
       EmplaceSafeString(body, "description", exc_bp->label);
     } else {
       llvm::StringRef reason = "breakpoint";
-      InstructionBreakpoint *inst_bp =
+      const auto *inst_bp =
           dap.GetBreakpointFromStopReason<InstructionBreakpoint>(thread);
       if (inst_bp) {
         reason = "instruction breakpoint";
       } else {
-        FunctionBreakpoint *function_bp =
+        const auto *function_bp =
             dap.GetBreakpointFromStopReason<FunctionBreakpoint>(thread);
         if (function_bp) {
           reason = "function breakpoint";
@@ -992,6 +994,8 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
   } break;
   case lldb::eStopReasonWatchpoint: {
     // Assuming that all watch points are data breakpoints.
+    // data breakpoints are DAP terminology for watchpoints.
+    // https://microsoft.github.io/debug-adapter-protocol/specification#Requests_SetDataBreakpoints
     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);