[PATCH v2 1/2] Use a shell command as a socket for gdbserver

[PATCH v2 1/2] Use a shell command as a socket for gdbserver

[Gabriel Corona]

This brings feature parity with gdb in this regard.

This can be used to do gdbserver over unix socket (without redirecting
the inferior stdio to /dev/null):

    $ gdbserver '|socat STDIO UNIX-LISTEN:foo.sock' ./foo

and in gdb:

    (gdb) target remote |socat STDIO UNIX:foo.sock

Altneratively, we can initiate a connection to a remote server:

    $ gdbserver '|socat STDIO TCP:gdb.example.com:9000' ./foo

gdb/gdbserver/ChangeLog:

        * remote-utils.c: add support for using a shell command stdio as COMM
        * remote.c: add documentation about this feature
---
 gdb/gdbserver/remote-utils.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/server.c       |  3 +-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..f15f1ce 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -58,6 +58,8 @@
 #endif
 #include <sys/stat.h>
 
+#include "gdb_wait.h"
+
 #if USE_WIN32API
 #include <winsock2.h>
 #endif
@@ -239,6 +241,14 @@ remote_prepare (char *name)
       return;
     }
 
+#ifndef USE_WIN32API
+  if (name[0] == '|')
+    {
+      transport_is_reliable = 1;
+      return;
+    }
+#endif
+
   port_str = strchr (name, ':');
   if (port_str == NULL)
     {
@@ -280,6 +290,56 @@ remote_prepare (char *name)
   transport_is_reliable = 1;
 }
 
+#ifndef USE_WIN32API
+static int
+open_shell_command (char *command)
+{
+  int sockets[2];
+  pid_t child, grandchild, res;
+  int status;
+
+  if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sockets) < 0)
+    perror_with_name ("Can't get socketpair");
+  child = fork ();
+  if (child < 0)
+    {
+      perror_with_name ("Can't fork");
+    }
+  else if (child == 0)
+    {
+      if (close (sockets[0]) < 0)
+	perror_with_name ("Can't close socket");
+      if (dup2 (sockets[1], 0) < 0 || dup2 (sockets[1], 1) < 0)
+	perror_with_name ("Can't dup socket to stdio");
+      if (close (sockets[1]) < 0)
+	perror_with_name ("Can't close original socket");
+      /* Double fork in order to inherit the grandchild. The process
+	 is expected to exit when the other end of the socketpair is
+	 closed.
+       */
+      grandchild = fork ();
+      if (grandchild < 0)
+	perror_with_name ("Can't double fork command process");
+      if (grandchild != 0)
+	exit (0);
+      execl ("/bin/sh", "sh", "-c", command, NULL);
+      perror_with_name ("Can't exec command");
+    }
+  else
+    {
+      close (sockets[1]);
+      signal (SIGPIPE, SIG_IGN);
+      while ((res = waitpid (child, &status, 0)) < 0 && errno == EINTR);
+      if (res < 0)
+	perror_with_name ("Can't wait child process");
+      if (!WIFEXITED (status) || WEXITSTATUS (status))
+	perror_with_name ("Child process did not terminate correctly");
+      return sockets[0];
+     }
+  return -1;
+}
+#endif
+
 /* Open a connection to a remote debugger.
    NAME is the filename used for communication.  */
 
@@ -288,6 +348,17 @@ remote_open (char *name)
 {
   char *port_str;
 
+#ifndef USE_WIN32API
+  if (name[0] == '|')
+    {
+      fprintf (stderr, "Remote debugging using shell command\n");
+      remote_desc = open_shell_command (name + 1);
+      enable_async_notification (remote_desc);
+      add_file_handler (remote_desc, handle_serial_event, NULL);
+      return;
+    }
+#endif
+
   port_str = strchr (name, ':');
 #ifdef USE_WIN32API
   if (port_str == NULL)
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index d2e20d9..9ed4049 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3036,7 +3036,8 @@ gdbserver_usage (FILE *stream)
 	   "\tgdbserver [OPTIONS] --attach COMM PID\n"
 	   "\tgdbserver [OPTIONS] --multi COMM\n"
 	   "\n"
-	   "COMM may either be a tty device (for serial debugging),\n"
+	   "COMM may be a tty device (for serial debugging),\n"
+	   "'|some shell command' to use stdin/stdout of a given shell command,\n"
 	   "HOST:PORT to listen for a TCP connection, or '-' or 'stdio' to use \n"
 	   "stdin/stdout of gdbserver.\n"
 	   "PROG is the executable program.  ARGS are arguments passed to inferior.\n"
-- 
2.1.4

[PATCH v2 2/2] Make gdbserver connect to SOCK_STREAM sockets

[Gabriel Corona]

In GDB:

    (gdb) target remote |socat STDIO UNIX-LISTEN:foo.sock

In gdbserver:

   $ gdbserver foo.sock ./foo

gdb/gdbserver/ChangeLog:

   * remote-utils.c: add support for connecting to a SOCK_STREAM socket
   * remote.c: add documentation about this feature
---
 gdb/gdbserver/remote-utils.c | 40 ++++++++++++++++++++++++++++++++++++++--
 gdb/gdbserver/server.c       |  1 +
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index f15f1ce..ef4d8d6 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -36,6 +36,9 @@
 #if HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
 #endif
+#if HAVE_SYS_UN_H
+#include <sys/un.h>
+#endif
 #if HAVE_NETDB_H
 #include <netdb.h>
 #endif
@@ -340,6 +343,31 @@ open_shell_command (char *command)
 }
 #endif
 
