From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14257 invoked by alias); 9 Oct 2018 18:02:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14247 invoked by uid 89); 9 Oct 2018 18:02:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Oct 2018 18:02:17 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0C0E18764F; Tue, 9 Oct 2018 18:02:16 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1C6BF600C1; Tue, 9 Oct 2018 18:02:14 +0000 (UTC) Subject: Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. To: John Darrington , gdb-patches@sourceware.org References: <20181009173257.11250-1-john@darrington.wattle.id.au> <20181009173257.11250-2-john@darrington.wattle.id.au> From: Pedro Alves Message-ID: Date: Tue, 09 Oct 2018 18:02:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181009173257.11250-2-john@darrington.wattle.id.au> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-10/txt/msg00230.txt.bz2 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 > #endif > +#if HAVE_SYS_UN_H > +#include > +#endif > #if HAVE_SYS_SOCKET_H > #include > #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