diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp index 096a43ad..a925544d 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp @@ -29,6 +29,13 @@ 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 AvrGdbRsp::resolveCommandPacket( diff --git a/src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp b/src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp index 3dee46c3..f7bbc8da 100644 --- a/src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp +++ b/src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp @@ -36,7 +36,13 @@ namespace Bloom::DebugServer::Gdb::CommandPackets class CommandPacket: public Packet { public: - explicit CommandPacket(const RawPacket& rawPacket): Packet(rawPacket) {} + explicit CommandPacket(const RawPacket& rawPacket) + : Packet(rawPacket) + {} + + virtual bool requiresBreakpointFlush() const { + return false; + } /** * Should handle the command for the current active debug session. diff --git a/src/DebugServer/Gdb/CommandPackets/ContinueExecution.hpp b/src/DebugServer/Gdb/CommandPackets/ContinueExecution.hpp index 0253392f..de29fbe0 100644 --- a/src/DebugServer/Gdb/CommandPackets/ContinueExecution.hpp +++ b/src/DebugServer/Gdb/CommandPackets/ContinueExecution.hpp @@ -28,6 +28,10 @@ namespace Bloom::DebugServer::Gdb::CommandPackets explicit ContinueExecution(const RawPacket& rawPacket); + bool requiresBreakpointFlush() const override { + return true; + } + void handle( DebugSession& debugSession, Services::TargetControllerService& targetControllerService diff --git a/src/DebugServer/Gdb/CommandPackets/Detach.hpp b/src/DebugServer/Gdb/CommandPackets/Detach.hpp index 8289e159..b80a709e 100644 --- a/src/DebugServer/Gdb/CommandPackets/Detach.hpp +++ b/src/DebugServer/Gdb/CommandPackets/Detach.hpp @@ -9,6 +9,10 @@ namespace Bloom::DebugServer::Gdb::CommandPackets public: explicit Detach(const RawPacket& rawPacket); + bool requiresBreakpointFlush() const override { + return true; + } + void handle( DebugSession& debugSession, Services::TargetControllerService& targetControllerService diff --git a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp index efc0c50d..350f4e17 100644 --- a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp +++ b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp @@ -51,9 +51,18 @@ namespace Bloom::DebugServer::Gdb::CommandPackets } void RemoveBreakpoint::handle(DebugSession& debugSession, TargetControllerService& targetControllerService) { - Logger::debug("Removing breakpoint at address " + std::to_string(this->address)); + 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)); debugSession.connection.writePacket(OkResponsePacket()); diff --git a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp b/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp index 59366563..240951da 100644 --- a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp +++ b/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp @@ -54,7 +54,18 @@ namespace Bloom::DebugServer::Gdb::CommandPackets Logger::debug("Handling SetBreakpoint packet"); try { - targetControllerService.setBreakpoint(TargetBreakpoint(this->address)); + 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); + } + debugSession.connection.writePacket(OkResponsePacket()); } catch (const Exception& exception) { diff --git a/src/DebugServer/Gdb/CommandPackets/StepExecution.hpp b/src/DebugServer/Gdb/CommandPackets/StepExecution.hpp index a433fe36..01e45dcc 100644 --- a/src/DebugServer/Gdb/CommandPackets/StepExecution.hpp +++ b/src/DebugServer/Gdb/CommandPackets/StepExecution.hpp @@ -23,6 +23,10 @@ namespace Bloom::DebugServer::Gdb::CommandPackets explicit StepExecution(const RawPacket& rawPacket); + bool requiresBreakpointFlush() const override { + return false; + } + void handle( DebugSession& debugSession, Services::TargetControllerService& targetControllerService diff --git a/src/DebugServer/Gdb/DebugSession.cpp b/src/DebugServer/Gdb/DebugSession.cpp index 0b550787..ec8e1c7b 100644 --- a/src/DebugServer/Gdb/DebugSession.cpp +++ b/src/DebugServer/Gdb/DebugSession.cpp @@ -7,11 +7,13 @@ namespace Bloom::DebugServer::Gdb DebugSession::DebugSession( Connection&& connection, const std::set>>& supportedFeatures, - const TargetDescriptor& targetDescriptor + const TargetDescriptor& targetDescriptor, + const GdbDebugServerConfig& serverConfig ) : connection(std::move(connection)) , supportedFeatures(supportedFeatures) , gdbTargetDescriptor(targetDescriptor) + , serverConfig(serverConfig) { this->supportedFeatures.insert({ Feature::PACKET_SIZE, std::to_string(Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE) diff --git a/src/DebugServer/Gdb/DebugSession.hpp b/src/DebugServer/Gdb/DebugSession.hpp index 3e9b332f..68316249 100644 --- a/src/DebugServer/Gdb/DebugSession.hpp +++ b/src/DebugServer/Gdb/DebugSession.hpp @@ -2,12 +2,16 @@ #include #include +#include #include "TargetDescriptor.hpp" +#include "GdbDebugServerConfig.hpp" #include "Connection.hpp" #include "Feature.hpp" #include "ProgrammingSession.hpp" +#include "src/Targets/TargetMemory.hpp" + namespace Bloom::DebugServer::Gdb { class DebugSession @@ -29,6 +33,18 @@ namespace Bloom::DebugServer::Gdb */ const TargetDescriptor& gdbTargetDescriptor; + /** + * The current server configuration. + */ + 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 breakpointAddresses; + std::unordered_set 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. @@ -53,7 +69,8 @@ namespace Bloom::DebugServer::Gdb DebugSession( Connection&& connection, const std::set>>& supportedFeatures, - const TargetDescriptor& targetDescriptor + const TargetDescriptor& targetDescriptor, + const GdbDebugServerConfig& serverConfig ); DebugSession(const DebugSession& other) = delete; diff --git a/src/DebugServer/Gdb/GdbDebugServerConfig.cpp b/src/DebugServer/Gdb/GdbDebugServerConfig.cpp index 14d839ee..5e50ca8d 100644 --- a/src/DebugServer/Gdb/GdbDebugServerConfig.cpp +++ b/src/DebugServer/Gdb/GdbDebugServerConfig.cpp @@ -30,5 +30,17 @@ namespace Bloom::DebugServer::Gdb ); } } + + if (debugServerConfig.debugServerNode["enableBreakpointCaching"]) { + if (YamlUtilities::isCastable(debugServerConfig.debugServerNode["enableBreakpointCaching"])) { + this->breakpointCachingEnabled = debugServerConfig.debugServerNode["enableBreakpointCaching"].as(); + + } else { + Logger::error( + "Invalid GDB debug server config parameter ('enableBreakpointCaching') provided - value must be " + "castable to a boolean. The parameter will be ignored." + ); + } + } } } diff --git a/src/DebugServer/Gdb/GdbDebugServerConfig.hpp b/src/DebugServer/Gdb/GdbDebugServerConfig.hpp index babde8ca..b1d9c750 100644 --- a/src/DebugServer/Gdb/GdbDebugServerConfig.hpp +++ b/src/DebugServer/Gdb/GdbDebugServerConfig.hpp @@ -24,6 +24,19 @@ 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); }; } diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.cpp b/src/DebugServer/Gdb/GdbRspDebugServer.cpp index 234c3493..28561a6b 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.cpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.cpp @@ -154,7 +154,8 @@ namespace Bloom::DebugServer::Gdb this->activeDebugSession.emplace( std::move(connection), this->getSupportedFeatures(), - this->getGdbTargetDescriptor() + this->getGdbTargetDescriptor(), + this->debugServerConfig ); /* @@ -183,6 +184,10 @@ 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); } @@ -367,4 +372,28 @@ 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(); + } } diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.hpp b/src/DebugServer/Gdb/GdbRspDebugServer.hpp index 983ad0b4..30709845 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.hpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.hpp @@ -197,5 +197,10 @@ 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(); }; }