Implemented disabling of GDB packet acknowledgement, and disabled it by default. The new packetAcknowledgement debug server config param can be used to keep it enabled.

This commit is contained in:
Nav
2024-10-19 23:10:34 +01:00
parent 9b1489fbf2
commit 7a54632966
7 changed files with 73 additions and 14 deletions

View File

@@ -155,11 +155,18 @@ namespace DebugServer::Gdb::AvrGdb
}
std::set<std::pair<Feature, std::optional<std::string>>> AvrGdbRsp::getSupportedFeatures() {
return {
auto output = std::set<std::pair<Feature, std::optional<std::string>>>{
{Feature::HARDWARE_BREAKPOINTS, std::nullopt},
{Feature::SOFTWARE_BREAKPOINTS, std::nullopt},
{Feature::MEMORY_MAP_READ, std::nullopt},
{Feature::VCONT_ACTIONS_QUERY, std::nullopt},
};
if (!this->debugServerConfig.packetAcknowledgement) {
output.emplace(Feature::NO_ACK_MODE, std::nullopt);
}
return output;
}
void AvrGdbRsp::handleTargetStoppedGdbResponse(Targets::TargetMemoryAddress programAddress) {

View File

@@ -61,6 +61,17 @@ namespace DebugServer::Gdb::CommandPackets
return;
}
if (!debugSession.serverConfig.packetAcknowledgement && packetString.find("QStartNoAckMode") == 0) {
Logger::info("Handling QStartNoAckMode");
/*
* We respond to the command before actually disabling packet acknowledgement, because GDB will send one
* final acknowledgement byte to acknowledge the response.
*/
debugSession.connection.writePacket(OkResponsePacket{});
debugSession.connection.packetAcknowledgement = false;
return;
}
Logger::debug("Unknown GDB RSP packet: " + packetString + " - returning empty response");
// GDB expects an empty response for all unsupported commands

View File

@@ -135,8 +135,10 @@ namespace DebugServer::Gdb
)
);
if (this->packetAcknowledgement) {
// Acknowledge receipt
this->write({'+'});
}
output.emplace_back(std::move(rawPacket));
byteIndex = packetIndex;
@@ -149,22 +151,32 @@ namespace DebugServer::Gdb
}
void Connection::writePacket(const ResponsePacket& packet) {
// Write the packet repeatedly until the GDB client acknowledges it.
int attempts = 0;
const auto rawPacket = packet.toRawPacket();
Logger::debug("Writing GDB packet: " + std::string{rawPacket.begin(), rawPacket.end()});
do {
this->write(rawPacket);
if (this->packetAcknowledgement) {
auto attempts = std::size_t{0};
auto ackByte = this->readSingleByte(false);
while (ackByte != '+') {
if (attempts > 10) {
throw ClientCommunicationError{
"Failed to write GDB response packet - client failed to acknowledge receipt - retry limit reached"
};
}
if (ackByte == '-') {
// GDB has requested retransmission
Logger::debug("Sending packet again, upon GDB's request");
this->write(rawPacket);
attempts++;
} while (this->readSingleByte(false).value_or(0) != '+');
}
ackByte = this->readSingleByte(false);
++attempts;
}
}
}
void Connection::accept(int serverSocketFileDescriptor) {

View File

@@ -31,6 +31,8 @@ namespace DebugServer::Gdb
*/
static constexpr auto ABSOLUTE_MAXIMUM_PACKET_READ_SIZE = 2097000; // 2MiB
bool packetAcknowledgement = true;
Connection(int serverSocketFileDescriptor, EventFdNotifier& interruptEventNotifier);
Connection() = delete;

View File

@@ -11,6 +11,7 @@ namespace DebugServer::Gdb
PACKET_SIZE,
MEMORY_MAP_READ,
VCONT_ACTIONS_QUERY,
NO_ACK_MODE,
};
static inline BiMap<Feature, std::string> getGdbFeatureToNameMapping() {
@@ -20,6 +21,7 @@ namespace DebugServer::Gdb
{Feature::PACKET_SIZE, "PacketSize"},
{Feature::MEMORY_MAP_READ, "qXfer:memory-map:read"},
{Feature::VCONT_ACTIONS_QUERY, "vContSupported"},
{Feature::NO_ACK_MODE, "QStartNoAckMode"},
};
}
}

View File

@@ -42,5 +42,17 @@ namespace DebugServer::Gdb
);
}
}
if (debugServerConfig.debugServerNode["packetAcknowledgement"]) {
if (YamlUtilities::isCastable<bool>(debugServerConfig.debugServerNode["packetAcknowledgement"])) {
this->packetAcknowledgement = debugServerConfig.debugServerNode["packetAcknowledgement"].as<bool>();
} else {
Logger::error(
"Invalid GDB debug server config parameter ('packetAcknowledgement') provided - value must be"
" castable to a boolean. The parameter will be ignored."
);
}
}
}
}

View File

@@ -35,6 +35,19 @@ namespace DebugServer::Gdb
*/
bool rangeStepping = true;
/**
* Determines whether Bloom will seek to disable packet acknowledgement with GDB, at the start of the debug
* session.
*
* If this is set to false, Bloom will communicate its ability to disable package acknowledgment to GDB.
* GDB may then send the appropriate packet to disable packet acknowledgment. However, this isn't
* guaranteed - GDB may be configured to keep packet acknowledgment enabled (via the
* `set remote noack-packet off` command).
*
* This parameter is optional. If not specified, the default value set here will be used.
*/
bool packetAcknowledgement = false;
explicit GdbDebugServerConfig(const DebugServerConfig& debugServerConfig);
};
}