Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/3180
Change subject: misc: Make the remote GDB stub more resilient to bad
connections.
......................................................................
misc: Make the remote GDB stub more resilient to bad connections.
Currently, if the remote gdb stub fails to read a byte from an incoming
packet because the connection has been dropped, the read call will return
anyway and the calling code will have no way to know something bad
happened. It might reattempt the read over and over again waiting for some
particular byte, doomed to never make forward progress.
This change modifies the remote GDB code so that if a read or write call
fails, it will instead detach from the debugger and continue. Before this
change, When simulating a port scan, ie connecting to the debugger port
and then immediately dropping the connection using this command:
nc -v -n -z -w 1 127.0.0.1 7000
gem5 would enter the previously described death spiral. After it, gem5
detaches from the bad connection and resumes execution. Subsequently
attaching with gdb was successful.
This code is written in a C centric style, and would benefit from some
refactoring.
Change-Id: Ie3c0bb35b9cfe3671d0f731e3907548bae0d292f
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 144 insertions(+), 75 deletions(-)
diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 665dba8..7a0e3b1 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -125,6 +125,7 @@
#include <unistd.h>
#include <csignal>
+#include <cstdint>
#include <cstdio>
#include <string>
@@ -368,24 +369,32 @@
//
//
-uint8_t
-BaseRemoteGDB::getbyte()
+bool
+BaseRemoteGDB::getbyte(uint8_t &b)
{
- uint8_t b;
- if (::read(fd, &b, 1) != 1)
- warn("could not read byte from debugger");
- return b;
+ if (::read(fd, &b, sizeof(b)) == sizeof(b)) {
+ return true;
+ } else {
+ warn("Couldn't read data from debugger, detaching.");
+ detach();
+ return false;
+ }
}
-void
+bool
BaseRemoteGDB::putbyte(uint8_t b)
{
- if (::write(fd, &b, 1) != 1)
- warn("could not write byte to debugger");
+ if (::write(fd, &b, sizeof(b)) == sizeof(b)) {
+ return true;
+ } else {
+ warn("Couldn't write data to the debugger, detaching.");
+ detach();
+ return false;
+ }
}
// Send a packet to gdb
-void
+int
BaseRemoteGDB::send(const char *bp)
{
const char *p;
@@ -396,20 +405,27 @@
do {
p = bp;
//Start sending a packet
- putbyte(GDBStart);
+ if (!putbyte(GDBStart))
+ return -1;
//Send the contents, and also keep a check sum.
for (csum = 0; (c = *p); p++) {
- putbyte(c);
+ if (!putbyte(c))
+ return -1;
csum += c;
}
- //Send the ending character.
- putbyte(GDBEnd);
- //Sent the checksum.
- putbyte(i2digit(csum >> 4));
- putbyte(i2digit(csum));
- //Try transmitting over and over again until the other end doesn't
send an
- //error back.
- } while ((c = getbyte() & 0x7f) == GDBBadP);
+ if (//Send the ending character.
+ !putbyte(GDBEnd) ||
+ //Sent the checksum.
+ !putbyte(i2digit(csum >> 4)) ||
+ !putbyte(i2digit(csum))) {
+ return -1;
+ }
+ //Try transmitting over and over again until the other end doesn't
+ //send an error back.
+ if (!getbyte(c))
+ return -1;
+ } while ((c & 0x7f) == GDBBadP);
+ return 0;
}
// Receive a packet from gdb
@@ -417,19 +433,26 @@
BaseRemoteGDB::recv(char *bp, int maxlen)
{
char *p;
- int c, csum;
+ uint8_t c;
+ int csum;
int len;
do {
p = bp;
csum = len = 0;
//Find the beginning of a packet
- while ((c = getbyte()) != GDBStart)
- ;
+ do {
+ if (!getbyte(c))
+ return -1;
+ } while (c != GDBStart);
//Read until you find the end of the data in the packet, and keep
//track of the check sum.
- while ((c = getbyte()) != GDBEnd && len < maxlen) {
+ while (len < maxlen) {
+ if (!getbyte(c))
+ return -1;
+ if (c == GDBEnd)
+ break;
c &= 0x7f;
csum += c;
*p++ = c;
@@ -442,29 +465,35 @@
//If the command was too long, report an error.
if (len >= maxlen) {
- putbyte(GDBBadP);
+ if (!putbyte(GDBBadP))
+ return -1;
continue;
}
//Bring in the checksum. If the check sum matches, csum will be 0.
- csum -= digit2i(getbyte()) * 16;
- csum -= digit2i(getbyte());
+ uint8_t csum1, csum2;
+ if (!getbyte(csum1) || !getbyte(csum2))
+ return -1;
+ csum -= digit2i(csum1) * 16;
+ csum -= digit2i(csum2);
//If the check sum was correct
if (csum == 0) {
//Report that the packet was received correctly
- putbyte(GDBGoodP);
+ if (!putbyte(GDBGoodP))
+ return -1;
// Sequence present?
if (bp[2] == ':') {
- putbyte(bp[0]);
- putbyte(bp[1]);
+ if (!putbyte(bp[0]) || !putbyte(bp[1]))
+ return -1;
len -= 3;
memcpy(bp, bp+3, len);
}
break;
}
//Otherwise, report that there was a mistake.
- putbyte(GDBBadP);
+ if (!putbyte(GDBBadP))
+ return -1;
} while (1);
DPRINTF(GDBRecv, "recv: %s: %s\n", gdb_command(*bp), bp);
@@ -732,14 +761,18 @@
} else {
// Tell remote host that an exception has occurred.
snprintf(buffer, bufferSize, "S%02x", type);
- send(buffer);
+ if (send(buffer) < 0)
+ goto out;
}
// Stick frame regs into our reg cache.
regCache->getRegs(context);
for (;;) {
- datalen = recv(data, sizeof(data));
+ int recved = recv(data, sizeof(data));
+ if (recved < 0)
+ goto out;
+ datalen = recved;
data[sizeof(data) - 1] = 0; // Sentinel
command = data[0];
subcmd = 0;
@@ -752,7 +785,8 @@
// of this loop when he issues a "remote-signal".
snprintf(buffer, bufferSize,
"S%02x", type);
- send(buffer);
+ if (send(buffer) < 0)
+ goto out;
continue;
case GDBRegR:
@@ -760,36 +794,43 @@
panic("buffer too small");
mem2hex(buffer, regCache->data(), regCache->size());
- send(buffer);
+ if (send(buffer) < 0)
+ goto out;
continue;
case GDBRegW:
p = hex2mem(regCache->data(), p, regCache->size());
- if (p == NULL || *p != '\0')
- send("E01");
- else {
+ if (p == NULL || *p != '\0') {
+ if (send("E01") < 0)
+ goto out;
+ } else {
regCache->setRegs(context);
- send("OK");
+ if (send("OK") < 0)
+ goto out;
}
continue;
case GDBMemR:
val = hex2i(&p);
if (*p++ != ',') {
- send("E02");
+ if (send("E02") < 0)
+ goto out;
continue;
}
len = hex2i(&p);
if (*p != '\0') {
- send("E03");
+ if (send("E03") < 0)
+ goto out;
continue;
}
if (len > bufferSize) {
- send("E04");
+ if (send("E04") < 0)
+ goto out;
continue;
}
if (!acc(val, len)) {
- send("E05");
+ if (send("E05") < 0)
+ goto out;
continue;
}
@@ -798,50 +839,65 @@
// officially support those...
char *temp = new char[2*len+1];
mem2hex(temp, buffer, len);
- send(temp);
+ if (send(temp) < 0) {
+ delete [] temp;
+ goto out;
+ }
delete [] temp;
} else {
- send("E05");
+ if (send("E05") < 0)
+ goto out;
}
continue;
case GDBMemW:
val = hex2i(&p);
if (*p++ != ',') {
- send("E06");
+ if (send("E06") < 0)
+ goto out;
continue;
}
len = hex2i(&p);
if (*p++ != ':') {
- send("E07");
+ if (send("E07") < 0)
+ goto out;
continue;
}
if (len > datalen - (p - data)) {
- send("E08");
+ if (send("E08") < 0)
+ goto out;
continue;
}
p = hex2mem(buffer, p, bufferSize);
if (p == NULL) {
- send("E09");
+ if (send("E09") < 0)
+ goto out;
continue;
}
if (!acc(val, len)) {
- send("E0A");
+ if (send("E0A") < 0)
+ goto out;
continue;
}
- if (write(val, (size_t)len, buffer))
- send("OK");
- else
- send("E0B");
+ if (write(val, (size_t)len, buffer)) {
+ if (send("OK") < 0)
+ goto out;
+ } else {
+ if (send("E0B") < 0)
+ goto out;
+ }
continue;
case GDBSetThread:
subcmd = *p++;
val = hex2i(&p);
- if (val == 0)
- send("OK");
- else
- send("E01");
+ if (val == 0) {
+ if (send("OK") < 0)
+ goto out;
+ } else {
+ if (send("E01") < 0)
+ goto out;
+ }
continue;
case GDBDetach:
@@ -887,9 +943,11 @@
case GDBClrHwBkpt:
subcmd = *p++;
- if (*p++ != ',') send("E0D");
+ if (*p++ != ',' && send("E0D") < 0)
+ goto out;
val = hex2i(&p);
- if (*p++ != ',') send("E0D");
+ if (*p++ != ',' && send("E0D") < 0)
+ goto out;
len = hex2i(&p);
DPRINTF(GDBMisc, "clear %s, addr=%#x, len=%d\n",
@@ -910,18 +968,22 @@
case '3': // read watchpoint
case '4': // access watchpoint
default: // unknown
- send("");
+ if (send("") < 0)
+ goto out;
break;
}
- send(ret ? "OK" : "E0C");
+ if (send(ret ? "OK" : "E0C") < 0)
+ goto out;
continue;
case GDBSetHwBkpt:
subcmd = *p++;
- if (*p++ != ',') send("E0D");
+ if (*p++ != ',' && send("E0D") < 0)
+ goto out;
val = hex2i(&p);
- if (*p++ != ',') send("E0D");
+ if (*p++ != ',' && send("E0D") < 0)
+ goto out;
len = hex2i(&p);
DPRINTF(GDBMisc, "set %s, addr=%#x, len=%d\n",
@@ -942,19 +1004,24 @@
case '3': // read watchpoint
case '4': // access watchpoint
default: // unknown
- send("");
+ if (send("") < 0)
+ goto out;
break;
}
- send(ret ? "OK" : "E0C");
+ if (send(ret ? "OK" : "E0C") < 0)
+ goto out;
continue;
case GDBQueryVar:
var = string(p, datalen - 1);
- if (var == "C")
- send("QC0");
- else
- send("");
+ if (var == "C") {
+ if (send("QC0") < 0)
+ goto out;
+ } else {
+ if (send("") < 0)
+ goto out;
+ }
continue;
case GDBSetBaud:
@@ -972,14 +1039,16 @@
DPRINTF(GDBMisc, "Unsupported command: %s\n",
gdb_command(command));
DDUMP(GDBMisc, (uint8_t *)data, datalen);
- send("");
+ if (send("") < 0)
+ goto out;
continue;
default:
// Unknown command.
DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
command, command);
- send("");
+ if (send("") < 0)
+ goto out;
continue;
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 2ab7a84..d211345 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -203,11 +203,11 @@
};
protected:
- uint8_t getbyte();
- void putbyte(uint8_t b);
+ bool getbyte(uint8_t &b);
+ bool putbyte(uint8_t b);
int recv(char *data, int len);
- void send(const char *data);
+ int send(const char *data);
protected:
// Machine memory
--
To view, visit https://gem5-review.googlesource.com/3180
To unsubscribe, visit https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie3c0bb35b9cfe3671d0f731e3907548bae0d292f
Gerrit-Change-Number: 3180
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
