GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested.

  When invoking gdbserver, if the COMM parameter does not include a colon (:) and
is not the name of an existing character device, then a local (unix) domain
socket will be created with that name and gdbserver will listen for connections
on that.

    gdb/doc/
    * gdb.texinfo (Server): Describe connection over a local domain socket.

    gdb/gdbserver/
    * NEWS: Mention new feature.
    * configure.ac (AC_CHECK_HEADERS): Add sys/un.h.
    * remote-utils.c (remote_prepare): Create a local socket if requested.
    * remote-utils.c (remote_open):  Don't attempt to open a file if it's a socket.
    * remote-utils.c (handle_accept_event): Display the name of the socket on connection.
---
 gdb/NEWS                     |   4 ++
 gdb/doc/gdb.texinfo          |  26 +++++--
 gdb/gdbserver/configure.ac   |   2 +-
 gdb/gdbserver/remote-utils.c | 164 ++++++++++++++++++++++++++++++-------------
 4 files changed, 141 insertions(+), 55 deletions(-)
  

> From: John Darrington <john@darrington.wattle.id.au>
> Cc: John Darrington <john@darrington.wattle.id.au>
> Date: Tue,  9 Oct 2018 19:32:57 +0200
> 
> When invoking gdbserver, if the COMM parameter does not include a colon (:) and
> is not the name of an existing character device, then a local (unix) domain
> socket will be created with that name and gdbserver will listen for connections
> on that.
> 
>     gdb/doc/
>     * gdb.texinfo (Server): Describe connection over a local domain socket.
> 
>     gdb/gdbserver/
>     * NEWS: Mention new feature.
>     * configure.ac (AC_CHECK_HEADERS): Add sys/un.h.
>     * remote-utils.c (remote_prepare): Create a local socket if requested.
>     * remote-utils.c (remote_open):  Don't attempt to open a file if it's a socket.
>     * remote-utils.c (handle_accept_event): Display the name of the socket on connection.

Thanks.  The documentation parts are approved, but I think we should
say somewhere that this feature is only available on systems that
support Unix domain sockets.
  

On 10/09/2018 06:32 PM, John Darrington wrote:
> When invoking gdbserver, if the COMM parameter does not include a colon (:) and
> is not the name of an existing character device, then a local (unix) domain
> socket will be created with that name and gdbserver will listen for connections
> on that.

Is that "colon/no-colon" magic something that tools frequently do?
Off hand it doesn't seem like a good idea to me, because if you typo a
character device name or unix tcp host name, you end up creating a
unix socket.  I'd think an explicit option would be better.  E.g.,
reuse gdb's connection specs, i.e., a prefix, like:

 gdbserver unix:/tmp/some/path 

which can be extended just like gdb's:

 gdbserver tcp:host:port
 gdbserver tcp6:host:port

etc.

That would suggest extending parse_connection_spec to support "unix" specs.

> 
>     gdb/doc/
>     * gdb.texinfo (Server): Describe connection over a local domain socket.
> 
>     gdb/gdbserver/
>     * NEWS: Mention new feature.

NEWS is under gdb/, not gdb/gdbserver/.

>     * configure.ac (AC_CHECK_HEADERS): Add sys/un.h.
>     * remote-utils.c (remote_prepare): Create a local socket if requested.
>     * remote-utils.c (remote_open):  Don't attempt to open a file if it's a socket.
>     * remote-utils.c (handle_accept_event): Display the name of the socket on connection.

Don't repeat "* remote-utils.c" for the second and third functions.

Add "* configure: Regenerate.".


