From 60b6383993013e720092025032a5844caac03111 Mon Sep 17 00:00:00 2001
From: Ben McMorran <bemcmorr@microsoft.com>
Date: Wed, 12 Jul 2023 11:25:12 -0700
Subject: Debugger: Always clear existing breakpoints on setBreakpoints

Fixes: #25063
---
 Source/cmDebuggerBreakpointManager.cxx           | 21 ++++++++++++++++-----
 Source/cmDebuggerBreakpointManager.h             |  2 ++
 Tests/CMakeLib/testDebuggerBreakpointManager.cxx | 13 +++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Source/cmDebuggerBreakpointManager.cxx b/Source/cmDebuggerBreakpointManager.cxx
index 152f0f5..4ae6728 100644
--- a/Source/cmDebuggerBreakpointManager.cxx
+++ b/Source/cmDebuggerBreakpointManager.cxx
@@ -6,6 +6,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <memory>
+#include <utility>
 
 #include <cm3p/cppdap/optional.h>
 #include <cm3p/cppdap/session.h>
@@ -78,12 +79,14 @@ cmDebuggerBreakpointManager::HandleSetBreakpointsRequest(
     cmSystemTools::GetActualCaseForPath(request.source.path.value());
   const dap::array<dap::SourceBreakpoint> defaultValue{};
   const auto& breakpoints = request.breakpoints.value(defaultValue);
+
+  if (Breakpoints.find(sourcePath) != Breakpoints.end()) {
+    Breakpoints[sourcePath].clear();
+  }
+  response.breakpoints.resize(breakpoints.size());
+
   if (ListFileFunctionLines.find(sourcePath) != ListFileFunctionLines.end()) {
     // The file has loaded, we can validate breakpoints.
-    if (Breakpoints.find(sourcePath) != Breakpoints.end()) {
-      Breakpoints[sourcePath].clear();
-    }
-    response.breakpoints.resize(breakpoints.size());
     for (size_t i = 0; i < breakpoints.size(); i++) {
       int64_t correctedLine =
         CalibrateBreakpointLine(sourcePath, breakpoints[i].line);
@@ -106,7 +109,6 @@ cmDebuggerBreakpointManager::HandleSetBreakpointsRequest(
     // The file has not loaded, validate breakpoints later.
     ListFilePendingValidations.emplace(sourcePath);
 
-    response.breakpoints.resize(breakpoints.size());
     for (size_t i = 0; i < breakpoints.size(); i++) {
       Breakpoints[sourcePath].emplace_back(NextBreakpointId++,
                                            breakpoints[i].line);
@@ -191,6 +193,15 @@ std::vector<int64_t> cmDebuggerBreakpointManager::GetBreakpoints(
   return breakpoints;
 }
 
+size_t cmDebuggerBreakpointManager::GetBreakpointCount() const
+{
+  size_t count = 0;
+  for (auto const& pair : Breakpoints) {
+    count += pair.second.size();
+  }
+  return count;
+}
+
 void cmDebuggerBreakpointManager::ClearAll()
 {
   std::unique_lock<std::mutex> lock(Mutex);
diff --git a/Source/cmDebuggerBreakpointManager.h b/Source/cmDebuggerBreakpointManager.h
index a4e5df5..747722f 100644
--- a/Source/cmDebuggerBreakpointManager.h
+++ b/Source/cmDebuggerBreakpointManager.h
@@ -4,6 +4,7 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#include <cstddef>
 #include <cstdint>
 #include <mutex>
 #include <string>
@@ -55,6 +56,7 @@ public:
                         std::vector<cmListFileFunction> const& functions);
   std::vector<int64_t> GetBreakpoints(std::string const& sourcePath,
                                       int64_t line);
+  size_t GetBreakpointCount() const;
   void ClearAll();
 };
 
diff --git a/Tests/CMakeLib/testDebuggerBreakpointManager.cxx b/Tests/CMakeLib/testDebuggerBreakpointManager.cxx
index 83734ea..f654442 100644
--- a/Tests/CMakeLib/testDebuggerBreakpointManager.cxx
+++ b/Tests/CMakeLib/testDebuggerBreakpointManager.cxx
@@ -51,6 +51,13 @@ static bool testHandleBreakpointRequestBeforeFileIsLoaded()
                     sourcePath, false);
   ASSERT_BREAKPOINT(response.breakpoints[2], 2, sourceBreakpoints[2].line,
                     sourcePath, false);
+  ASSERT_TRUE(breakpointManager.GetBreakpointCount() == 3);
+
+  // setBreakpoints should override any existing breakpoints
+  setBreakpointRequest.breakpoints.value().clear();
+  helper.Client->send(setBreakpointRequest).get();
+  ASSERT_TRUE(breakpointManager.GetBreakpointCount() == 0);
+
   return true;
 }
 
@@ -103,6 +110,12 @@ static bool testHandleBreakpointRequestAfterFileIsLoaded()
                     sourcePath, true);
 
   ASSERT_TRUE(notExpectBreakpointEvents.load());
+  ASSERT_TRUE(breakpointManager.GetBreakpointCount() == 5);
+
+  // setBreakpoints should override any existing breakpoints
+  setBreakpointRequest.breakpoints.value().clear();
+  helper.Client->send(setBreakpointRequest).get();
+  ASSERT_TRUE(breakpointManager.GetBreakpointCount() == 0);
 
   return true;
 }
-- 
cgit v0.12