[Simulavr-devel] Implementing GDB "target remote | simulavr"

[Petr Hluzín]

Hi folks

GDB can launch and connect to a simulator or other remote target by
typing "target remote | SIM". GDB will launch the program 'SIM',
attach its standard input and output internally and send remote
protocol packets to it. [1]

This has some advantages:
* user can have multiple simulation sessions without any hassle
** tools can use simulavr without risks of affecting computer's
globally visible state
* simulavr's listening sockets is not exposed to network attacks
** personal firewalls will not bark om creating listening socket
* users are not required to type something they do not care about
(specially when using just one instance)

If implemented in simulavr, it needs to know if:
a) user is using it standalone, with --file xxx.elf
b) user will connect gdb to TCP port, with --gdbserver
c) user is connecting via this proposed thing, with flag --gdb-pipe or
--gdb-stdin

Should we name the option "--gdb-pipe" or "--gdb-stdin"? Or something else?
Why would user prefer the one?

Note: We should be able to detect whether GDB is trying to push remote
packets on stdin or not - when/if proper select() mechanism is
implemented. So we might live without the option. Or at least provide
diagnostic.

I wrote some code which is supposed to implement this feature but it
does not work well.
On native windows reads work fine, but simulavr blocks indefinitely
when trying to flush stderr.
On windows against Cygwin-built GDB: reads from stdin yield nothing,
but 'yes | simulavr --gdb-pipe' works fine.
On gentoo linux: ./configure insists on BFD library and I do not know
how to workaround it, so not tested.

[1] http://sourceware.org/gdb/current/onlinedocs/gdb/Connecting.html#Connecting

The patch is attached. (The coding style sucks because I tried
everything to get it work.) Comments welcome. I ran out of ideas how
to fix it.

If you want to test the patch then patch against recent git tree
because simulavr used to output some diagnostic to stdout which
confused GDB. (There might be some remaining issues.) Use command like
"target remote | outdir/simulavr.exe --device atmega8 --gdb-stdin". If
you get message "Remote communication error: Resource temporarily
unavailable." then it appears to mean "Connection to target closed
unexpectedly".