+#ifndef USE_WIN32API
+static int
+socket_open (char *name)
+{
+  int sock;
+  struct sockaddr_un addr;
+
+  if (strlen (name) >= sizeof (addr.sun_path))
+    return -1;
+  sock = socket (AF_UNIX, SOCK_STREAM, 0);
+  if (sock < 0)
+    return -1;
+
+  addr.sun_family = AF_UNIX;
+  strcpy (addr.sun_path, name);
+  if (connect (sock, (const struct sockaddr *) &addr,
+	       sizeof (struct sockaddr_un)) < 0)
+    {
+      close (sock);
+      return -1;
+    }
+  return sock;
+}
+#endif
+
 /* Open a connection to a remote debugger.
    NAME is the filename used for communication.  */
 
@@ -382,10 +410,18 @@ remote_open (char *name)
   else if (port_str == NULL)
     {
       struct stat statbuf;
+      int res;
 
-      if (stat (name, &statbuf) == 0
+      res = stat (name, &statbuf);
+      if (res == 0
 	  && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode)))
-	remote_desc = open (name, O_RDWR);
+	{
+	  remote_desc = open (name, O_RDWR);
+	}
+      else if (res == 0 && S_ISSOCK (statbuf.st_mode))
+	{
+	  remote_desc = socket_open (name);
+	}
       else
 	{
 	  errno = EINVAL;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 9ed4049..675f9a5 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3037,6 +3037,7 @@ gdbserver_usage (FILE *stream)
 	   "\tgdbserver [OPTIONS] --multi COMM\n"
 	   "\n"
 	   "COMM may be a tty device (for serial debugging),\n"
+	   "an existing pathname stream-oriented socket,\n"
 	   "'|some shell command' to use stdin/stdout of a given shell command,\n"
 	   "HOST:PORT to listen for a TCP connection, or '-' or 'stdio' to use \n"
 	   "stdin/stdout of gdbserver.\n"
-- 
2.1.4

Re: [PATCH v2 1/2] Use a shell command as a socket for gdbserver

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]

On 04 May 2015 23:54, Gabriel Corona wrote:
> +static int
> +open_shell_command (char *command)

const ?

> +{
> +  int sockets[2];
> +  pid_t child, grandchild, res;
> +  int status;
> +
> +  if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sockets) < 0)
> +    perror_with_name ("Can't get socketpair");
> +  child = fork ();

prefer a blank line above the fork assignment

> +      if (dup2 (sockets[1], 0) < 0 || dup2 (sockets[1], 1) < 0)
> +	perror_with_name ("Can't dup socket to stdio");

use STDIN_FILENO & STDOUT_FILENO instead of 0 & 1

> +      if (close (sockets[1]) < 0)
> +	perror_with_name ("Can't close original socket");

dup2 has an edge case where if the fd is the same as an existing one, it won't 
create a new one.  i think before you close, you need to compare it to 
STDIN_FILENO & STDOUT_FILENO and only close it if it doesn't match.

> +      /* Double fork in order to inherit the grandchild. The process
> +	 is expected to exit when the other end of the socketpair is
> +	 closed.
> +       */

prefer a blank line above this comment to space the code out.  also, GNU style 
says:
 - use two spaces after periods
 - cuddle the trailing */ rather than putting it on a new line by itself

> +  else
> +    {
> +      close (sockets[1]);

check return value ?

> +      signal (SIGPIPE, SIG_IGN);
> +      while ((res = waitpid (child, &status, 0)) < 0 && errno == EINTR);
> +      if (res < 0)
> +	perror_with_name ("Can't wait child process");

phrasing is awkward ... maybe insert "on" or "for the" in there ?

> +     }
> +  return -1;
> +}

i think GNU style says you should have a blank line above this last return
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/2] Make gdbserver connect to SOCK_STREAM sockets

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On 04 May 2015 23:54, Gabriel Corona wrote:
> +static int
> +socket_open (char *name)

const

> +  int sock;
> +  struct sockaddr_un addr;
> +
> +  if (strlen (name) >= sizeof (addr.sun_path))
> +    return -1;
> +  sock = socket (AF_UNIX, SOCK_STREAM, 0);

prefer a blank line before the socket()

> +  if (sock < 0)
> +    return -1;
> +
> +  addr.sun_family = AF_UNIX;
> +  strcpy (addr.sun_path, name);
> +  if (connect (sock, (const struct sockaddr *) &addr,
> +	       sizeof (struct sockaddr_un)) < 0)
> +    {
> +      close (sock);
> +      return -1;
> +    }
> +  return sock;
> +}

blank line above this last return statement
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]