> ---
>  gdb/NEWS                     |   4 ++
>  gdb/doc/gdb.texinfo          |  26 +++++--
>  gdb/gdbserver/configure.ac   |   2 +-
>  gdb/gdbserver/remote-utils.c | 164 ++++++++++++++++++++++++++++++-------------
>  4 files changed, 141 insertions(+), 55 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 78d20713a8..95c5083d9b 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -15,6 +15,10 @@
>    can be passed using the '[ADDRESS]:PORT' notation, or the regular
>    'ADDRESS:PORT' method.
>  
> +* GDB and GDBserver now support local domain socket connections.  The
> +  name of a local domain socket may be provided instead of the
> +  [ADDRESS]:PORT notation.
> +
>  * DWARF index cache: GDB can now automatically save indices of DWARF
>    symbols on disk to speed up further loading of the same binaries.
>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index efd6dffb1e..44b319ebc7 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -21075,9 +21075,13 @@ syntax is:
>  target> gdbserver @var{comm} @var{program} [ @var{args} @dots{} ]
>  @end smallexample
>  
> -@var{comm} is either a device name (to use a serial line), or a TCP
> +@var{comm} is either an existing device name (to use a serial line), or a TCP
>  hostname and portnumber, or @code{-} or @code{stdio} to use
>  stdin/stdout of @code{gdbserver}.
> +If @var{comm} is none of the above, then a local domain socket
> +will be created with that name.
> +@cindex local socket
> +@cindex Unix domain socket
>  For example, to debug Emacs with the argument
>  @samp{foo.txt} and communicate with @value{GDBN} over the serial port
>  @file{/dev/com1}:
> @@ -21107,6 +21111,20 @@ conflicts with another service, @code{gdbserver} prints an error message
>  and exits.}  You must use the same port number with the host @value{GDBN}
>  @code{target remote} command.
>  
> +If the target and local machine are one and the same, then you can
> +use local domain socket instead of a TCP connection:
> +
> +@smallexample
> +target> gdbserver /tmp/local-socket emacs foo.txt
> +@end smallexample
> +
> +A local domain socket called @file{/tmp/local-socket} will be created
> +in the filesystem.
> +If there is already a local domain socket with this name it will be removed.
> +@smallexample
> +(gdb) target remote /tmp/local-socket
> +@end smallexample
> +
>  The @code{stdio} connection is useful when starting @code{gdbserver}
>  with ssh:
>  
> @@ -21155,10 +21173,10 @@ In case more than one copy of @var{program} is running, or @var{program}
>  has multiple threads, most versions of @code{pidof} support the
>  @code{-s} option to only return the first process ID.
>  
> -@subsubsection TCP port allocation lifecycle of @code{gdbserver}
> +@subsubsection Socket lifecycle of @code{gdbserver}
>  
>  This section applies only when @code{gdbserver} is run to listen on a TCP
> -port.
> +port or a unix domain socket.
>  
>  @code{gdbserver} normally terminates after all of its debugged processes have
>  terminated in @kbd{target remote} mode.  On the other hand, for @kbd{target
> @@ -21174,7 +21192,7 @@ Such reconnecting is useful for features like @ref{disconnected tracing}.  For
>  completeness, at most one @value{GDBN} can be connected at a time.
>  
>  @cindex @option{--once}, @code{gdbserver} option
> -By default, @code{gdbserver} keeps the listening TCP port open, so that
> +By default, @code{gdbserver} keeps the listening socket open, so that
>  subsequent connections are possible.  However, if you start @code{gdbserver}
>  with the @option{--once} option, it will stop listening for any further
>  connection attempts after connecting to the first @value{GDBN} session.  This
> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> index fa3ca53efd..de7e7d4103 100644
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -98,7 +98,7 @@ ACX_CONFIGURE_DIR(["../../libiberty"], ["build-libiberty-gdbserver"])
>  AC_CHECK_HEADERS(termios.h sys/reg.h string.h dnl
>  		 proc_service.h sys/procfs.h linux/elf.h dnl
>  		 fcntl.h signal.h sys/file.h dnl
> -		 sys/ioctl.h netinet/in.h sys/socket.h netdb.h dnl
> +		 sys/ioctl.h netinet/in.h sys/socket.h sys/un.h netdb.h dnl
>  		 netinet/tcp.h arpa/inet.h)
>  AC_FUNC_FORK
>  AC_CHECK_FUNCS(getauxval pread pwrite pread64 setns)
> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
> index 9199a9c7ad..00e3340156 100644
> --- a/gdb/gdbserver/remote-utils.c
> +++ b/gdb/gdbserver/remote-utils.c
> @@ -38,6 +38,9 @@
>  #if HAVE_NETINET_IN_H
>  #include <netinet/in.h>
>  #endif
> +#if HAVE_SYS_UN_H
> +#include <sys/un.h>
> +#endif
>  #if HAVE_SYS_SOCKET_H
>  #include <sys/socket.h>
>  #endif
> @@ -193,20 +196,35 @@ handle_accept_event (int err, gdb_client_data client_data)
>       descriptor open for add_file_handler to wait for a new connection.  */
>    delete_file_handler (listen_desc);
>  
> -  /* Convert IP address to string.  */
> -  char orig_host[GDB_NI_MAX_ADDR], orig_port[GDB_NI_MAX_PORT];
> -
> -  int r = getnameinfo ((struct sockaddr *) &sockaddr, len,
> -		       orig_host, sizeof (orig_host),
> -		       orig_port, sizeof (orig_port),
> -		       NI_NUMERICHOST | NI_NUMERICSERV);
> +  if (sockaddr.ss_family == AF_UNIX)
> +    {
> +      struct sockaddr_un su;
> +      socklen_t len;
> +      if (0 != getsockname (listen_desc, (struct sockaddr *) &su, &len))

We don't use that "constant on lhs style".  Put the 0 != on the rhs.

> +	{
> +	  perror (_("Could not obtain remote address"));
> +	}

Remove unnecessary curly braces.

>  
> -  if (r != 0)
> -    fprintf (stderr, _("Could not obtain remote address: %s\n"),
> -	     gai_strerror (r));
> +      fprintf (stderr, _("Remote debugging on local socket bound to %s\n"),
> +	       su.sun_path);
> +    }
>    else
> -    fprintf (stderr, _("Remote debugging from host %s, port %s\n"),
> -	     orig_host, orig_port);
> +    {
> +      /* Convert IP address to string.  */
> +      char orig_host[GDB_NI_MAX_ADDR], orig_port[GDB_NI_MAX_PORT];
> +
> +      int r = getnameinfo ((struct sockaddr *) &sockaddr, len,
> +			   orig_host, sizeof (orig_host),
> +			   orig_port, sizeof (orig_port),
> +			   NI_NUMERICHOST | NI_NUMERICSERV);
> +
> +      if (r != 0)
> +	fprintf (stderr, _("Could not obtain remote address: %s\n"),
> +		 gai_strerror (r));
> +      else
> +	fprintf (stderr, _("Remote debugging from host %s, port %s\n"),
> +		 orig_host, orig_port);
> +    }
>  
>    enable_async_notification (remote_desc);
>  
> @@ -250,6 +268,9 @@ remote_prepare (const char *name)
>    struct addrinfo hint;
>    struct addrinfo *ainfo;
>  
> +  struct sockaddr *addr;
> +  socklen_t addrlen;
> +
>    memset (&hint, 0, sizeof (hint));
>    /* Assume no prefix will be passed, therefore we should use
>       AF_UNSPEC.  */
> @@ -260,10 +281,17 @@ remote_prepare (const char *name)
>    parsed_connection_spec parsed
>      = parse_connection_spec_without_prefix (name, &hint);
>  
> +  struct stat statbuf;
> +  int stat_result = stat (name, &statbuf);
> +
>    if (parsed.port_str.empty ())
>      {
> -      cs.transport_is_reliable = 0;
> -      return;
> +      if (stat_result == 0
> +	  && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode)))
> +	{
> +	  cs.transport_is_reliable = 0;
> +	  return;
> +	}
>      }
>  
>  #ifdef USE_WIN32API
> @@ -276,47 +304,82 @@ remote_prepare (const char *name)
>      }
>  #endif
>  
> -  int r = getaddrinfo (parsed.host_str.c_str (), parsed.port_str.c_str (),
> -		       &hint, &ainfo);
> +  struct sockaddr_un unix_addr;
>  
> -  if (r != 0)
> -    error (_("%s: cannot resolve name: %s"), name, gai_strerror (r));
> +#ifndef UNIX_PATH_MAX
> +#define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path)
> +#endif
>  
> -  scoped_free_addrinfo freeaddrinfo (ainfo);
> +  if (parsed.port_str.empty ())
> +    {
> +      if (strlen (name) > UNIX_PATH_MAX - 1)
> +	{
> +	  error
> +	    (_("%s is too long.  Socket names may be no longer than %s bytes."),
> +	     name, pulongest (UNIX_PATH_MAX - 1));
> +	  return;
> +	}
> +      listen_desc = socket (AF_UNIX,  SOCK_STREAM,  0);
> +      if (listen_desc < 0)
> +	perror_with_name ("Can't open socket");
> +
> +      memset (&unix_addr, 0, sizeof (unix_addr));
> +      unix_addr.sun_family = AF_UNIX;
> +      strncpy (unix_addr.sun_path, parsed.host_str.c_str(), UNIX_PATH_MAX - 1);
> +      if (stat_result == 0
> +	  && (S_IFSOCK & statbuf.st_mode))
> +	unlink (name);
> +
> +      addr = (struct sockaddr *) &unix_addr;
> +      addrlen = sizeof (unix_addr);
> +    }
> +  else
> +    {
> +      struct addrinfo *iter;
>  
> -  struct addrinfo *iter;
> +      int r = getaddrinfo (parsed.host_str.c_str (),
> +			   parsed.port_str.c_str (),
> +			   &hint, &ainfo);
>  
> -  for (iter = ainfo; iter != NULL; iter = iter->ai_next)
> -    {
> -      listen_desc = gdb_socket_cloexec (iter->ai_family, iter->ai_socktype,
> -					iter->ai_protocol);
> +      if (r != 0)
> +	error (_("%s: cannot resolve name: %s"), name, gai_strerror (r));
>  
> -      if (listen_desc >= 0)
> -	break;
> -    }
> +      scoped_free_addrinfo freeaddrinfo (ainfo);
>  
> -  if (iter == NULL)
> -    perror_with_name ("Can't open socket");
> +      for (iter = ainfo; iter != NULL; iter = iter->ai_next)
> +	{
> +	  listen_desc = gdb_socket_cloexec (iter->ai_family, iter->ai_socktype,
> +					    iter->ai_protocol);
>  
> -  /* Allow rapid reuse of this port. */
> -  tmp = 1;
> -  setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
> -	      sizeof (tmp));
> +	  if (listen_desc >= 0)
> +	    break;
> +	}
>  
> -  switch (iter->ai_family)
> -    {
> -    case AF_INET:
> -      ((struct sockaddr_in *) iter->ai_addr)->sin_addr.s_addr = INADDR_ANY;
> -      break;
> -    case AF_INET6:
> -      ((struct sockaddr_in6 *) iter->ai_addr)->sin6_addr = in6addr_any;
> -      break;
> -    default:
> -      internal_error (__FILE__, __LINE__,
> -		      _("Invalid 'ai_family' %d\n"), iter->ai_family);
> +      if (iter == NULL)
> +	perror_with_name ("Can't open socket");
> +
> +      /* Allow rapid reuse of this port. */
> +      tmp = 1;
> +      setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
> +		  sizeof (tmp));
> +
> +      switch (iter->ai_family)
> +	{
> +	case AF_INET:
> +	  ((struct sockaddr_in *) iter->ai_addr)->sin_addr.s_addr = INADDR_ANY;
> +	  break;
> +	case AF_INET6:
> +	  ((struct sockaddr_in6 *) iter->ai_addr)->sin6_addr = in6addr_any;
> +	  break;
> +	default:
> +	  internal_error (__FILE__, __LINE__,
> +			  _("Invalid 'ai_family' %d\n"), iter->ai_family);
> +	}
> +      addr = iter->ai_addr;
> +      addrlen = iter->ai_addrlen;
>      }
>  
> -  if (bind (listen_desc, iter->ai_addr, iter->ai_addrlen) != 0)
> +  if (bind (listen_desc, addr, addrlen) != 0)
>      perror_with_name ("Can't bind address");
>  
>    if (listen (listen_desc, 1) != 0)
> @@ -334,6 +397,9 @@ remote_open (const char *name)
>    const char *port_str;
>  
>    port_str = strchr (name, ':');
> +  struct stat statbuf;
> +  int stat_result = stat (name, &statbuf);
> +
>  #ifdef USE_WIN32API
>    if (port_str == NULL)
>      error ("Only HOST:PORT is supported on this platform.");
> @@ -353,12 +419,10 @@ remote_open (const char *name)
>        add_file_handler (remote_desc, handle_serial_event, NULL);
>      }
>  #ifndef USE_WIN32API
> -  else if (port_str == NULL)
> +  else if (port_str == NULL && stat_result == 0
> +	   && 0 == (S_IFSOCK & statbuf.st_mode))

