From e6287f7b197e75cc93570efd6e5c742b00a03019 Mon Sep 17 00:00:00 2001 From: Quentin Forcioli Date: Fri, 12 Aug 2022 15:44:54 +0200 Subject: [PATCH] base: making GDB's getbyte and send more resilient Add a try_getbyte function that feature a timeout. This function uses select to detect update on the file descriptor used to communicate with the remote. It is used to implement getbyte and to clean the file descriptor before sending a message with send. Now getbyte and send can recover from certains error like interruption by other signals (EINTR) or delays causing the remote server to send error packet to the stub. Change-Id: Ie06845ba59dee0ad831632d5bc2b15132c9d5450 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63526 Reviewed-by: Bobby Bruce Maintainer: Bobby Bruce Tested-by: kokoro --- src/base/remote_gdb.cc | 57 ++++++++++++++++++++++++++++++++++++++---- src/base/remote_gdb.hh | 1 + 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc index 4a0c632f64..fb778a64e4 100644 --- a/src/base/remote_gdb.cc +++ b/src/base/remote_gdb.cc @@ -129,7 +129,9 @@ #include "base/remote_gdb.hh" +#include #include +#include #include #include @@ -575,12 +577,54 @@ uint8_t BaseRemoteGDB::getbyte() { uint8_t b; - if (::read(fd, &b, sizeof(b)) == sizeof(b)) - return b; - - throw BadClient("Couldn't read data from debugger."); + while (!try_getbyte(&b,-1));//no timeout + return b; } +bool +BaseRemoteGDB::try_getbyte(uint8_t* c,int timeout_ms) +{ + if (!c) + panic("try_getbyte called with a null pointer as c"); + int res,retval; + //Allow read to fail if it was interrupted by a signal (EINTR). + errno = 0; + //preparing fd_sets + fd_set rfds; + FD_ZERO(&rfds); + FD_SET(fd, &rfds); + + //setting up a timeout if timeout_ms is positive + struct timeval tv;struct timeval* tv_ptr; + if (timeout_ms >= 0){ + tv.tv_sec = timeout_ms/1000; + tv.tv_usec = timeout_ms%1000; + tv_ptr = &tv; + }else{ + tv_ptr = NULL; + } + //Using select to check if the FD is ready to be read. + while(true){ + do { + errno = 0; + retval = ::select(fd + 1, &rfds, NULL, NULL, tv_ptr); + if (retval < 0 && errno != EINTR){//error + DPRINTF(GDBMisc,"getbyte failed errno=%i retval=%i\n", + errno,retval); + throw BadClient("Couldn't read data from debugger."); + } + //a EINTR error means that the select call was interrupted + //by another signal + }while (errno == EINTR); + if (retval == 0) + return false;//timed out + //reading (retval>0) + res = ::read(fd, c, sizeof(*c)); + if (res == sizeof(*c)) + return true;//read successfully + //read failed (?) retrying select + } +} void BaseRemoteGDB::putbyte(uint8_t b) { @@ -650,7 +694,8 @@ BaseRemoteGDB::send(const char *bp) uint8_t csum, c; DPRINTF(GDBSend, "send: %s\n", bp); - + //removing GDBBadP that could be waiting in the buffer + while (try_getbyte(&c,0)); do { p = bp; // Start sending a packet @@ -668,6 +713,8 @@ BaseRemoteGDB::send(const char *bp) // Try transmitting over and over again until the other end doesn't // send an error back. c = getbyte(); + if ((c & 0x7f) == GDBBadP) + DPRINTF(GDBSend, "PacketError\n"); } while ((c & 0x7f) == GDBBadP); } diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh index 53cfedc387..3d8f703f2f 100644 --- a/src/base/remote_gdb.hh +++ b/src/base/remote_gdb.hh @@ -237,6 +237,7 @@ class BaseRemoteGDB // Transfer data to/from GDB. uint8_t getbyte(); + bool try_getbyte(uint8_t* c,int timeout=-1);//return true if successful void putbyte(uint8_t b); void recv(std::vector &bp);