Removed breakpoint caching in GDB server - it's not needed!

GDB can be configured to avoid needlessly updating breakpoints on the target

src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp

@@ -29,13 +29,6 @@ namespace Bloom::DebugServer::Gdb::AvrGdb
         this->gdbTargetDescriptor = TargetDescriptor(
             this->targetControllerService.getTargetDescriptor()
         );
-
-        if (!this->debugServerConfig.breakpointCachingEnabled) {
-            Logger::warning(
-                "Breakpoint caching has been disabled - this could result in excessive wear of the AVR target's"
-                " flash memory"
-            );
-        }
     }
 
     std::unique_ptr<Gdb::CommandPackets::CommandPacket> AvrGdbRsp::resolveCommandPacket(

src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp

@@ -40,10 +40,6 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
             : Packet(rawPacket)
         {}
 
-        virtual bool requiresBreakpointFlush() const {
-            return false;
-        }
-
         /**
          * Should handle the command for the current active debug session.
          *

src/DebugServer/Gdb/CommandPackets/ContinueExecution.hpp

@@ -28,10 +28,6 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
 
         explicit ContinueExecution(const RawPacket& rawPacket);
 
-        bool requiresBreakpointFlush() const override {
-            return true;
-        }
-
         void handle(
             DebugSession& debugSession,
             Services::TargetControllerService& targetControllerService

src/DebugServer/Gdb/CommandPackets/Detach.hpp

@@ -9,10 +9,6 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
     public:
         explicit Detach(const RawPacket& rawPacket);
 
-        bool requiresBreakpointFlush() const override {
-            return true;
-        }
-
         void handle(
             DebugSession& debugSession,
             Services::TargetControllerService& targetControllerService

src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp

@@ -54,13 +54,6 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
         Logger::debug("Handling RemoveBreakpoint packet");
 
         try {
-            if (debugSession.serverConfig.breakpointCachingEnabled) {
-                debugSession.breakpointAddressesPendingRemoval.insert(this->address);
-                debugSession.connection.writePacket(OkResponsePacket());
-
-                return;
-            }
-
             Logger::debug("Removing breakpoint at address " + std::to_string(this->address));
 
             targetControllerService.removeBreakpoint(TargetBreakpoint(this->address));

src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp

@@ -54,18 +54,8 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
         Logger::debug("Handling SetBreakpoint packet");
 
         try {
-            if (
-                !debugSession.serverConfig.breakpointCachingEnabled
-                || !debugSession.breakpointAddresses.contains(this->address)
-            ) {
-                targetControllerService.setBreakpoint(TargetBreakpoint(this->address));
-            }
-
-            if (debugSession.serverConfig.breakpointCachingEnabled) {
-                debugSession.breakpointAddresses.insert(this->address);
-                debugSession.breakpointAddressesPendingRemoval.erase(this->address);
-            }
 
+            targetControllerService.setBreakpoint(TargetBreakpoint(this->address));
             debugSession.connection.writePacket(OkResponsePacket());
 
         } catch (const Exception& exception) {

src/DebugServer/Gdb/CommandPackets/StepExecution.hpp

@@ -23,10 +23,6 @@ namespace Bloom::DebugServer::Gdb::CommandPackets
 
         explicit StepExecution(const RawPacket& rawPacket);
 
-        bool requiresBreakpointFlush() const override {
-            return false;
-        }
-
         void handle(
             DebugSession& debugSession,
             Services::TargetControllerService& targetControllerService

src/DebugServer/Gdb/DebugSession.hpp

@@ -2,7 +2,6 @@
 
 #include <cstdint>
 #include <optional>
-#include <unordered_set>
 
 #include "TargetDescriptor.hpp"
 #include "GdbDebugServerConfig.hpp"
@@ -38,13 +37,6 @@ namespace Bloom::DebugServer::Gdb
          */
         const GdbDebugServerConfig& serverConfig;
 
-        /**
-         * Internal bookkeeping of breakpoints managed by GDB. These will both remain empty if the user has disabled
-         * breakpoint caching.
-         */
-        std::unordered_set<Targets::TargetMemoryAddress> breakpointAddresses;
-        std::unordered_set<Targets::TargetMemoryAddress> breakpointAddressesPendingRemoval;
-
         /**
          * When the GDB client is waiting for the target to halt, this is set to true so we know when to notify the
          * client.

src/DebugServer/Gdb/GdbDebugServerConfig.cpp

@@ -30,17 +30,5 @@ namespace Bloom::DebugServer::Gdb
                 );
             }
         }
-
-        if (debugServerConfig.debugServerNode["enableBreakpointCaching"]) {
-            if (YamlUtilities::isCastable<bool>(debugServerConfig.debugServerNode["enableBreakpointCaching"])) {
-                this->breakpointCachingEnabled = debugServerConfig.debugServerNode["enableBreakpointCaching"].as<bool>();
-
-            } else {
-                Logger::error(
-                    "Invalid GDB debug server config parameter ('enableBreakpointCaching') provided - value must be "
-                    "castable to a boolean. The parameter will be ignored."
-                );
-            }
-        }
     }
 }

src/DebugServer/Gdb/GdbDebugServerConfig.hpp

@@ -24,19 +24,6 @@ namespace Bloom::DebugServer::Gdb
          */
         std::string listeningAddress = "127.0.0.1";
 
-        /**
-         * GDB tends to remove all breakpoints when target execution stops, and then installs them again, just before
-         * resuming target execution. This can result in excessive wearing of the target's program memory, as well as
-         * a negative impact on performance.
-         *
-         * When breakpoint caching is enabled, Bloom's GDB server will perform internal bookkeeping of the breakpoints
-         * installed and removed via GDB. Then, just before resuming target execution, it will only apply the necessary
-         * changes to the target, avoiding the excessive wear and IO.
-         *
-         * This param is optional, and is enabled by default.
-         */
-        bool breakpointCachingEnabled = true;
-
         explicit GdbDebugServerConfig(const DebugServerConfig& debugServerConfig);
     };
 }

