Quentin Forcioli has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/63526?usp=email )

Change subject: base: making GDB's getbyte and send more resilient
......................................................................

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 <bbr...@ucdavis.edu>
Maintainer: Bobby Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 76 insertions(+), 5 deletions(-)

Approvals:
  Bobby Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index 4a0c632..fb778a6 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -129,7 +129,9 @@

 #include "base/remote_gdb.hh"

+#include <sys/select.h>
 #include <sys/signal.h>
+#include <sys/time.h>
 #include <unistd.h>

 #include <cassert>
@@ -575,12 +577,54 @@
 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 @@
     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 @@
         // 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 53cfedc..3d8f703 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -237,6 +237,7 @@

     // 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<char> &bp);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/63526?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ie06845ba59dee0ad831632d5bc2b15132c9d5450
Gerrit-Change-Number: 63526
Gerrit-PatchSet: 11
Gerrit-Owner: Quentin Forcioli <quentin.forci...@telecom-paris.fr>
Gerrit-Reviewer: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Quentin Forcioli <quentin.forci...@telecom-paris.fr>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org

Reply via email to