Constant on rhs.

>      {
> -      struct stat statbuf;
> -
> -      if (stat (name, &statbuf) == 0
> -	  && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode)))
> +      if (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode))
>  	remote_desc = open (name, O_RDWR);
>        else
>  	{
> 

Thanks,
Pedro Alves
  

On Tue, Oct 09, 2018 at 07:02:14PM +0100, Pedro Alves wrote:
     On 10/09/2018 06:32 PM, John Darrington wrote:
     > When invoking gdbserver, if the COMM parameter does not include a colon (:) and
     > is not the name of an existing character device, then a local (unix) domain
     > socket will be created with that name and gdbserver will listen for connections
     > on that.
     
     Is that "colon/no-colon" magic something that tools frequently do?

Not exactly.  Tools with which I'm familiar with work as follows:

:1234           Creates a unix domain socket on the local host called 1234
localhost:1234  Listens on TCP port 1234

which is the way I think gdb ought to work, but this would be
inconsistent with it's current behaviour and cause confusion if somebody
used an old version of gdb with a new version of gdbserver or
vici-versa.

J'
  

On 10/09/2018 07:41 PM, John Darrington wrote:
> On Tue, Oct 09, 2018 at 07:02:14PM +0100, Pedro Alves wrote:
>      On 10/09/2018 06:32 PM, John Darrington wrote:
>      > When invoking gdbserver, if the COMM parameter does not include a colon (:) and
>      > is not the name of an existing character device, then a local (unix) domain
>      > socket will be created with that name and gdbserver will listen for connections
>      > on that.
>      
>      Is that "colon/no-colon" magic something that tools frequently do?
> 
> Not exactly.  Tools with which I'm familiar with work as follows:
> 
> :1234           Creates a unix domain socket on the local host called 1234
> localhost:1234  Listens on TCP port 1234
> 
> which is the way I think gdb ought to work, but this would be
> inconsistent with it's current behaviour and cause confusion if somebody
> used an old version of gdb with a new version of gdbserver or
> vici-versa.

In that example you didn't even pass a path to a unix domain socket.
You let the tool create it, I suppose.  It doesn't feel like
apples to apples.

In those tools you know, how would you pass the path to the local
socket then?

Thanks,
Pedro Alves
  

On Tue, Oct 09, 2018 at 07:53:55PM +0100, Pedro Alves wrote:
     On 10/09/2018 07:41 PM, John Darrington wrote:
     > On Tue, Oct 09, 2018 at 07:02:14PM +0100, Pedro Alves wrote:
     >      On 10/09/2018 06:32 PM, John Darrington wrote:
     >      > When invoking gdbserver, if the COMM parameter does not include a colon (:) and
     >      > is not the name of an existing character device, then a local (unix) domain
     >      > socket will be created with that name and gdbserver will listen for connections
     >      > on that.
     >      
     >      Is that "colon/no-colon" magic something that tools frequently do?
     > 
     > Not exactly.  Tools with which I'm familiar with work as follows:
     > 
     > :1234           Creates a unix domain socket on the local host called 1234
     > localhost:1234  Listens on TCP port 1234
     > 
     > which is the way I think gdb ought to work, but this would be
     > inconsistent with it's current behaviour and cause confusion if somebody
     > used an old version of gdb with a new version of gdbserver or
     > vici-versa.
     
     In that example you didn't even pass a path to a unix domain socket.
     You let the tool create it, I suppose.  It doesn't feel like
     apples to apples.
     
     In those tools you know, how would you pass the path to the local
     socket then?
     

If you give no path, it'll refer to a socket in the current working
directory.  An example with a path would be  :/tmp/this/socket