src/DebugServer/Gdb/GdbRspDebugServer.cpp

@@ -184,10 +184,6 @@ namespace Bloom::DebugServer::Gdb
             const auto commandPacket = this->waitForCommandPacket();
 
             if (commandPacket) {
-                if (this->debugServerConfig.breakpointCachingEnabled && commandPacket->requiresBreakpointFlush()) {
-                    this->flushBreakpointRemovals();
-                }
-
                 commandPacket->handle(this->activeDebugSession.value(), this->targetControllerService);
             }
 
@@ -372,28 +368,4 @@ namespace Bloom::DebugServer::Gdb
             return;
         }
     }
-
-    void GdbRspDebugServer::flushBreakpointRemovals() {
-        Logger::debug(
-            "Removing " + std::to_string(this->activeDebugSession->breakpointAddressesPendingRemoval.size())
-            + " breakpoint(s)"
-        );
-
-        for (const auto& breakpointAddress : this->activeDebugSession->breakpointAddressesPendingRemoval) {
-            try {
-                Logger::debug("Removing breakpoint at address " + std::to_string(breakpointAddress));
-
-                this->targetControllerService.removeBreakpoint(Targets::TargetBreakpoint(breakpointAddress));
-                this->activeDebugSession->breakpointAddresses.erase(breakpointAddress);
-
-            } catch (const Exception& exception) {
-                Logger::error(
-                    "Failed to remove breakpoint at address " + std::to_string(breakpointAddress) + " from target - "
-                    + exception.getMessage()
-                );
-            }
-        }
-
-        this->activeDebugSession->breakpointAddressesPendingRemoval.clear();
-    }
 }

src/DebugServer/Gdb/GdbRspDebugServer.hpp

@@ -197,10 +197,5 @@ namespace Bloom::DebugServer::Gdb
          * a "stop reply" packet to the client once the target execution stops.
          */
         void onTargetExecutionStopped(const Events::TargetExecutionStopped&);
-
-        /**
-         * Actions any pending breakpoint removals.
-         */
-        void flushBreakpointRemovals();
     };
 }