Tidying

src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp

@@ -42,7 +42,7 @@ namespace DebugServer::Gdb::AvrGdb
     {}
 
     std::string AvrGdbRsp::getName() const {
-        return "AVR GDB Remote Serial Protocol Debug Server";
+        return "AVR GDB Remote Serial Protocol Server";
     }
 
     std::unique_ptr<CommandPackets::AvrGdbCommandPacketInterface> AvrGdbRsp::rawPacketToCommandPacket(

src/DebugServer/Gdb/Connection.cpp

@@ -11,6 +11,7 @@
 #include "Exceptions/ClientCommunicationError.hpp"
 
 #include "src/Exceptions/Exception.hpp"
+#include "src/Exceptions/InternalFatalErrorException.hpp"
 
 #include "src/Logger/Logger.hpp"
 #include "src/Services/StringService.hpp"
@@ -207,6 +208,10 @@ namespace DebugServer::Gdb
         bool interruptible,
         std::optional<std::chrono::milliseconds> timeout
     ) {
+        if (bytes.has_value() && *bytes > Connection::ABSOLUTE_MAXIMUM_PACKET_READ_SIZE) {
+            throw InternalFatalErrorException{"Attempted to read too many bytes from GDB socket"};
+        }
+
         auto output = std::vector<unsigned char>{};
 
         if (this->readInterruptEnabled != interruptible) {

src/DebugServer/Gdb/DebugSession.hpp

@@ -36,7 +36,7 @@ namespace DebugServer::Gdb
          * The current server configuration.
          *
          * TODO: I think this should be moved out of the DebugSession struct and passed into CommandPacket::handle()
-         *       function. Review after v1.1.0.
+         *       function. Review after v2.0.0.
          */
         const GdbDebugServerConfig& serverConfig;
 

src/DebugServer/Gdb/GdbRspDebugServer.hpp

@@ -117,10 +117,6 @@ namespace DebugServer::Gdb
         GdbRspDebugServer& operator = (const GdbRspDebugServer& other) = delete;
         GdbRspDebugServer& operator = (GdbRspDebugServer&& other) = delete;
 
-        [[nodiscard]] std::string getName() const override {
-            return "GDB Remote Serial Protocol DebugServer";
-        }
-
         /**
          * Prepares the GDB server for listing on the selected address and port.
          */
@@ -291,6 +287,7 @@ namespace DebugServer::Gdb
                 return;
 
             } catch (const ::Exceptions::FatalErrorException& exception) {
+                Logger::error("Fatal error occurred - closing connection");
                 this->endDebugSession();
                 throw exception;
             }

src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp

@@ -28,8 +28,7 @@
 #include "src/Exceptions/Exception.hpp"
 #include "src/Exceptions/InvalidConfig.hpp"
 #include "src/Exceptions/InternalFatalErrorException.hpp"
-#include "src/TargetController/Exceptions/DeviceFailure.hpp"
-#include "src/TargetController/Exceptions/DeviceInitializationFailure.hpp"
+#include "src/TargetController/Exceptions/TargetFailure.hpp"
 #include "src/TargetController/Exceptions/TargetOperationFailure.hpp"
 
 #include "src/Logger/Logger.hpp"
@@ -80,18 +79,13 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
     void DebugTranslator::activate() {
         this->debugModuleDescriptor.hartIndices = this->discoverHartIndices();
         if (this->debugModuleDescriptor.hartIndices.empty()) {
-            throw Exceptions::TargetOperationFailure{"Failed to discover any RISC-V harts"};
+            throw Exceptions::TargetFailure{"Failed to discover any RISC-V harts"};
         }
 
-        Logger::debug("Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size()));
-
-        /*
-         * We only support debugging a single RISC-V hart, for now.
-         *
-         * If we discover more than one, we select the first one and ensure that this is explicitly communicated to the
-         * user.
-         */
         if (this->debugModuleDescriptor.hartIndices.size() > 1) {
+            Logger::debug(
+                "Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size())
+            );
             Logger::warning("Bloom only supports debugging a single RISC-V hart - selecting first available");
         }
 
@@ -156,7 +150,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         }
 
         if (this->debugModuleDescriptor.memoryAccessStrategies.empty()) {
-            throw Exceptions::TargetOperationFailure{"Target doesn't support any known memory access strategies"};
+            throw Exceptions::TargetFailure{"Target doesn't support any known memory access strategies"};
         }
 
         this->memoryAccessStrategy = this->determineMemoryAccessStrategy();
@@ -203,7 +197,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         this->writeDebugModuleControlRegister(controlRegister);
 
         if (!statusRegister.allHalted) {
-            throw Exceptions::Exception{"Target took too long to halt selected harts"};
+            throw Exceptions::TargetOperationFailure{"Target took too long to halt selected harts"};
         }
     }
 
@@ -233,7 +227,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         if (!statusRegister.allResumeAcknowledge) {
             Logger::debug("Failed to resume target execution - stopping target");
             this->stop();
-            throw Exceptions::Exception{"Target took too long to acknowledge resume request"};
+            throw Exceptions::TargetOperationFailure{"Target took too long to acknowledge resume request"};
         }
     }
 