diff --git a/src/avrerror.cpp b/src/avrerror.cpp
index 6f05056..2c2378a 100644
--- a/src/avrerror.cpp
+++ b/src/avrerror.cpp
@@ -167,6 +170,7 @@ void SystemConsoleHandler::vffatal(const char
*file, int line, const char *fmt,
     va_end(ap);
     if(useExitAndAbort) {
         *wrnStream << "\n" << messageStringBuffer << "\n" << std::endl;
+        wrnStream->flush();
         exit(1);
     } else {
         throw (char const*)messageStringBuffer;
diff --git a/src/cmd/gdbserver.cpp b/src/cmd/gdbserver.cpp
index 91ccd49..379c152 100644
--- a/src/cmd/gdbserver.cpp
+++ b/src/cmd/gdbserver.cpp
@@ -49,6 +49,7 @@ using namespace std;
 #include "gdb.h"

 #ifdef _MSC_VER
+#  include <io.h>
 #  define snprintf _snprintf
 #endif

@@ -93,8 +94,12 @@ void GdbServerSocketMingW::End() {

 GdbServerSocketMingW::GdbServerSocketMingW(int port): _socket(0), _conn(0) {
     sockaddr_in sa;
-
     Start();
+    if(port == -1) {
+        // GDB is connected on stdin/stdout, no TCP.
+        _conn = -1;
+        _socket = INVALID_SOCKET;
+    } else {
     _socket = socket(AF_INET, SOCK_STREAM, 0);
     if(_socket == INVALID_SOCKET)
         avr_error("Couldn't create socket: INVALID_SOCKET");
@@ -111,6 +116,7 @@ GdbServerSocketMingW::GdbServerSocketMingW(int
port): _socket(0), _conn(0) {
     }

     listen(_socket, 1); // only 1 connection at time
+    }
 }

 GdbServerSocketMingW::~GdbServerSocketMingW() {
@@ -127,26 +133,36 @@ void GdbServerSocketMingW::Close(void) {

 int GdbServerSocketMingW::ReadByte(void) {
     char buf[1];
-    int rv = recv(_conn, buf, 1, 0);
+    _doserrno = 0;
+    int rv = (_conn != -1) ? recv( _conn, buf, 1, 0 ) : _read(0, buf, 1);
+    int nErr = _doserrno;
+    bool asfsaf = (nErr == EBADF);
+    char * sErr = strerror(nErr);
     if(rv <= 0)
         return -1;
     return buf[0];
 }

 void GdbServerSocketMingW::Write(const void* buf, size_t count) {
-    send(_conn, (const char *)buf, count, 0);
+    if(_conn != -1)
+        send(_conn, (const char *)buf, count, 0);
+    else
+        _write(1, buf, count);
 }

 void GdbServerSocketMingW::SetBlockingMode(int mode) {
+    if(_conn == INVALID_SOCKET)
+        return;  // GDB on stdin/stdout, non-blocking not possible on Windows
     u_long arg = 1;
     if(mode)
         arg = 0;
     int res = ioctlsocket(_conn, FIONBIO, &arg);
-    if(res)
-        avr_warning( "fcntl failed: %d\n", WSAGetLastError() );
+    if(res)
+        avr_warning( "fcntl failed: %d\n", WSAGetLastError() );
 }
-
 bool GdbServerSocketMingW::Connect(void) {
+    if(_socket==INVALID_SOCKET)
+        return true;  // GDB is already connected to stdin/stdout
     _conn = accept(_socket, 0, 0);
     if(_conn == INVALID_SOCKET) {
         int rc = WSAGetLastError();
@@ -157,7 +173,6 @@ bool GdbServerSocketMingW::Connect(void) {
     }
     return true;
 }
-
 void GdbServerSocketMingW::CloseConnection(void) {
     closesocket(_conn);
 }
@@ -167,6 +182,10 @@ void GdbServerSocketMingW::CloseConnection(void) {
 GdbServerSocketUnix::GdbServerSocketUnix(int port) {
     conn = -1;        //no connection opened

+    if(port == -1) {
+        // GDB is connected on stdin/stdout, no TCP.
+        _conn = -1;
+    } else {
     if((sock = socket(PF_INET, SOCK_STREAM, 0)) < 0)
         avr_error("Can't create socket: %s", strerror(errno));

@@ -186,6 +205,7 @@ GdbServerSocketUnix::GdbServerSocketUnix(int port) {

     if(listen(sock, 1) < 0)
         avr_error("Can not listen on socket: %s", strerror(errno));
+    }
 }

 GdbServerSocketUnix::~GdbServerSocketUnix() {
@@ -203,7 +223,7 @@ int GdbServerSocketUnix::ReadByte(void) {
     int cnt = MAX_READ_RETRY;

     while(cnt--) {
-        res = read(conn, &c, 1);
+        res = (conn != -1) ? read(conn, &c, 1) : read(0, &c, 1);
         if(res < 0) {
             if (errno == EAGAIN)
                 /* fd was set to non-blocking and no data was available */
@@ -227,7 +247,7 @@ int GdbServerSocketUnix::ReadByte(void) {
 void GdbServerSocketUnix::Write(const void* buf, size_t count) {
     int res;

-    res = write(conn, buf, count);
+    res = (conn != -1) ? write(conn, buf, count) : write(1, buf, count);

     /* FIXME: should we try and catch interrupted system calls here? */
     if(res < 0)
@@ -302,8 +322,10 @@ GdbServer::GdbServer(AvrDevice *c, int _port, int
debug, int _waitForGdbConnecti
     server = new GdbServerSocketUnix(_port);
 #endif

-    fprintf(stderr, "Waiting on port %d for gdb client to
connect...\n", _port);
-
+    if(_port != -1)
+        fprintf(stderr, "Waiting on port %d for gdb client to
connect...\n", _port);
+    else
+        fprintf(stderr, "Using gdb client in stdin/stdout.\n");
 }

 //make the instance of static list of all gdb servers here
@@ -1318,7 +1340,7 @@ void GdbServer::Run( )
     }
 }

-//! try to open a new connection to gdb
+//! try to accept a new connection from gdb
 void GdbServer::TryConnectGdb() {
     time_t newTime = time(NULL);

diff --git a/src/cmd/main.cpp b/src/cmd/main.cpp
index a5341c3..65d4a55 100644
--- a/src/cmd/main.cpp
+++ b/src/cmd/main.cpp
@@ -152,7 +152,16 @@ int main(int argc, char *argv[]) {
     string writeToPipeFileName = "";

     vector<string> terminationArgs;
-
+/*
+       Sleep(10000);
+       byte buff[100];
+       DWORD bytes = -5;
+       HANDLE handle = GetStdHandle(STD_INPUT_HANDLE);
+       ::ReadFile(handle, buff, 10, &bytes, NULL);
+       ::CloseHandle(handle);
+       bytes = fread(buff, 1, 10, stdin);
+       FILE * x = freopen( "CONIN$", "r", stdin );
+*/
     vector<string> tracer_opts;
     bool tracer_dump_avail = false;
     string tracer_avail_out;
@@ -164,6 +173,8 @@ int main(int argc, char *argv[]) {
             {"file", 1, 0, 'f'},
             {"device", 1, 0, 'd'},
             {"gdbserver", 0, 0, 'g'},
+            {"gdb-stdin", 0, 0, 'H'},
+            {"gdb-pipe", 0, 0, 'H'},
             {"maxruntime", 1, 0, 'm'},
             {"nogdbwait", 0, 0, 'n'},
             {"trace", 1, 0, 't'},
@@ -276,7 +287,15 @@ int main(int argc, char *argv[]) {
                 global_gdb_debug = 1;
                 gdbserver_flag = 1;
                 break;
-
+
+            case 'H':  // launched from GDB: "target remote |
simulavr --gdb-stdin"
+                if (global_verbose_on)
+                    cout << "Running with GDB on stdin" << endl;
+                gdbserver_flag=1;
+                global_gdbserver_port = -1;
+                _CrtDbgBreak();
+                break;
+
             case 'p':
                 if(!StringToLong( optarg, &global_gdbserver_port, NULL, 10)) {
                     cerr << "GDB Server Port is not a number" << endl;
@@ -344,7 +363,7 @@ int main(int argc, char *argv[]) {
     SetDumpTraceArgs(tracer_opts, dev1);

     if(!gdbserver_flag && filename == "unknown") {
-        cerr << "Specify either --file <executable> or --gdbserver" << endl;
+        cerr << "Specify either --file <executable> or --gdbserver
(or --gdb-stdin)" << endl;
         exit(1);
     }



-- 
Petr Hluzin

gdb-pipe-connection.patch
Description: Binary data