From fbe22b72edcfc79582fa6330393c2f17cef707f2 Mon Sep 17 00:00:00 2001 From: Nav Date: Tue, 25 Oct 2022 19:13:02 +0100 Subject: [PATCH] Tidying --- .../Gdb/AvrGdb/CommandPackets/ReadMemory.cpp | 20 +++++++++++++------ .../Gdb/AvrGdb/CommandPackets/WriteMemory.cpp | 8 ++++---- src/DebugServer/Gdb/TargetDescriptor.hpp | 18 +++++++++++++++++ 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp index 754d6295..d6a62aed 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp @@ -22,7 +22,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets throw Exception("Invalid packet length"); } - auto packetString = QString::fromLocal8Bit( + const auto packetString = QString::fromLocal8Bit( reinterpret_cast(this->data.data() + 1), static_cast(this->data.size() - 1) ); @@ -31,7 +31,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * The read memory ('m') packet consists of two segments, an address and a number of bytes to read. * These are separated by a comma character. */ - auto packetSegments = packetString.split(","); + const auto packetSegments = packetString.split(","); if (packetSegments.size() != 2) { throw Exception( @@ -46,6 +46,10 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets throw Exception("Failed to parse start address from read memory packet data"); } + /* + * Extract the memory type from the memory address (see Gdb::TargetDescriptor::memoryOffsetsByType for more on + * this). + */ this->memoryType = gdbTargetDescriptor.getMemoryTypeFromGdbAddress(gdbStartAddress); this->startAddress = gdbStartAddress & ~(gdbTargetDescriptor.getMemoryOffset(this->memoryType)); @@ -67,9 +71,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets } if (this->bytes == 0) { - debugSession.connection.writePacket( - ResponsePacket(std::vector()) - ); + debugSession.connection.writePacket(ResponsePacket(std::vector())); return; } @@ -112,6 +114,12 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets return; } + /* + * GDB may request more bytes than what's available (even though we give it a memory map?!) - ensure that + * we don't try to read any more than what's available. + * + * We fill the out-of-bounds bytes with 0x00, below. + */ const auto bytesToRead = (this->startAddress <= memoryDescriptor.addressRange.endAddress) ? std::min(this->bytes, (memoryDescriptor.addressRange.endAddress - this->startAddress) + 1) : 0; @@ -127,7 +135,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets } if (bytesToRead < this->bytes) { - // GDB requested some out-of-bounds memory - fill the inaccessible bytes with 0s + // GDB requested some out-of-bounds memory - fill the inaccessible bytes with 0x00 memoryBuffer.insert(memoryBuffer.end(), (this->bytes - bytesToRead), 0x00); } diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp index 2c2e7e09..d98e9ac5 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp @@ -22,7 +22,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets throw Exception("Invalid packet length"); } - auto packetString = QString::fromLocal8Bit( + const auto packetString = QString::fromLocal8Bit( reinterpret_cast(this->data.data() + 1), static_cast(this->data.size() - 1) ); @@ -31,7 +31,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets * The write memory ('M') packet consists of three segments, an address, a length and a buffer. * The address and length are separated by a comma character, and the buffer proceeds a colon character. */ - auto packetSegments = packetString.split(","); + const auto packetSegments = packetString.split(","); if (packetSegments.size() != 2) { throw Exception( "Unexpected number of segments in packet data: " + std::to_string(packetSegments.size()) @@ -48,7 +48,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets this->memoryType = gdbTargetDescriptor.getMemoryTypeFromGdbAddress(gdbStartAddress); this->startAddress = gdbStartAddress & ~(gdbTargetDescriptor.getMemoryOffset(this->memoryType)); - auto lengthAndBufferSegments = packetSegments.at(1).split(":"); + const auto lengthAndBufferSegments = packetSegments.at(1).split(":"); if (lengthAndBufferSegments.size() != 2) { throw Exception( "Unexpected number of segments in packet data: " @@ -56,7 +56,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets ); } - auto bufferSize = lengthAndBufferSegments.at(0).toUInt(&conversionStatus, 16); + const auto bufferSize = lengthAndBufferSegments.at(0).toUInt(&conversionStatus, 16); if (!conversionStatus) { throw Exception("Failed to parse write length from write memory packet data"); } diff --git a/src/DebugServer/Gdb/TargetDescriptor.hpp b/src/DebugServer/Gdb/TargetDescriptor.hpp index 0eff043d..9dbc3b03 100644 --- a/src/DebugServer/Gdb/TargetDescriptor.hpp +++ b/src/DebugServer/Gdb/TargetDescriptor.hpp @@ -37,6 +37,12 @@ namespace Bloom::DebugServer::Gdb return this->memoryOffsetsByType.valueAt(memoryType).value_or(0); } + /** + * Helper method to extract the target memory type (Flash, RAM, etc) from a GDB memory address. + * + * @param address + * @return + */ Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) const { // Start with the largest offset until we find a match for ( @@ -89,7 +95,19 @@ namespace Bloom::DebugServer::Gdb virtual const std::vector& getRegisterNumbers() const = 0; private: + /** + * When GDB sends us a memory address, the memory type (Flash, RAM, EEPROM, etc) is embedded within. This is + * done by ORing the address with some constant. For example, for AVR targets, RAM addresses are ORed with + * 0x00800000. Flash addresses are left unchanged. EEPROM addressing is not supported in GDB (for AVR targets). + * + * memoryOffsetsByType is a mapping of memory types to these known constants (which we're calling offsets). + * Because these offsets vary by target, the mapping lives here, in the GDB target descriptor. + */ BiMap memoryOffsetsByType; + + /** + * Sorted set of the known memory offsets (see memoryOffsetsByType). + */ std::set memoryOffsets; }; }