@@ -292,7 +286,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         });
 
         if (!statusRegister.allHaveReset) {
-            throw Exceptions::Exception{"Target took too long to reset"};
+            throw Exceptions::TargetOperationFailure{"Target took too long to reset"};
         }
 
         this->initDebugControlStatusRegister();
@@ -307,7 +301,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
 
         const auto triggerDescriptorOpt = this->getAvailableTrigger();
         if (!triggerDescriptorOpt.has_value()) {
-            throw Exceptions::Exception{"Insufficient resources - no available trigger"};
+            throw Exceptions::TargetOperationFailure{"Insufficient resources - no available trigger"};
         }
 
         const auto& triggerDescriptor = triggerDescriptorOpt->get();
@@ -343,13 +337,13 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
             return;
         }
 
-        throw Exceptions::Exception{"Unsupported trigger"};
+        throw Exceptions::TargetOperationFailure{"Unsupported trigger"};
     }
 
     void DebugTranslator::clearTriggerBreakpoint(TargetMemoryAddress address) {
         const auto triggerIndexIt = this->triggerIndicesByBreakpointAddress.find(address);
         if (triggerIndexIt == this->triggerIndicesByBreakpointAddress.end()) {
-            throw Exceptions::Exception{"Unknown hardware breakpoint"};
+            throw Exceptions::TargetOperationFailure{"Unknown hardware breakpoint"};
         }
 
         const auto& triggerDescriptor = this->debugModuleDescriptor.triggerDescriptorsByIndex.at(triggerIndexIt->second);
@@ -566,7 +560,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
             }
 
             if (writeSelectError != AbstractCommandError::NONE) {
-                throw Exceptions::Exception{
+                throw Exceptions::TargetOperationFailure{
                     "Failed to write to TRIGGER_SELECT register - abstract command error: 0x"
                         + Services::StringService::toHex(static_cast<std::uint8_t>(writeSelectError))
                 };
@@ -649,7 +643,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         }
 
         if (!controlRegister.debugModuleActive) {
-            throw Exceptions::Exception{"Took too long to enable debug module"};
+            throw Exceptions::TargetOperationFailure{"Took too long to enable debug module"};
         }
     }
 
@@ -671,7 +665,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         }
 
         if (controlRegister.debugModuleActive) {
-            throw Exceptions::Exception{"Took too long to disable debug module"};
+            throw Exceptions::TargetOperationFailure{"Took too long to disable debug module"};
         }
     }
 
