diff --git a/CMakeLists.txt b/CMakeLists.txt index 621a0daf..cb43fb70 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -129,7 +129,6 @@ add_executable(Bloom src/DebugServers/GdbRsp/GdbDebugServerConfig.cpp src/DebugServers/GdbRsp/Connection.cpp src/DebugServers/GdbRsp/DebugSession.cpp - src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.cpp src/DebugServers/GdbRsp/CommandPackets/CommandPacket.cpp src/DebugServers/GdbRsp/CommandPackets/SupportedFeaturesQuery.cpp src/DebugServers/GdbRsp/CommandPackets/ReadRegisters.cpp diff --git a/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.cpp b/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.cpp deleted file mode 100644 index af200a2c..00000000 --- a/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.cpp +++ /dev/null @@ -1,141 +0,0 @@ -#include "CommandPacketFactory.hpp" - -#include -#include -#include - -namespace Bloom::DebugServers::Gdb -{ - using CommandPackets::CommandPacket; - - std::vector> CommandPacketFactory::extractRawPackets( - std::vector buffer - ) { - std::vector> output; - - std::size_t bufferIndex; - std::size_t bufferSize = buffer.size(); - unsigned char byte; - for (bufferIndex = 0; bufferIndex < bufferSize; bufferIndex++) { - byte = buffer[bufferIndex]; - - if (byte == 0x03) { - /* - * This is an interrupt packet - it doesn't carry any of the usual packet frame bytes, so we'll just - * add them here, in order to keep things consistent. - * - * Because we're effectively faking the packet frame, we can use any value for the checksum. - */ - output.push_back({'$', byte, '#', 'F', 'F'}); - - } else if (byte == '$') { - // Beginning of packet - std::vector rawPacket; - rawPacket.push_back('$'); - - auto packetIndex = bufferIndex; - bool validPacket = false; - bool isByteEscaped = false; - - for (packetIndex++; packetIndex < bufferSize; packetIndex++) { - byte = buffer[packetIndex]; - - if (byte == '}' && !isByteEscaped) { - isByteEscaped = true; - continue; - } - - if (byte == '$' && !isByteEscaped) { - // Unexpected end of packet - validPacket = false; - break; - } - - if (byte == '#' && !isByteEscaped) { - // End of packet data - if ((bufferSize - 1) < (packetIndex + 2)) { - // There should be at least two more bytes in the buffer, for the checksum. - break; - } - - rawPacket.push_back(byte); - - // Add the checksum bytes and break the loop - rawPacket.push_back(buffer[++packetIndex]); - rawPacket.push_back(buffer[++packetIndex]); - validPacket = true; - break; - } - - if (isByteEscaped) { - // Escaped bytes are XOR'd with a 0x20 mask. - byte ^= 0x20; - isByteEscaped = false; - } - - rawPacket.push_back(byte); - } - - if (validPacket) { - output.push_back(rawPacket); - bufferIndex = packetIndex; - } - } - } - - return output; - } - - std::unique_ptr CommandPacketFactory::create(std::vector rawPacket) { - if (rawPacket.size() == 5 && rawPacket[1] == 0x03) { - // This is an interrupt request - create a fake packet for it - return std::make_unique(rawPacket); - } - - auto rawPacketString = std::string(rawPacket.begin(), rawPacket.end()); - - if (rawPacketString.size() >= 2) { - /* - * First byte of the raw packet will be 0x24 ('$'), so find() should return 1, not 0, when - * looking for a command identifier string. - */ - if (rawPacketString.find("qSupported") == 1) { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 'g' || rawPacketString[1] == 'p') { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 'P') { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 'c') { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 's') { - return std::make_unique(rawPacket); - } - -// if (rawPacketString[1] == 'm') { -// return std::make_unique(rawPacket); -// } -// -// if (rawPacketString[1] == 'M') { -// return std::make_unique(rawPacket); -// } - - if (rawPacketString[1] == 'Z') { - return std::make_unique(rawPacket); - } - - if (rawPacketString[1] == 'z') { - return std::make_unique(rawPacket); - } - } - - return std::make_unique(rawPacket); - } -} diff --git a/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.hpp b/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.hpp deleted file mode 100644 index 096e50ec..00000000 --- a/src/DebugServers/GdbRsp/CommandPackets/CommandPacketFactory.hpp +++ /dev/null @@ -1,47 +0,0 @@ -#pragma once - -#include -#include - -// Command packets -#include "CommandPacket.hpp" -#include "InterruptExecution.hpp" -#include "SupportedFeaturesQuery.hpp" -#include "ReadRegisters.hpp" -#include "WriteRegister.hpp" -#include "StepExecution.hpp" -#include "ContinueExecution.hpp" -#include "SetBreakpoint.hpp" -#include "RemoveBreakpoint.hpp" - -namespace Bloom::DebugServers::Gdb -{ - /** - * The CommandPacketFactory class provides a means for extracting raw packet data from a raw buffer, and - * constructing the appropriate CommandPacket objects. - */ - class CommandPacketFactory - { - public: - /** - * Extracts raw GDB RSP packets from buffer. - * - * @param buffer - * The buffer from which to extract the raw GDB RSP packets. - * - * @return - * A vector of raw packets. - */ - static std::vector> extractRawPackets(std::vector buffer); - - /** - * Constructs the appropriate CommandPacket object from a single raw GDB RSP packet. - * - * @param rawPacket - * The raw GDB RSP packet from which to construct the CommandPacket object. - * - * @return - */ - static std::unique_ptr create(std::vector rawPacket); - }; -} diff --git a/src/DebugServers/GdbRsp/CommandPackets/InterruptExecution.hpp b/src/DebugServers/GdbRsp/CommandPackets/InterruptExecution.hpp index acdea58b..27f121d1 100644 --- a/src/DebugServers/GdbRsp/CommandPackets/InterruptExecution.hpp +++ b/src/DebugServers/GdbRsp/CommandPackets/InterruptExecution.hpp @@ -10,7 +10,7 @@ namespace Bloom::DebugServers::Gdb::CommandPackets * * Technically, interrupts are not sent by the client in the form of a typical GDP RSP packet. Instead, they're * just sent as a single byte from the client. We fake the packet on our end, to save us the headache of dealing - * with this inconsistency. We do this in CommandPacketFactory::extractRawPackets(). + * with this inconsistency. We do this in Connection::readRawPackets(). */ class InterruptExecution: public CommandPacket { diff --git a/src/DebugServers/GdbRsp/Connection.cpp b/src/DebugServers/GdbRsp/Connection.cpp index bfac3bb7..cc7d1abf 100644 --- a/src/DebugServers/GdbRsp/Connection.cpp +++ b/src/DebugServers/GdbRsp/Connection.cpp @@ -5,8 +5,6 @@ #include #include -#include "CommandPackets/CommandPacketFactory.hpp" - #include "Exceptions/ClientDisconnected.hpp" #include "Exceptions/ClientCommunicationError.hpp" #include "src/Exceptions/Exception.hpp" @@ -16,11 +14,11 @@ namespace Bloom::DebugServers::Gdb { - using namespace CommandPackets; - using namespace ResponsePackets; using namespace Exceptions; using namespace Bloom::Exceptions; + using ResponsePackets::ResponsePacket; + void Connection::accept(int serverSocketFileDescriptor) { int socketAddressLength = sizeof(this->socketAddress); @@ -63,25 +61,80 @@ namespace Bloom::DebugServers::Gdb } } - std::vector> Connection::readPackets() { - auto buffer = this->read(); - Logger::debug("GDB client data received (" + std::to_string(buffer.size()) + " bytes): " - + std::string(buffer.begin(), buffer.end())); + std::vector Connection::readRawPackets() { + std::vector output; - auto rawPackets = CommandPacketFactory::extractRawPackets(buffer); - std::vector> output; + const auto bytes = this->read(); - for (const auto& rawPacket : rawPackets) { - try { - output.push_back(CommandPacketFactory::create(rawPacket)); - this->write({'+'}); + std::size_t bufferSize = bytes.size(); + for (std::size_t byteIndex = 0; byteIndex < bufferSize; byteIndex++) { + auto byte = bytes[byteIndex]; - } catch (const ClientDisconnected& exception) { - throw exception; + if (byte == 0x03) { + /* + * This is an interrupt packet - it doesn't carry any of the usual packet frame bytes, so we'll just + * add them here, in order to keep things consistent. + * + * Because we're effectively faking the packet frame, we can use any value for the checksum. + */ + output.push_back({'$', byte, '#', 'F', 'F'}); - } catch (const Exception& exception) { - Logger::error("Failed to parse GDB packet - " + exception.getMessage()); - this->write({'-'}); + } else if (byte == '$') { + // Beginning of packet + RawPacketType rawPacket; + rawPacket.push_back('$'); + + auto packetIndex = byteIndex; + bool validPacket = false; + bool isByteEscaped = false; + + for (packetIndex++; packetIndex < bufferSize; packetIndex++) { + byte = bytes[packetIndex]; + + if (byte == '}' && !isByteEscaped) { + isByteEscaped = true; + continue; + } + + if (!isByteEscaped) { + if (byte == '$') { + // Unexpected end of packet + validPacket = false; + break; + } + + if (byte == '#') { + // End of packet data + if ((bufferSize - 1) < (packetIndex + 2)) { + // There should be at least two more bytes in the buffer, for the checksum. + break; + } + + rawPacket.push_back(byte); + + // Add the checksum bytes and break the loop + rawPacket.push_back(bytes[++packetIndex]); + rawPacket.push_back(bytes[++packetIndex]); + validPacket = true; + break; + } + + } else { + // Escaped bytes are XOR'd with a 0x20 mask. + byte ^= 0x20; + isByteEscaped = false; + } + + rawPacket.push_back(byte); + } + + if (validPacket) { + // Acknowledge receipt + this->write({'+'}); + + output.push_back(rawPacket); + byteIndex = packetIndex; + } } } diff --git a/src/DebugServers/GdbRsp/Connection.hpp b/src/DebugServers/GdbRsp/Connection.hpp index 90595605..48d81e53 100644 --- a/src/DebugServers/GdbRsp/Connection.hpp +++ b/src/DebugServers/GdbRsp/Connection.hpp @@ -10,15 +10,14 @@ #include #include "src/Helpers/EventNotifier.hpp" -#include "src/DebugServers/GdbRsp/CommandPackets/CommandPacket.hpp" + +#include "src/DebugServers/GdbRsp/Packet.hpp" #include "src/DebugServers/GdbRsp/ResponsePackets/ResponsePacket.hpp" namespace Bloom::DebugServers::Gdb { /** * The Connection class represents an active connection between the GDB RSP server and client. - * - * All interfacing with the GDB client should take place here. */ class Connection { @@ -57,11 +56,11 @@ namespace Bloom::DebugServers::Gdb }; /** - * Waits for incoming data from the client and returns any received command packets. + * Waits for incoming data from the client and returns the raw GDB packets. * * @return */ - std::vector> readPackets(); + std::vector readRawPackets(); /** * Sends a response packet to the client. diff --git a/src/DebugServers/GdbRsp/GdbRspDebugServer.cpp b/src/DebugServers/GdbRsp/GdbRspDebugServer.cpp index ef10fe66..8c97420a 100644 --- a/src/DebugServers/GdbRsp/GdbRspDebugServer.cpp +++ b/src/DebugServers/GdbRsp/GdbRspDebugServer.cpp @@ -14,6 +14,18 @@ #include "src/Exceptions/InvalidConfig.hpp" #include "src/Exceptions/DebugServerInterrupted.hpp" +// Command packets +#include "CommandPackets/CommandPacket.hpp" +#include "CommandPackets/SupportedFeaturesQuery.hpp" +#include "CommandPackets/InterruptExecution.hpp" +#include "CommandPackets/ContinueExecution.hpp" +#include "CommandPackets/StepExecution.hpp" +#include "CommandPackets/ReadRegisters.hpp" +#include "CommandPackets/WriteRegister.hpp" +#include "CommandPackets/SetBreakpoint.hpp" +#include "CommandPackets/RemoveBreakpoint.hpp" + +// Response packets #include "ResponsePackets/TargetStopped.hpp" namespace Bloom::DebugServers::Gdb @@ -21,6 +33,8 @@ namespace Bloom::DebugServers::Gdb using namespace Exceptions; using namespace Bloom::Exceptions; + using CommandPackets::CommandPacket; + GdbRspDebugServer::GdbRspDebugServer( const DebugServerConfig& debugServerConfig, EventListener& eventListener @@ -146,13 +160,14 @@ namespace Bloom::DebugServers::Gdb } } -// auto packets = this->activeDebugSession->connection.readPackets(); -// -// // Only process the last packet - any others will likely be duplicates from an impatient client -// if (!packets.empty()) { -// // Double-dispatch to appropriate handler -// packets.back()->dispatchToHandler(*this); -// } + auto commandPacket = this->waitForCommandPacket(); + + if (commandPacket == nullptr) { + // Likely an interrupt + return; + } + + commandPacket->handle(this->activeDebugSession.value(), this->targetControllerConsole); } catch (const ClientDisconnected&) { Logger::info("GDB RSP client disconnected"); @@ -214,6 +229,63 @@ namespace Bloom::DebugServers::Gdb return std::nullopt; } + std::unique_ptr GdbRspDebugServer::waitForCommandPacket() { + const auto rawPackets = this->activeDebugSession->connection.readRawPackets(); + + if (rawPackets.empty()) { + // The wait was interrupted + return nullptr; + } + + // We only process the last packet - any others will probably be duplicates from an impatient client. + return this->resolveCommandPacket(rawPackets.back()); + } + + std::unique_ptr GdbRspDebugServer::resolveCommandPacket(const RawPacketType& rawPacket) { + if (rawPacket.size() == 5 && rawPacket[1] == 0x03) { + // This is an interrupt request - create a fake packet for it + return std::make_unique(rawPacket); + } + + const auto rawPacketString = std::string(rawPacket.begin(), rawPacket.end()); + + if (rawPacketString.size() >= 2) { + /* + * First byte of the raw packet will be 0x24 ('$'), so find() should return 1, not 0, when + * looking for a command identifier string. + */ + if (rawPacketString.find("qSupported") == 1) { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 'g' || rawPacketString[1] == 'p') { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 'P') { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 'c') { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 's') { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 'Z') { + return std::make_unique(rawPacket); + } + + if (rawPacketString[1] == 'z') { + return std::make_unique(rawPacket); + } + } + + return std::make_unique(rawPacket); + } + void GdbRspDebugServer::terminateActiveDebugSession() { if (this->activeDebugSession.has_value()) { this->activeDebugSession->terminate(); diff --git a/src/DebugServers/GdbRsp/GdbRspDebugServer.hpp b/src/DebugServers/GdbRsp/GdbRspDebugServer.hpp index 256507e0..5e0407ec 100644 --- a/src/DebugServers/GdbRsp/GdbRspDebugServer.hpp +++ b/src/DebugServers/GdbRsp/GdbRspDebugServer.hpp @@ -19,7 +19,7 @@ #include "Signal.hpp" #include "RegisterDescriptor.hpp" #include "Feature.hpp" -#include "CommandPackets/CommandPacketFactory.hpp" +#include "CommandPackets/CommandPacket.hpp" #include "src/EventManager/Events/TargetControllerStateReported.hpp" #include "src/EventManager/Events/TargetExecutionStopped.hpp" @@ -103,6 +103,10 @@ namespace Bloom::DebugServers::Gdb */ int enableReuseAddressSocketOption = 1; + /** + * When a connection with a GDB client is established, a new instance of the DebugSession class is created and + * held here. A value of std::nullopt means there is no active debug session present. + */ std::optional activeDebugSession; /** @@ -112,10 +116,51 @@ namespace Bloom::DebugServers::Gdb */ std::optional waitForConnection(); + /** + * Waits for a command packet from the connected GDB client. + * + * @return + */ + std::unique_ptr waitForCommandPacket(); + + /** + * Should construct a derived instance of the CommandPackets::CommandPacket class, from a raw packet. + * + * Derived implementations of this GDB server class can override this function to include support for more + * specific GDB commands. For example, the AVR GDB implementation overrides this function to support AVR + * specific implementations of the read and write memory GDB commands. + * + * This function should never return a nullptr. If the command is unknown, it should simply return an + * instance of the CommandPackets::CommandPacket base class. The handler (CommandPacket::handle()) for the base + * class will handle unknown packets by responding with an empty response packet (as is expected by the GDB + * client). + * + * @param rawPacket + * @return + */ + virtual std::unique_ptr resolveCommandPacket(const RawPacketType& rawPacket); + + /** + * Terminates any active debug session (if any) by closing the connection to the GDB client. + */ void terminateActiveDebugSession(); + /** + * Should return the GDB target descriptor for the connected target. + * + * NOTE: This function returns a target descriptor specific to GDB and the target. It's not the same data + * struct as Targets::TargetDescriptor. But the GDB target descriptor does hold an instance to + * Targets::TargetDescriptor. See the Gdb::TargetDescriptor::targetDescriptor class member. + * + * @return + */ virtual const TargetDescriptor& getGdbTargetDescriptor() = 0; + /** + * Responds to any unexpected TargetController state changes. + * + * @param event + */ void onTargetControllerStateReported(const Events::TargetControllerStateReported& event); /** diff --git a/src/DebugServers/GdbRsp/Packet.hpp b/src/DebugServers/GdbRsp/Packet.hpp index 8be18cd4..2619f6fe 100644 --- a/src/DebugServers/GdbRsp/Packet.hpp +++ b/src/DebugServers/GdbRsp/Packet.hpp @@ -9,6 +9,8 @@ namespace Bloom::DebugServers::Gdb { + using RawPacketType = std::vector; + /** * The Packet class implements the data structure for GDB RSP packets. * @@ -17,7 +19,7 @@ namespace Bloom::DebugServers::Gdb class Packet { public: - explicit Packet(const std::vector& rawPacket) { + explicit Packet(const RawPacketType& rawPacket) { this->init(rawPacket); } @@ -119,7 +121,7 @@ namespace Bloom::DebugServers::Gdb protected: std::vector data; - void init(const std::vector& rawPacket) { + void init(const RawPacketType& rawPacket) { this->data.insert( this->data.begin(), rawPacket.begin() + 1,