From 17c301b57a4c51e3f57dfd7d3b5db285861bf35d Mon Sep 17 00:00:00 2001 From: Nav Date: Sun, 3 Apr 2022 20:35:53 +0100 Subject: [PATCH] Tidied AVR GDB memory access command packet classes --- CMakeLists.txt | 1 - .../AbstractMemoryAccessPacket.cpp | 17 -------------- ...cket.hpp => MemoryAccessCommandPacket.hpp} | 22 +++++++++++++------ .../Gdb/AvrGdb/CommandPackets/ReadMemory.cpp | 15 ++++++++----- .../Gdb/AvrGdb/CommandPackets/ReadMemory.hpp | 12 ++++++---- .../Gdb/AvrGdb/CommandPackets/WriteMemory.cpp | 18 +++++++++------ .../Gdb/AvrGdb/CommandPackets/WriteMemory.hpp | 15 +++++++++---- 7 files changed, 55 insertions(+), 45 deletions(-) delete mode 100644 src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp rename src/DebugServer/Gdb/AvrGdb/CommandPackets/{AbstractMemoryAccessPacket.hpp => MemoryAccessCommandPacket.hpp} (53%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6999b0ce..965b46e6 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -144,7 +144,6 @@ add_executable(Bloom # AVR GDB Server src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp src/DebugServer/Gdb/AvrGdb/TargetDescriptor.cpp - src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp deleted file mode 100644 index e0dc7597..00000000 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.cpp +++ /dev/null @@ -1,17 +0,0 @@ -#include "AbstractMemoryAccessPacket.hpp" - -#include "src/DebugServer/Gdb/ResponsePackets/TargetStopped.hpp" -#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" -#include "src/DebugServer/Gdb/Signal.hpp" - -#include "src/Logger/Logger.hpp" -#include "src/Exceptions/Exception.hpp" - -namespace Bloom::DebugServer::Gdb::CommandPackets -{ - using ResponsePackets::ErrorResponsePacket; - - using Exceptions::Exception; - - -} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/MemoryAccessCommandPacket.hpp similarity index 53% rename from src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.hpp rename to src/DebugServer/Gdb/AvrGdb/CommandPackets/MemoryAccessCommandPacket.hpp index ca0b8bf3..91c9c6b1 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/AbstractMemoryAccessPacket.hpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/MemoryAccessCommandPacket.hpp @@ -10,13 +10,21 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets { /** - * The ReadMemory class implements a structure for "m" packets. Upon receiving these packets, the server is - * expected to read memory from the target and send it the client. + * The MemoryAccessCommandPacket class is a base class for memory access GDB commands that are specific to the AVR + * architecture. + * + * With the GDB implementation for the AVR architecture, read/write memory commands include a special memory + * address. The memory type (FLASH, RAM, EEPROM, etc) is embedded within the 7 most significant bits of the memory + * address. + * + * This class provides functions to extract and remove the memory type from a given memory address. */ - class AbstractMemoryAccessPacket: public Bloom::DebugServer::Gdb::CommandPackets::CommandPacket + class MemoryAccessCommandPacket: public Bloom::DebugServer::Gdb::CommandPackets::CommandPacket { public: - explicit AbstractMemoryAccessPacket(const std::vector& rawPacket): CommandPacket(rawPacket) {}; + explicit MemoryAccessCommandPacket(const std::vector& rawPacket) + : CommandPacket(rawPacket) + {}; protected: /** @@ -32,7 +40,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * @return */ Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) { - if ((address & AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U) { + if ((address & MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U) { return Targets::TargetMemoryType::RAM; } @@ -46,8 +54,8 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * @return */ std::uint32_t removeMemoryTypeIndicatorFromGdbAddress(std::uint32_t address) { - return (address & AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U - ? (address & ~(AbstractMemoryAccessPacket::AVR_GDB_MEMORY_ADDRESS_MASK)) + return (address & MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK) != 0U + ? (address & ~(MemoryAccessCommandPacket::AVR_GDB_MEMORY_ADDRESS_MASK)) : address; } }; diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp index 062e8dfd..e3e3512d 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp @@ -14,7 +14,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets using Exceptions::Exception; ReadMemory::ReadMemory(const std::vector& rawPacket) - : AbstractMemoryAccessPacket(rawPacket) + : MemoryAccessCommandPacket(rawPacket) { if (this->data.size() < 4) { throw Exception("Invalid packet length"); @@ -38,12 +38,15 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets } bool conversionStatus = false; - this->startAddress = packetSegments.at(0).toUInt(&conversionStatus, 16); + const auto gdbStartAddress = packetSegments.at(0).toUInt(&conversionStatus, 16); if (!conversionStatus) { throw Exception("Failed to parse start address from read memory packet data"); } + this->memoryType = this->getMemoryTypeFromGdbAddress(gdbStartAddress); + this->startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(gdbStartAddress); + this->bytes = packetSegments.at(1).toUInt(&conversionStatus, 16); if (!conversionStatus) { @@ -55,9 +58,11 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets Logger::debug("Handling ReadMemory packet"); try { - auto memoryType = this->getMemoryTypeFromGdbAddress(this->startAddress); - auto startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(this->startAddress); - auto memoryBuffer = targetControllerConsole.readMemory(memoryType, startAddress, this->bytes); + auto memoryBuffer = targetControllerConsole.readMemory( + this->memoryType, + this->startAddress, + this->bytes + ); auto hexMemoryBuffer = Packet::dataToHex(memoryBuffer); debugSession.connection.writePacket( diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.hpp index 3968c7a8..8809f502 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.hpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.hpp @@ -3,7 +3,7 @@ #include #include -#include "AbstractMemoryAccessPacket.hpp" +#include "MemoryAccessCommandPacket.hpp" namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets { @@ -11,15 +11,19 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * The ReadMemory class implements a structure for "m" packets. Upon receiving these packets, the server is * expected to read memory from the target and send it the client. */ - class ReadMemory: public AbstractMemoryAccessPacket + class ReadMemory: public MemoryAccessCommandPacket { public: /** - * The startAddress sent from the GDB client will include additional bits used to indicate the memory type. - * These bits have to be removed from the address before it can be used as a start address. + * Start address of the memory operation. */ std::uint32_t startAddress = 0; + /** + * The type of memory to read from. + */ + Targets::TargetMemoryType memoryType = Targets::TargetMemoryType::FLASH; + /** * Number of bytes to read. */ diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp index b8224a2b..142f72b2 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp @@ -14,7 +14,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets using namespace Bloom::Exceptions; WriteMemory::WriteMemory(const std::vector& rawPacket) - : AbstractMemoryAccessPacket(rawPacket) + : MemoryAccessCommandPacket(rawPacket) { if (this->data.size() < 4) { throw Exception("Invalid packet length"); @@ -37,12 +37,15 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets } bool conversionStatus = false; - this->startAddress = packetSegments.at(0).toUInt(&conversionStatus, 16); + const auto gdbStartAddress = packetSegments.at(0).toUInt(&conversionStatus, 16); if (!conversionStatus) { throw Exception("Failed to parse start address from write memory packet data"); } + this->memoryType = this->getMemoryTypeFromGdbAddress(gdbStartAddress); + this->startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(gdbStartAddress); + auto lengthAndBufferSegments = packetSegments.at(1).split(":"); if (lengthAndBufferSegments.size() != 2) { throw Exception( @@ -67,16 +70,17 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets Logger::debug("Handling WriteMemory packet"); try { - const auto memoryType = this->getMemoryTypeFromGdbAddress(this->startAddress); - - if (memoryType == Targets::TargetMemoryType::FLASH) { + if (this->memoryType == Targets::TargetMemoryType::FLASH) { throw Exception( "GDB client requested a flash memory write - This is not currently supported by Bloom." ); } - const auto startAddress = this->removeMemoryTypeIndicatorFromGdbAddress(this->startAddress); - targetControllerConsole.writeMemory(memoryType, startAddress, this->buffer); + targetControllerConsole.writeMemory( + this->memoryType, + this->startAddress, + this->buffer + ); debugSession.connection.writePacket(OkResponsePacket()); diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.hpp index 9ffa4b2a..02263c0a 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.hpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.hpp @@ -3,7 +3,7 @@ #include #include -#include "AbstractMemoryAccessPacket.hpp" +#include "MemoryAccessCommandPacket.hpp" namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets { @@ -11,15 +11,22 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * The WriteMemory class implements the structure for "M" packets. Upon receiving this packet, the server is * expected to write data to the target's memory, at the specified start address. */ - class WriteMemory: public AbstractMemoryAccessPacket + class WriteMemory: public MemoryAccessCommandPacket { public: /** - * Like with the ReadMemory command packet, the start address carries additional bits that indicate - * the memory type. + * Start address of the memory operation. */ std::uint32_t startAddress = 0; + /** + * The type of memory to read from. + */ + Targets::TargetMemoryType memoryType = Targets::TargetMemoryType::FLASH; + + /** + * Data to write. + */ Targets::TargetMemoryBuffer buffer; explicit WriteMemory(const std::vector& rawPacket);