@@ -720,7 +714,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         const auto result = this->tryReadCpuRegister(number, flags);
 
         if (!result.hasValue()) {
-            throw Exceptions::Exception{
+            throw Exceptions::TargetOperationFailure{
                 "Failed to read CPU register (number: 0x" + Services::StringService::toHex(number)
                     + ") - abstract command error: 0x"
                     + Services::StringService::toHex(static_cast<std::uint8_t>(result.error()))
@@ -770,7 +764,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
     ) {
         const auto commandError = this->tryWriteCpuRegister(number, value, flags);
         if (commandError != AbstractCommandError::NONE) {
-            throw Exceptions::Exception{
+            throw Exceptions::TargetOperationFailure{
                 "Failed to write to CPU register (number: 0x" + Services::StringService::toHex(number)
                     + ") - abstract command error: 0x"
                     + Services::StringService::toHex(static_cast<std::uint8_t>(commandError))
@@ -825,7 +819,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         }
 
         if (abstractStatusRegister.busy) {
-            throw Exceptions::Exception{"Abstract command took too long to execute"};
+            throw Exceptions::TargetOperationFailure{"Abstract command took too long to execute"};
         }
 
         if (abstractStatusRegister.commandError != AbstractCommandError::NONE) {
@@ -840,7 +834,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
     ) {
         const auto commandError = this->tryExecuteAbstractCommand(abstractCommandRegister);
         if (commandError != AbstractCommandError::NONE) {
-            throw Exceptions::Exception{
+            throw Exceptions::TargetOperationFailure{
                 "Failed to execute abstract command - error: 0x"
                     + Services::StringService::toHex(static_cast<std::uint8_t>(commandError))
             };
@@ -969,7 +963,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         });
 
         if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) {
-            throw Exceptions::Exception{
+            throw Exceptions::TargetOperationFailure{
                 "Cannot read memory via RISC-V debug module program buffer - insufficient program buffer size"
             };
         }
@@ -1043,7 +1037,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
             if (abstractStatusRegister.commandError != AbstractCommandError::NONE) {
                 this->clearAbstractCommandError();
 
-                throw Exceptions::Exception{
+                throw Exceptions::TargetOperationFailure{
                     "Program buffer execution failed - abstract command error: 0x"
                         + Services::StringService::toHex(abstractStatusRegister.commandError)
                 };
@@ -1091,7 +1085,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
         });
 
         if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) {
-            throw Exceptions::Exception{
+            throw Exceptions::TargetOperationFailure{
                 "Cannot write to memory via RISC-V debug module program buffer - insufficient program buffer size"
             };
         }
@@ -1144,7 +1138,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
             if (abstractStatusRegister.commandError != AbstractCommandError::NONE) {
                 this->clearAbstractCommandError();
 
-                throw Exceptions::Exception{
+                throw Exceptions::TargetOperationFailure{
                     "Program buffer execution failed - abstract command error: 0x"
                         + Services::StringService::toHex(abstractStatusRegister.commandError)
                 };
@@ -1203,7 +1197,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
             return;
         }
 
-        throw Exceptions::Exception{"Unsupported trigger"};
+        throw Exceptions::TargetOperationFailure{"Unsupported trigger"};
     }
 
     DebugTranslator::PreservedCpuRegister::PreservedCpuRegister(
@@ -1238,9 +1232,9 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec
              * will be left in an undefined state. More specifically, the state of the program running on the target
              * may be corrupted. We cannot recover from this.
              *
-             * DeviceFailure exceptions are considered to be fatal. A clean shutdown will follow.
+             * TargetFailure exceptions are considered to be fatal. A clean shutdown will follow.
              */
-            throw Exceptions::DeviceFailure{
+            throw Exceptions::TargetFailure{
                 "Failed to restore CPU register (number: 0x"
                     + Services::StringService::toHex(this->registerNumber) + ") - error: " + exception.getMessage()
                     + " - the target is now in an undefined state and may require a reset"

src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.cpp

@@ -142,21 +142,24 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink
         Targets::TargetMemoryAddress startAddress,
         Targets::TargetMemoryBufferSpan buffer
     ) {
-        constexpr auto packetSize = std::uint8_t{64};
+        constexpr auto packetSize = WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE;
         const auto bufferSize = static_cast<Targets::TargetMemorySize>(buffer.size());
-        const auto packetsRequired = static_cast<std::uint32_t>(
+        const auto packetsRequired = static_cast<std::size_t>(
             std::ceil(static_cast<float>(bufferSize) / static_cast<float>(packetSize))
         );
 
-        for (auto i = std::uint32_t{0}; i < packetsRequired; ++i) {
-            const auto segmentSize = static_cast<std::uint8_t>(
-                std::min(
-                    static_cast<std::uint8_t>(bufferSize - (i * packetSize)),
+        for (auto i = std::size_t{0}; i < packetsRequired; ++i) {
+            const auto segmentSize = std::min(
+                static_cast<Targets::TargetMemorySize>(bufferSize - (i * packetSize)),
                 packetSize
-                )
             );
+            assert(segmentSize <= 0xFF);
+
             const auto response = this->sendCommandAndWaitForResponse(
-                Commands::PreparePartialFlashBlockWrite{startAddress + (packetSize * i), segmentSize}
+                Commands::PreparePartialFlashBlockWrite{
+                    static_cast<Targets::TargetMemorySize>(startAddress + (packetSize * i)),
+                    static_cast<std::uint8_t>(segmentSize)
+                }
             );
 
             if (response.payload.size() != 1) {

src/DebugToolDrivers/WCH/Protocols/WchLink/WchLinkInterface.hpp

@@ -30,6 +30,8 @@ namespace DebugToolDrivers::Wch::Protocols::WchLink
         : public ::DebugToolDrivers::Protocols::RiscVDebugSpec::DebugTransportModuleInterface
     {
     public:
+        static constexpr Targets::TargetMemorySize MAX_PARTIAL_BLOCK_WRITE_SIZE = 64;
+
         WchLinkInterface(Usb::UsbInterface& usbInterface, Usb::UsbDevice& usbDevice);
 
         DeviceInfo getDeviceInfo();

src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp

@@ -210,7 +210,7 @@ namespace DebugToolDrivers::Wch
              * See WchLinkDebugInterface::writeFlashMemory() below, for more.
              */
             const auto bufferSize = static_cast<TargetMemorySize>(buffer.size());
-            const auto alignmentSize = bufferSize > WchLinkDebugInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE
+            const auto alignmentSize = bufferSize > WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE
                 ? this->programmingBlockSize
                 : 1;
 
@@ -286,7 +286,7 @@ namespace DebugToolDrivers::Wch
          *     Writes any number of bytes to flash, but limited to a maximum of 64 bytes per write. Larger writes
          *     must be split into multiple writes.
          * - Full block write
-         *     Writes an entire block to flash. Where the block size is target-specific (resides in the target's
+         *     Writes an entire block to flash, where the block size is target-specific (resides in the target's
          *     TDF). Requires alignment to the block size. Requires reattaching to the target at the end of the
          *     programming session.
          *
@@ -294,7 +294,7 @@ namespace DebugToolDrivers::Wch
          * target. But the partial block write is faster and more suitable for writing buffers that are smaller than
          * 64 bytes, such as when we're inserting software breakpoints.
          */
-        if (buffer.size() <= WchLinkDebugInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE) {
+        if (buffer.size() <= WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE) {
             return this->wchLinkInterface.writeFlashPartialBlock(startAddress, buffer);
         }
 

src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp

@@ -77,8 +77,6 @@ namespace DebugToolDrivers::Wch
         ) override;
 
     private:
-        static constexpr Targets::TargetMemorySize MAX_PARTIAL_BLOCK_WRITE_SIZE = 64;
-
         const WchLinkToolConfig& toolConfig;
         const Targets::RiscV::RiscVTargetConfig& targetConfig;
         const Targets::RiscV::TargetDescriptionFile& targetDescriptionFile;

src/TargetController/Exceptions/TargetFailure.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+#include "src/Exceptions/FatalErrorException.hpp"
+
+namespace Exceptions
+{
+    class TargetFailure: public FatalErrorException
+    {
+    public:
+        explicit TargetFailure(const std::string& message)
+            : FatalErrorException(message)
+        {}
+    };
+}

src/Targets/RiscV/Wch/WchRiscV.cpp

@@ -118,7 +118,7 @@ namespace Targets::RiscV::Wch
          *
          * @TODO: Currently, this just assumes that the alias segment always maps to the program memory segment, but I
          *        believe it may map to the boot program memory segment in some cases. This needs to be revisited
-         *        before v1.1.0.
+         *        before v2.0.0.
          */
         if (memorySegmentDescriptor == this->mappedProgramMemorySegmentDescriptor) {
             const auto newAddress = startAddress - this->mappedProgramMemorySegmentDescriptor.addressRange.startAddress

src/Targets/TargetPinDescriptor.hpp

@@ -10,7 +10,7 @@ namespace Targets
     {
     public:
         std::string position;
-        std::uint16_t numericPosition; // TODO: Consider removing this bodge. Review after v1.1.0
+        std::uint16_t numericPosition; // TODO: Consider removing this bodge. Review after v2.0.0
         std::optional<std::string> padKey;
 
         TargetPinDescriptor(