* Gdbserver can listen on local domain sockets. @ 2018-10-09 17:33 John Darrington 2018-10-09 17:33 ` [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested John Darrington 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-09 17:33 UTC (permalink / raw) To: gdb-patches This is the complement to commit c1168a2f66553cd4730931cf59e3be8378a1a03f It allows gdbserver to use a local (unix) domain socket instead of a TCP socket. It probably doesn't have much practical use, except that it makes automated testing of gdb's remote target a little easier. ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 17:33 Gdbserver can listen on local domain sockets John Darrington @ 2018-10-09 17:33 ` John Darrington 2018-10-09 17:56 ` Eli Zaretskii 2018-10-09 18:02 ` Pedro Alves 0 siblings, 2 replies; 40+ messages in thread From: John Darrington @ 2018-10-09 17:33 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington 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(-) 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)) + { + perror (_("Could not obtain remote address")); + } - 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)) { - 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 { -- 2.11.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 17:33 ` [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested John Darrington @ 2018-10-09 17:56 ` Eli Zaretskii 2018-10-09 18:02 ` Pedro Alves 1 sibling, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-10-09 17:56 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches > 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. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 17:33 ` [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested John Darrington 2018-10-09 17:56 ` Eli Zaretskii @ 2018-10-09 18:02 ` Pedro Alves 2018-10-09 18:41 ` John Darrington 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington 1 sibling, 2 replies; 40+ messages in thread From: Pedro Alves @ 2018-10-09 18:02 UTC (permalink / raw) To: John Darrington, gdb-patches 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 18:02 ` Pedro Alves @ 2018-10-09 18:41 ` John Darrington 2018-10-09 18:53 ` Pedro Alves 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington 1 sibling, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-09 18:41 UTC (permalink / raw) To: Pedro Alves; +Cc: John Darrington, gdb-patches 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' ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 18:41 ` John Darrington @ 2018-10-09 18:53 ` Pedro Alves 2018-10-09 19:00 ` John Darrington 0 siblings, 1 reply; 40+ messages in thread From: Pedro Alves @ 2018-10-09 18:53 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 18:53 ` Pedro Alves @ 2018-10-09 19:00 ` John Darrington 2018-10-09 19:06 ` Pedro Alves 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-09 19:00 UTC (permalink / raw) To: Pedro Alves; +Cc: John Darrington, gdb-patches 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 19:00 ` John Darrington @ 2018-10-09 19:06 ` Pedro Alves 0 siblings, 0 replies; 40+ messages in thread From: Pedro Alves @ 2018-10-09 19:06 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches -- Thanks, Pedro Alves On 10/09/2018 08:00 PM, John Darrington wrote: > 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 Ah, I see now. Still, yeah, I don't think we can change GDB like that. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-09 18:02 ` Pedro Alves 2018-10-09 18:41 ` John Darrington @ 2018-10-13 17:58 ` John Darrington 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington ` (4 more replies) 1 sibling, 5 replies; 40+ messages in thread From: John Darrington @ 2018-10-13 17:58 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington When invoking gdbserver, if the COMM parameter takes the form "unix::/path/name" then a local (unix) domain socket will be created with that name and gdbserver will listen for connections on that. gdb/ * NEWS: Mention new feature. gdb/gdbserver/ * configure.ac (AC_CHECK_HEADERS): Add sys/un.h. * configure: Regenerate. * remote-utils.c (remote_prepare): Create a local socket if requested. (remote_open): Don't attempt to open a file if it's a socket. (handle_accept_event): Display the name of the socket on connection. gdb/common/ * netstuff.c (parse_connection_spec)[prefixes]: New member for local domain sockets. --- gdb/NEWS | 4 ++ gdb/common/netstuff.c | 9 +++ gdb/gdbserver/configure.ac | 2 +- gdb/gdbserver/remote-utils.c | 148 ++++++++++++++++++++++++++++++------------- 4 files changed, 117 insertions(+), 46 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 126e61e282..23de349324 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/common/netstuff.c b/gdb/common/netstuff.c index c1c401ccb2..8da6aba24e 100644 --- a/gdb/common/netstuff.c +++ b/gdb/common/netstuff.c @@ -57,6 +57,8 @@ parse_connection_spec_without_prefix (std::string spec, struct addrinfo *hint) || std::count (spec.begin (), spec.end (), ':') > 1))); + bool is_unix = hint->ai_family == AF_UNIX; + if (is_ipv6) { if (spec[0] == '[') @@ -109,6 +111,12 @@ parse_connection_spec_without_prefix (std::string spec, struct addrinfo *hint) if (ret.host_str.empty ()) ret.host_str = "localhost"; + if (is_unix && ret.host_str != "localhost") + error (_("The host name must be empty or 'localhost' for a unix domain socket.")); + + if (is_unix && ret.port_str.empty ()) + error (_("A path name must be specified for a unix domain socket.")); + return ret; } @@ -138,6 +146,7 @@ parse_connection_spec (const char *spec, struct addrinfo *hint) { "tcp4:", AF_INET, SOCK_STREAM }, { "udp6:", AF_INET6, SOCK_DGRAM }, { "tcp6:", AF_INET6, SOCK_STREAM }, + { "unix:", AF_LOCAL, SOCK_STREAM }, }; for (const host_prefix prefix : prefixes) 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..0eb53c2116 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,33 @@ 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 (getsockname (listen_desc, (struct sockaddr *) &su, &len) != 0) + perror (_("Could not obtain remote address")); - 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 +266,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. */ @@ -258,7 +277,7 @@ remote_prepare (const char *name) hint.ai_protocol = IPPROTO_TCP; parsed_connection_spec parsed - = parse_connection_spec_without_prefix (name, &hint); + = parse_connection_spec (name, &hint); if (parsed.port_str.empty ()) { @@ -276,47 +295,86 @@ 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 (hint.ai_family == AF_LOCAL) + { + const char *sock_name = parsed.port_str.c_str (); + if (parsed.port_str.length() > UNIX_PATH_MAX - 1) + { + error + (_("%s is too long. Socket names may be no longer than %s bytes."), + sock_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"); - struct addrinfo *iter; + memset (&unix_addr, 0, sizeof (unix_addr)); + unix_addr.sun_family = AF_UNIX; + strncpy (unix_addr.sun_path, sock_name, UNIX_PATH_MAX - 1); - for (iter = ainfo; iter != NULL; iter = iter->ai_next) - { - listen_desc = gdb_socket_cloexec (iter->ai_family, iter->ai_socktype, - iter->ai_protocol); + struct stat statbuf; + int stat_result = stat (sock_name, &statbuf); + if (stat_result == 0 + && (S_IFSOCK & statbuf.st_mode)) + unlink (sock_name); - if (listen_desc >= 0) - break; + addr = (struct sockaddr *) &unix_addr; + addrlen = sizeof (unix_addr); } + else + { + struct addrinfo *iter; - if (iter == NULL) - perror_with_name ("Can't open socket"); + int r = getaddrinfo (parsed.host_str.c_str (), + parsed.port_str.c_str (), + &hint, &ainfo); - /* Allow rapid reuse of this port. */ - tmp = 1; - setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp, - sizeof (tmp)); + if (r != 0) + error (_("%s: cannot resolve name: %s"), name, gai_strerror (r)); - 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); + scoped_free_addrinfo freeaddrinfo (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 (listen_desc >= 0) + break; + } + + 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) @@ -354,11 +412,11 @@ remote_open (const char *name) } #ifndef USE_WIN32API else if (port_str == NULL) - { + { struct stat statbuf; if (stat (name, &statbuf) == 0 - && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode))) + && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode))) remote_desc = open (name, O_RDWR); else { -- 2.11.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington @ 2018-10-13 17:58 ` John Darrington 2018-10-13 18:10 ` Eli Zaretskii 2018-10-18 20:27 ` Sergio Durigan Junior 2018-10-13 17:58 ` [PATCH 4/4] GDB: Remote target can now accept the form unix::/path/to/socket John Darrington ` (3 subsequent siblings) 4 siblings, 2 replies; 40+ messages in thread From: John Darrington @ 2018-10-13 17:58 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington The documentation did not mention the possibility of invoking gdbserver with the new connection forms such as tcp6:host:port. This change fixes that. gdb/doc/ * gdb.texinfo (Server): Tabulate the various permitted forms of the @var{comm} metasyntactical variable. Include the unix:@var{host}:@var{socket} form as one of them. --- gdb/doc/gdb.texinfo | 60 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 1e97d692b6..e3b62221cf 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -21097,9 +21097,19 @@ 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 -hostname and portnumber, or @code{-} or @code{stdio} to use -stdin/stdout of @code{gdbserver}. +@code{gdbserver} waits passively for the host @value{GDBN} to communicate +with it. + +@var{comm} may take several forms: + +@table @code +@item @code{@var{device}} +A serial line device. + +@item @code{-} +@itemx @code{stdio} +To use the stdin/stdout of @code{gdbserver}. + For example, to debug Emacs with the argument @samp{foo.txt} and communicate with @value{GDBN} over the serial port @file{/dev/com1}: @@ -21108,8 +21118,27 @@ For example, to debug Emacs with the argument target> gdbserver /dev/com1 emacs foo.txt @end smallexample -@code{gdbserver} waits passively for the host @value{GDBN} to communicate -with it. +The @code{stdio} connection is useful when starting @code{gdbserver} +with ssh: + +@smallexample +(gdb) target remote | ssh -T hostname gdbserver - hello +@end smallexample + +The @samp{-T} option to ssh is provided because we don't need a remote pty, +and we don't want escape-character handling. Ssh does this by default when +a command is provided, the flag is provided to make it explicit. +You could elide it if you want to. + +Programs started with stdio-connected gdbserver have @file{/dev/null} for +@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for +display through a pipe connected to gdbserver. +Both @code{stdout} and @code{stderr} use the same pipe. + +@item @code{@var{host}:@var{port}} +@itemx @code{tcp:@var{host}:@var{port}} +@itemx @code{tcp4:@var{host}:@var{port}} +To use a @acronym{TCP} @acronym{IPv4} socket connection on port number @var{port}. To use a TCP connection instead of a serial line: @@ -21129,22 +21158,21 @@ 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. -The @code{stdio} connection is useful when starting @code{gdbserver} -with ssh: + +@item @code{tcp6:@var{host}:@var{port}} +To use a @acronym{TCP} @acronym{IPv6} socket connection on port number @var{port}. + +@item @code{unix:@var{host}:@var{local-socket}} +To use a Unix domain socket. This will create a socket with the file +system entry @var{local-socket} and listen on that. For example: @smallexample -(gdb) target remote | ssh -T hostname gdbserver - hello +target> gdbserver unix:localhost:/tmp/gdb-socket0 emacs foo.txt @end smallexample -The @samp{-T} option to ssh is provided because we don't need a remote pty, -and we don't want escape-character handling. Ssh does this by default when -a command is provided, the flag is provided to make it explicit. -You could elide it if you want to. +@var{host} must either be the null string or the literal string @code{localhost}. +@end table -Programs started with stdio-connected gdbserver have @file{/dev/null} for -@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for -display through a pipe connected to gdbserver. -Both @code{stdout} and @code{stderr} use the same pipe. @anchor{Attaching to a program} @subsubsection Attaching to a Running Program -- 2.11.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington @ 2018-10-13 18:10 ` Eli Zaretskii 2018-10-18 20:27 ` Sergio Durigan Junior 1 sibling, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-10-13 18:10 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches > From: John Darrington <john@darrington.wattle.id.au> > Cc: John Darrington <john@darrington.wattle.id.au> > Date: Sat, 13 Oct 2018 19:58:00 +0200 > > The documentation did not mention the possibility of invoking gdbserver > with the new connection forms such as tcp6:host:port. This change fixes > that. Thanks. Please see a few comments below. > +@table @code > +@item @code{@var{device}} > +A serial line device. Since your @table uses @code by default, there's no need to use @code in each @item of the table. > +@var{host} must either be the null string or the literal string @code{localhost}. What is a "null string"? Do you mean an empty string? Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington 2018-10-13 18:10 ` Eli Zaretskii @ 2018-10-18 20:27 ` Sergio Durigan Junior 2018-10-19 7:05 ` John Darrington 1 sibling, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-18 20:27 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Saturday, October 13 2018, John Darrington wrote: > The documentation did not mention the possibility of invoking gdbserver > with the new connection forms such as tcp6:host:port. This change fixes > that. Thanks for the patch. As I mentioned while reviewing patch #1, there is a reason why gdbserver doesn't accept prefixes (which means that there isn't a mistake in the docs). If you go with my suggestion and implement just the support for the "unix:" prefix, then this patch will need to be changed because it still won't be correct to mention the "tcp" prefixes. > gdb/doc/ > > * gdb.texinfo (Server): Tabulate the various permitted forms of the @var{comm} > metasyntactical variable. Include the unix:@var{host}:@var{socket} form as > one of them. > --- > gdb/doc/gdb.texinfo | 60 +++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 44 insertions(+), 16 deletions(-) > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 1e97d692b6..e3b62221cf 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -21097,9 +21097,19 @@ 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 > -hostname and portnumber, or @code{-} or @code{stdio} to use > -stdin/stdout of @code{gdbserver}. > +@code{gdbserver} waits passively for the host @value{GDBN} to communicate > +with it. > + > +@var{comm} may take several forms: > + > +@table @code > +@item @code{@var{device}} > +A serial line device. > + > +@item @code{-} > +@itemx @code{stdio} > +To use the stdin/stdout of @code{gdbserver}. > + > For example, to debug Emacs with the argument > @samp{foo.txt} and communicate with @value{GDBN} over the serial port > @file{/dev/com1}: > @@ -21108,8 +21118,27 @@ For example, to debug Emacs with the argument > target> gdbserver /dev/com1 emacs foo.txt > @end smallexample > > -@code{gdbserver} waits passively for the host @value{GDBN} to communicate > -with it. > +The @code{stdio} connection is useful when starting @code{gdbserver} > +with ssh: > + > +@smallexample > +(gdb) target remote | ssh -T hostname gdbserver - hello > +@end smallexample > + > +The @samp{-T} option to ssh is provided because we don't need a remote pty, > +and we don't want escape-character handling. Ssh does this by default when > +a command is provided, the flag is provided to make it explicit. > +You could elide it if you want to. > + > +Programs started with stdio-connected gdbserver have @file{/dev/null} for > +@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for > +display through a pipe connected to gdbserver. > +Both @code{stdout} and @code{stderr} use the same pipe. > + > +@item @code{@var{host}:@var{port}} > +@itemx @code{tcp:@var{host}:@var{port}} > +@itemx @code{tcp4:@var{host}:@var{port}} > +To use a @acronym{TCP} @acronym{IPv4} socket connection on port number @var{port}. > > To use a TCP connection instead of a serial line: > > @@ -21129,22 +21158,21 @@ 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. > > -The @code{stdio} connection is useful when starting @code{gdbserver} > -with ssh: > + > +@item @code{tcp6:@var{host}:@var{port}} > +To use a @acronym{TCP} @acronym{IPv6} socket connection on port number @var{port}. > + > +@item @code{unix:@var{host}:@var{local-socket}} > +To use a Unix domain socket. This will create a socket with the file > +system entry @var{local-socket} and listen on that. For example: > > @smallexample > -(gdb) target remote | ssh -T hostname gdbserver - hello > +target> gdbserver unix:localhost:/tmp/gdb-socket0 emacs foo.txt > @end smallexample > > -The @samp{-T} option to ssh is provided because we don't need a remote pty, > -and we don't want escape-character handling. Ssh does this by default when > -a command is provided, the flag is provided to make it explicit. > -You could elide it if you want to. > +@var{host} must either be the null string or the literal string @code{localhost}. > +@end table > > -Programs started with stdio-connected gdbserver have @file{/dev/null} for > -@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for > -display through a pipe connected to gdbserver. > -Both @code{stdout} and @code{stderr} use the same pipe. > > @anchor{Attaching to a program} > @subsubsection Attaching to a Running Program > -- > 2.11.0 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-18 20:27 ` Sergio Durigan Junior @ 2018-10-19 7:05 ` John Darrington 2018-10-19 20:45 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-19 7:05 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: John Darrington, gdb-patches On Thu, Oct 18, 2018 at 04:27:24PM -0400, Sergio Durigan Junior wrote: On Saturday, October 13 2018, John Darrington wrote: > The documentation did not mention the possibility of invoking gdbserver > with the new connection forms such as tcp6:host:port. This change fixes > that. Thanks for the patch. As I mentioned while reviewing patch #1, there is a reason why gdbserver doesn't accept prefixes (which means that there isn't a mistake in the docs). If you go with my suggestion and implement just the support for the "unix:" prefix, then this patch will need to be changed because it still won't be correct to mention the "tcp" prefixes. Thanks for the reviews. How about instead, we just add a test to print an error/warning if someone tries to start gdbserver with a udp prefix? J' -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-19 7:05 ` John Darrington @ 2018-10-19 20:45 ` Sergio Durigan Junior 2018-10-21 7:33 ` John Darrington 2018-10-23 18:25 ` John Darrington 0 siblings, 2 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-19 20:45 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Friday, October 19 2018, John Darrington wrote: > On Thu, Oct 18, 2018 at 04:27:24PM -0400, Sergio Durigan Junior wrote: > On Saturday, October 13 2018, John Darrington wrote: > > > The documentation did not mention the possibility of invoking gdbserver > > with the new connection forms such as tcp6:host:port. This change fixes > > that. > > Thanks for the patch. > > As I mentioned while reviewing patch #1, there is a reason why gdbserver > doesn't accept prefixes (which means that there isn't a mistake in the > docs). If you go with my suggestion and implement just the support for > the "unix:" prefix, then this patch will need to be changed because it > still won't be correct to mention the "tcp" prefixes. > > Thanks for the reviews. > > How about instead, we just add a test to print an error/warning if > someone tries to start gdbserver with a udp prefix? Hm, I thought about that while writing the email, but at the time it didn't seem like a good option. However, thinking about it again, I think it would be mostly fine. I guess it's OK to support the "tcp{,6}:" prefixes. The check needs to implemented on gdbserver/remote-utils.c:remote_open, and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP". And "parse_connection_spec_without_prefix" could be merged with "parse_connection_spec". Aside from the documentation, you'd also probably have to update the tests (both the selftests and the regular ones). Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-19 20:45 ` Sergio Durigan Junior @ 2018-10-21 7:33 ` John Darrington 2018-10-21 16:47 ` Sergio Durigan Junior 2018-10-23 18:25 ` John Darrington 1 sibling, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-21 7:33 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: John Darrington, gdb-patches On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote: The check needs to implemented on gdbserver/remote-utils.c:remote_open, and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP". And "parse_connection_spec_without_prefix" could be merged with "parse_connection_spec". A quick test shows that this is not necessary. Any attempt to start gdbserver with udp: will never get to remote_open because remote_prepare will fail when it gets to listen: "Can't listen on socket: Operation no supported." (listen is for connection orientated protocols only). J' -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-21 7:33 ` John Darrington @ 2018-10-21 16:47 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-21 16:47 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Sunday, October 21 2018, John Darrington wrote: > On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote: > > The check needs to implemented on gdbserver/remote-utils.c:remote_open, > and gdbserver needs to error out if "hint.ai_protocol == IPPROTO_UDP". > And "parse_connection_spec_without_prefix" could be merged with > "parse_connection_spec". > > A quick test shows that this is not necessary. Any attempt to start > gdbserver with udp: will never get to remote_open because remote_prepare > will fail when it gets to listen: > > "Can't listen on socket: Operation no supported." > > (listen is for connection orientated protocols only). Yes, but this error message is not entirely clear. We should show a more user-friendly message, and the best way to do that is to do an early check for the UDP protocol. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-19 20:45 ` Sergio Durigan Junior 2018-10-21 7:33 ` John Darrington @ 2018-10-23 18:25 ` John Darrington 2018-10-23 18:58 ` Sergio Durigan Junior 1 sibling, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-23 18:25 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: John Darrington, gdb-patches On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote: Aside from the documentation, you'd also probably have to update the tests (both the selftests and the regular ones). When I run "make check" I get a large number of failures, and I'm told that this is normal. How do I run just the tests which you think need to be adjusted? J' ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER 2018-10-23 18:25 ` John Darrington @ 2018-10-23 18:58 ` Sergio Durigan Junior 0 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-23 18:58 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Tuesday, October 23 2018, John Darrington wrote: > On Fri, Oct 19, 2018 at 04:45:14PM -0400, Sergio Durigan Junior wrote: > > Aside from the documentation, you'd also probably have to update the > tests (both the selftests and the regular ones). > > > When I run "make check" I get a large number of failures, and I'm told > that this is normal. How do I run just the tests which you think need > to be adjusted? You can take a look at the tests I touched when implementing IPv6: commit c7ab0aef11d91b637bf091aa9176b8dc4aadee46 Author: Sergio Durigan Junior <sergiodj@redhat.com> Date: Fri May 18 01:29:24 2018 -0400 Implement IPv6 support for GDB/gdbserver Specifically gdb.server/server-connect.exp. Offhand I'm not sure if any other test needs to be adjusted; probably not. I'd advise taking a look under the gdb.server/ directory just to double check. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 4/4] GDB: Remote target can now accept the form unix::/path/to/socket. 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington @ 2018-10-13 17:58 ` John Darrington 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington ` (2 subsequent siblings) 4 siblings, 0 replies; 40+ messages in thread From: John Darrington @ 2018-10-13 17:58 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington Allow target remote to use the unix::/path/to/socket syntax as well as just plain /path/to/socket gdb/ * ser-uds.c (uds_open): Use parse_connection_spec to deal with the comm form unix::/path/to/socket. * serial.c (serial_open): Consider the "unix:" prefix when deciding which interface to use. --- gdb/ser-uds.c | 18 ++++++++++++++++-- gdb/serial.c | 5 +++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/gdb/ser-uds.c b/gdb/ser-uds.c index a98469f67b..acace258be 100644 --- a/gdb/ser-uds.c +++ b/gdb/ser-uds.c @@ -23,6 +23,8 @@ #include <sys/socket.h> #include <sys/un.h> +#include <netdb.h> +#include "netstuff.h" #ifndef UNIX_PATH_MAX #define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path) @@ -33,9 +35,21 @@ static int uds_open (struct serial *scb, const char *name) { + struct addrinfo hint; + + memset (&hint, 0, sizeof (hint)); + /* Assume no prefix will be passed, therefore we should use + AF_UNSPEC. */ + hint.ai_family = AF_UNSPEC; + hint.ai_socktype = SOCK_STREAM; + + parsed_connection_spec parsed = parse_connection_spec (name, &hint); + + const char *socket_name = parsed.port_str.empty() ? name : parsed.port_str.c_str (); + struct sockaddr_un addr; - if (strlen (name) > UNIX_PATH_MAX - 1) + if (strlen (socket_name) > UNIX_PATH_MAX - 1) { warning (_("The socket name is too long. It may be no longer than %s bytes."), @@ -45,7 +59,7 @@ uds_open (struct serial *scb, const char *name) memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, name, UNIX_PATH_MAX - 1); + strncpy (addr.sun_path, socket_name, UNIX_PATH_MAX - 1); int sock = socket (AF_UNIX, SOCK_STREAM, 0); diff --git a/gdb/serial.c b/gdb/serial.c index 7f9362a3bf..f7c3e6e5ee 100644 --- a/gdb/serial.c +++ b/gdb/serial.c @@ -210,7 +210,7 @@ serial_open (const char *name) /* Check for a colon, suggesting an IP address/port pair. Do this *after* checking for all the interesting prefixes. We don't want to constrain the syntax of what can follow them. */ - else if (strchr (name, ':')) + else if (!startswith (name, "unix:") && (strchr (name, ':'))) ops = serial_interface_lookup ("tcp"); else { @@ -218,7 +218,8 @@ serial_open (const char *name) /* Check to see if name is a socket. If it is, then treat it as such. Otherwise assume that it's a character device. */ struct stat sb; - if (stat (name, &sb) == 0 && (sb.st_mode & S_IFMT) == S_IFSOCK) + if (startswith (name, "unix:") || + (stat (name, &sb) == 0 && (sb.st_mode & S_IFMT) == S_IFSOCK)) ops = serial_interface_lookup ("local"); else #endif -- 2.11.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection. 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington 2018-10-13 17:58 ` [PATCH 4/4] GDB: Remote target can now accept the form unix::/path/to/socket John Darrington @ 2018-10-13 17:58 ` John Darrington 2018-10-13 18:11 ` Eli Zaretskii ` (2 more replies) 2018-10-13 18:12 ` [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested Eli Zaretskii 2018-10-18 20:18 ` Sergio Durigan Junior 4 siblings, 3 replies; 40+ messages in thread From: John Darrington @ 2018-10-13 17:58 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington gdb/doc: * gdb.texinfo (Connecting)[Remote Connection Commands]: Provide alternative unix::/tmp/xxx example. Include @code{unix::@var{local-socket} in the list of remote and extended-remote syntaxes. --- gdb/doc/gdb.texinfo | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index b0dc3bf67c..1e97d692b6 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20829,6 +20829,15 @@ Note that this command has the same form as the command to connect to a serial line. @value{GDBN} will automatically determine which kind of file you have specified and will make the appropriate kind of connection. +The above command is identical to the command: + +@smallexample +target remote unix::/tmp/gdb-socket1 +@end smallexample +@noindent + +See below for the explanation of this syntax. + This feature is not available if the host system does not support Unix domain sockets. @@ -20839,6 +20848,7 @@ Unix domain sockets. @itemx target remote @code{tcp4:@var{host}:@var{port}} @itemx target remote @code{tcp6:@var{host}:@var{port}} @itemx target remote @code{tcp6:@var{[host]}:@var{port}} +@itemx target remote @code{unix::@var{local-socket}} @itemx target extended-remote @code{@var{host}:@var{port}} @itemx target extended-remote @code{@var{[host]}:@var{port}} @itemx target extended-remote @code{tcp:@var{host}:@var{port}} @@ -20846,8 +20856,10 @@ Unix domain sockets. @itemx target extended-remote @code{tcp4:@var{host}:@var{port}} @itemx target extended-remote @code{tcp6:@var{host}:@var{port}} @itemx target extended-remote @code{tcp6:@var{[host]}:@var{port}} +@itemx target extended-remote @code{unix::@var{local-socket}} @cindex @acronym{TCP} port, @code{target remote} -Debug using a @acronym{TCP} connection to @var{port} on @var{host}. +Debug using a @acronym{TCP} connection to @var{port} on @var{host} +or using the Unix domain socket @var{local-socket} on the local machine. The @var{host} may be either a host name, a numeric @acronym{IPv4} address, or a numeric @acronym{IPv6} address (with or without the square brackets to separate the address from the port); @var{port} @@ -20895,6 +20907,16 @@ target remote :1234 @noindent Note that the colon is still required here. +Alternatively you can use a Unix domain socket: + +@smallexample +target remote unix::/tmp/gdb-socket1 +@end smallexample +@noindent + +This has the advantage that it'll not fail if the port number is already +in use. + @item target remote @code{udp:@var{host}:@var{port}} @itemx target remote @code{udp:@var{[host]}:@var{port}} -- 2.11.0 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection. 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington @ 2018-10-13 18:11 ` Eli Zaretskii 2018-10-15 9:31 ` Simon Tatham 2018-10-18 20:21 ` Sergio Durigan Junior 2 siblings, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-10-13 18:11 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches > From: John Darrington <john@darrington.wattle.id.au> > Cc: John Darrington <john@darrington.wattle.id.au> > Date: Sat, 13 Oct 2018 19:57:59 +0200 > > gdb/doc: > * gdb.texinfo (Connecting)[Remote Connection Commands]: Provide alternative > unix::/tmp/xxx example. Include @code{unix::@var{local-socket} in > the list of remote and extended-remote syntaxes. OK. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection. 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington 2018-10-13 18:11 ` Eli Zaretskii @ 2018-10-15 9:31 ` Simon Tatham 2018-10-15 11:28 ` John Darrington 2018-10-18 20:21 ` Sergio Durigan Junior 2 siblings, 1 reply; 40+ messages in thread From: Simon Tatham @ 2018-10-15 9:31 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches, nd On 13/10/18 18:57, John Darrington wrote: > +Alternatively you can use a Unix domain socket: > + > +@smallexample > +target remote unix::/tmp/gdb-socket1 > +@end smallexample > +@noindent > + > +This has the advantage that it'll not fail if the port number is already > +in use. > + Hi, I was pointed at this thread by a colleague, because last week I was also considering submitting a patch to allow gdb and gdbserver to talk to each other over Unix-domain sockets, and he pointed out that it was already in progress :-) I'd like to suggest that this documentation change under-stresses what I see as the most important reason why this is a useful feature: security. The gdbserver protocol is cleartext and unauthenticated. Running it on a TCP port means that anyone who can connect to that port â and depending on the network environment, that might be a lot of people â can request gdbserver to execute arbitrary code in the context of the process being debugged, without having to give a vestige of proof as to their right to ask for it. This is not really the kind of feature we like about network protocols in the modern world! But Unix-domain sockets are access-controlled via the file permissions on the path leading to the socket file. If you use this new feature to make a Unix-domain socket inside a directory that only your user id has access to, then any process physically capable of connecting to the socket has already proved its right to run code under your user id. So this solves the whole issue, while keeping all the other conveniences of the socket-based gdbserver transport. Also, current versions of OpenSSH support general forwarding of Unix-domain sockets over SSH connections. Using that, it should even be possible to use this system for genuinely _remote_ debugging (i.e. between different machines), without reintroducing the same security hazard, because the Unix socket at each end is access-controlled, and the network connection in between them is cryptographically protected. Cheers, Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection. 2018-10-15 9:31 ` Simon Tatham @ 2018-10-15 11:28 ` John Darrington 0 siblings, 0 replies; 40+ messages in thread From: John Darrington @ 2018-10-15 11:28 UTC (permalink / raw) To: Simon Tatham; +Cc: John Darrington, gdb-patches, nd On Mon, Oct 15, 2018 at 10:31:01AM +0100, Simon Tatham wrote: Hi, I was pointed at this thread by a colleague, because last week I was also considering submitting a patch to allow gdb and gdbserver to talk to each other over Unix-domain sockets, and he pointed out that it was already in progress :-) I'd like to suggest that this documentation change under-stresses what I see as the most important reason why this is a useful feature: security. The gdbserver protocol is cleartext and unauthenticated. Running it on a TCP port means that anyone who can connect to that port ??? and depending on the network environment, that might be a lot of people ??? can request gdbserver to execute arbitrary code in the context of the process being debugged, without having to give a vestige of proof as to their right to ask for it. This is not really the kind of feature we like about network protocols in the modern world! But Unix-domain sockets are access-controlled via the file permissions on the path leading to the socket file. If you use this new feature to make a Unix-domain socket inside a directory that only your user id has access to, then any process physically capable of connecting to the socket has already proved its right to run code under your user id. So this solves the whole issue, while keeping all the other conveniences of the socket-based gdbserver transport. Cheers, Simon This is a good point. But really it belongs under the heading of the risks associated with TCP/IP sockets - not the risk which is absent when using Unix sockets. The documentation already has this warning: _Warning:_ 'gdbserver' does not have any built-in security. Do not run 'gdbserver' connected to any public network; a GDB connection to 'gdbserver' provides access to the target system with the same privileges as the user running 'gdbserver'. ... perhaps that could be expanded to discuss the relative merits of UDS vs. TCP/IP J' -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection. 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington 2018-10-13 18:11 ` Eli Zaretskii 2018-10-15 9:31 ` Simon Tatham @ 2018-10-18 20:21 ` Sergio Durigan Junior 2 siblings, 0 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-18 20:21 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Saturday, October 13 2018, John Darrington wrote: > gdb/doc: > * gdb.texinfo (Connecting)[Remote Connection Commands]: Provide alternative > unix::/tmp/xxx example. Include @code{unix::@var{local-socket} in ^ Missing close-curly bracket. > the list of remote and extended-remote syntaxes. > --- > gdb/doc/gdb.texinfo | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index b0dc3bf67c..1e97d692b6 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -20829,6 +20829,15 @@ Note that this command has the same form as the command to connect > to a serial line. @value{GDBN} will automatically determine which > kind of file you have specified and will make the appropriate kind > of connection. > +The above command is identical to the command: > + > +@smallexample > +target remote unix::/tmp/gdb-socket1 > +@end smallexample > +@noindent > + > +See below for the explanation of this syntax. > + > This feature is not available if the host system does not support > Unix domain sockets. > > @@ -20839,6 +20848,7 @@ Unix domain sockets. > @itemx target remote @code{tcp4:@var{host}:@var{port}} > @itemx target remote @code{tcp6:@var{host}:@var{port}} > @itemx target remote @code{tcp6:@var{[host]}:@var{port}} > +@itemx target remote @code{unix::@var{local-socket}} > @itemx target extended-remote @code{@var{host}:@var{port}} > @itemx target extended-remote @code{@var{[host]}:@var{port}} > @itemx target extended-remote @code{tcp:@var{host}:@var{port}} > @@ -20846,8 +20856,10 @@ Unix domain sockets. > @itemx target extended-remote @code{tcp4:@var{host}:@var{port}} > @itemx target extended-remote @code{tcp6:@var{host}:@var{port}} > @itemx target extended-remote @code{tcp6:@var{[host]}:@var{port}} > +@itemx target extended-remote @code{unix::@var{local-socket}} > @cindex @acronym{TCP} port, @code{target remote} > -Debug using a @acronym{TCP} connection to @var{port} on @var{host}. > +Debug using a @acronym{TCP} connection to @var{port} on @var{host} > +or using the Unix domain socket @var{local-socket} on the local machine. > The @var{host} may be either a host name, a numeric @acronym{IPv4} > address, or a numeric @acronym{IPv6} address (with or without the > square brackets to separate the address from the port); @var{port} > @@ -20895,6 +20907,16 @@ target remote :1234 > @noindent > > Note that the colon is still required here. > +Alternatively you can use a Unix domain socket: > + > +@smallexample > +target remote unix::/tmp/gdb-socket1 > +@end smallexample > +@noindent > + > +This has the advantage that it'll not fail if the port number is already > +in use. > + > > @item target remote @code{udp:@var{host}:@var{port}} > @itemx target remote @code{udp:@var{[host]}:@var{port}} > -- > 2.11.0 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington ` (2 preceding siblings ...) 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington @ 2018-10-13 18:12 ` Eli Zaretskii 2018-10-18 20:18 ` Sergio Durigan Junior 4 siblings, 0 replies; 40+ messages in thread From: Eli Zaretskii @ 2018-10-13 18:12 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches > From: John Darrington <john@darrington.wattle.id.au> > Cc: John Darrington <john@darrington.wattle.id.au> > Date: Sat, 13 Oct 2018 19:57:58 +0200 > > When invoking gdbserver, if the COMM parameter takes the form "unix::/path/name" > then a local (unix) domain socket will be created with that name and gdbserver > will listen for connections on that. > > gdb/ > * NEWS: Mention new feature. > > gdb/gdbserver/ > * configure.ac (AC_CHECK_HEADERS): Add sys/un.h. > * configure: Regenerate. > * remote-utils.c (remote_prepare): Create a local socket if requested. > (remote_open): Don't attempt to open a file if it's a socket. > (handle_accept_event): Display the name of the socket on connection. > > gdb/common/ > * netstuff.c (parse_connection_spec)[prefixes]: New member for local domain sockets. OK for the NEWS part. Thanks. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington ` (3 preceding siblings ...) 2018-10-13 18:12 ` [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested Eli Zaretskii @ 2018-10-18 20:18 ` Sergio Durigan Junior 2018-10-19 18:55 ` John Darrington 4 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-18 20:18 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Saturday, October 13 2018, John Darrington wrote: > When invoking gdbserver, if the COMM parameter takes the form "unix::/path/name" > then a local (unix) domain socket will be created with that name and gdbserver > will listen for connections on that. Thanks for the patch. > gdb/ > * NEWS: Mention new feature. > > gdb/gdbserver/ > * configure.ac (AC_CHECK_HEADERS): Add sys/un.h. > * configure: Regenerate. > * remote-utils.c (remote_prepare): Create a local socket if requested. > (remote_open): Don't attempt to open a file if it's a socket. > (handle_accept_event): Display the name of the socket on connection. > > gdb/common/ > * netstuff.c (parse_connection_spec)[prefixes]: New member for local domain sockets. > --- > gdb/NEWS | 4 ++ > gdb/common/netstuff.c | 9 +++ > gdb/gdbserver/configure.ac | 2 +- > gdb/gdbserver/remote-utils.c | 148 ++++++++++++++++++++++++++++++------------- > 4 files changed, 117 insertions(+), 46 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index 126e61e282..23de349324 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. Is it worth explicitly mentioning "UNIX domain socket"? > + > * 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/common/netstuff.c b/gdb/common/netstuff.c > index c1c401ccb2..8da6aba24e 100644 > --- a/gdb/common/netstuff.c > +++ b/gdb/common/netstuff.c > @@ -57,6 +57,8 @@ parse_connection_spec_without_prefix (std::string spec, struct addrinfo *hint) > || std::count (spec.begin (), > spec.end (), ':') > 1))); > > + bool is_unix = hint->ai_family == AF_UNIX; No need for a newline between the declarations of is_ipv6 and is_unix. Here, and everywhere else, AF_UNIX may be undefined if building GDB on a non-UNIX environment. I'm afraid you may have to guard this code with "HAVE_SYS_UN_H". > + > if (is_ipv6) > { > if (spec[0] == '[') > @@ -109,6 +111,12 @@ parse_connection_spec_without_prefix (std::string spec, struct addrinfo *hint) > if (ret.host_str.empty ()) > ret.host_str = "localhost"; > > + if (is_unix && ret.host_str != "localhost") > + error (_("The host name must be empty or 'localhost' for a unix domain socket.")); This line has more than 80 chars, so you will need to break it. Also, IIRC the official name for the system is "UNIX" (all caps). > + > + if (is_unix && ret.port_str.empty ()) > + error (_("A path name must be specified for a unix domain socket.")); So "port_str" is actually a path name if we're dealing with a UNIX socket? What do you think about changing the name of the field to "resource_str" or some such? > + > return ret; > } > > @@ -138,6 +146,7 @@ parse_connection_spec (const char *spec, struct addrinfo *hint) > { "tcp4:", AF_INET, SOCK_STREAM }, > { "udp6:", AF_INET6, SOCK_DGRAM }, > { "tcp6:", AF_INET6, SOCK_STREAM }, > + { "unix:", AF_LOCAL, SOCK_STREAM }, The comment I made about AF_UNIX above also applies to AF_LOCAL. > }; > > for (const host_prefix prefix : prefixes) > 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) The change for "gdb/gdbserver/configure" hasn't been included in the patch. Perhaps you forgot to regenerate it? > diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c > index 9199a9c7ad..0eb53c2116 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,33 @@ 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; Missing newline here. According to getsockname(2), 'len' should contain the initial size of the 'struct sockaddr' you're passing in the second argument. Therefore, you should declare 'len' as 'sizeof (su)'. Also, there's another 'len' declare in the scope above. In order to avoid -Wshadow warnings, I'd recommend you to choose another variable name here. Last, but not least, this code also needs to be guarded with #ifdef's. > + if (getsockname (listen_desc, (struct sockaddr *) &su, &len) != 0) > + perror (_("Could not obtain remote address")); > > - 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 +266,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. */ > @@ -258,7 +277,7 @@ remote_prepare (const char *name) > hint.ai_protocol = IPPROTO_TCP; > > parsed_connection_spec parsed > - = parse_connection_spec_without_prefix (name, &hint); > + = parse_connection_spec (name, &hint); There's a problem here: gdbserver doesn't work with UDP connections. Actually, this is the only reason why there's a "_without_prefix" variant of "parse_connection_spec": because, in this particular case, we *don't* want to consider prefixes. Because (so far) gdbserver deals only with TCP sockets, and because IPv4 and IPv6 addresses are easily distinguishable, we don't use any kind of prefix. However, with your patch, we will probably need a prefix indeed (to differentiate between serial tty devices and UNIX sockets, at least). My suggestion is that you make a "parse_connection_spec_maybe_unix" or some such, which takes into account *just* the UNIX prefix (and no other). After you extract the "unix:" prefix, you can pass the rest of the string to "parse_connection_spec_without_prefix" (which would be made "static" since it won't be used externally anymore). > > if (parsed.port_str.empty ()) > { > @@ -276,47 +295,86 @@ 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) ^ Missing whitespace between function and parens. > +#endif Even though I don't quite like using "PATH_MAX"-like because of reproducibility issues, I don't really know what else could be done here to prevent that. > > - scoped_free_addrinfo freeaddrinfo (ainfo); > + if (hint.ai_family == AF_LOCAL) > + { > + const char *sock_name = parsed.port_str.c_str (); > + if (parsed.port_str.length() > UNIX_PATH_MAX - 1) ^ Missing whitespace between function and parens. > + { > + error > + (_("%s is too long. Socket names may be no longer than %s bytes."), s/bytes/characters/ (increases readability, IMHO)? > + sock_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"); > > - struct addrinfo *iter; > + memset (&unix_addr, 0, sizeof (unix_addr)); > + unix_addr.sun_family = AF_UNIX; > + strncpy (unix_addr.sun_path, sock_name, UNIX_PATH_MAX - 1); > > - for (iter = ainfo; iter != NULL; iter = iter->ai_next) > - { > - listen_desc = gdb_socket_cloexec (iter->ai_family, iter->ai_socktype, > - iter->ai_protocol); > + struct stat statbuf; > + int stat_result = stat (sock_name, &statbuf); Missing newline here. No need for the "stat_result" variable; you can just check its return value in the "if" below. > + if (stat_result == 0 > + && (S_IFSOCK & statbuf.st_mode)) Don't think you need to break the line. You should use S_ISSOCK (statbuf.st_mode). > + unlink (sock_name); > > - if (listen_desc >= 0) > - break; > + addr = (struct sockaddr *) &unix_addr; > + addrlen = sizeof (unix_addr); > } > + else > + { > + struct addrinfo *iter; > > - if (iter == NULL) > - perror_with_name ("Can't open socket"); > + int r = getaddrinfo (parsed.host_str.c_str (), > + parsed.port_str.c_str (), > + &hint, &ainfo); > > - /* Allow rapid reuse of this port. */ > - tmp = 1; > - setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp, > - sizeof (tmp)); > + if (r != 0) > + error (_("%s: cannot resolve name: %s"), name, gai_strerror (r)); > > - 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); > + scoped_free_addrinfo freeaddrinfo (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 (listen_desc >= 0) > + break; > + } > + > + 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) > @@ -354,11 +412,11 @@ remote_open (const char *name) > } > #ifndef USE_WIN32API > else if (port_str == NULL) > - { > + { > struct stat statbuf; > > if (stat (name, &statbuf) == 0 > - && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode))) > + && (S_ISCHR (statbuf.st_mode) || S_ISFIFO (statbuf.st_mode))) These two changes are unrelated and should go in a separate patch (they can be pushed as obvious, BTW). > remote_desc = open (name, O_RDWR); > else > { > -- > 2.11.0 Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-18 20:18 ` Sergio Durigan Junior @ 2018-10-19 18:55 ` John Darrington 2018-10-19 19:43 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-19 18:55 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: John Darrington, gdb-patches On Thu, Oct 18, 2018 at 04:18:48PM -0400, Sergio Durigan Junior wrote: > + bool is_unix = hint->ai_family == AF_UNIX; No need for a newline between the declarations of is_ipv6 and is_unix. Here, and everywhere else, AF_UNIX may be undefined if building GDB on a non-UNIX environment. I'm afraid you may have to guard this code with "HAVE_SYS_UN_H". This is true. But a quick experiment showed me that there are quite a few other places in gdb which has this problem. I can prepare a separate patch to fix those. J' ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-19 18:55 ` John Darrington @ 2018-10-19 19:43 ` Sergio Durigan Junior 2018-10-28 16:20 ` Simon Marchi 0 siblings, 1 reply; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-19 19:43 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On Friday, October 19 2018, John Darrington wrote: > On Thu, Oct 18, 2018 at 04:18:48PM -0400, Sergio Durigan Junior wrote: > > > + bool is_unix = hint->ai_family == AF_UNIX; > > No need for a newline between the declarations of is_ipv6 and is_unix. > > Here, and everywhere else, AF_UNIX may be undefined if building GDB on a > non-UNIX environment. I'm afraid you may have to guard this code with > "HAVE_SYS_UN_H". > > > This is true. But a quick experiment showed me that there are quite a > few other places in gdb which has this problem. Which places? There are some files that are not compiled on certain systems, so it's fine to have system-dependent code without the guards. I don't use proprietary OSes, so the way I test here is to compile GDB using a mingw compiler. If it passes, then I assume things are OK. Your patch, for example, broke the compilation (because of AF_UNIX/AF_LOCAL). Cheers, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-19 19:43 ` Sergio Durigan Junior @ 2018-10-28 16:20 ` Simon Marchi 2018-10-28 18:10 ` John Darrington 0 siblings, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-10-28 16:20 UTC (permalink / raw) To: Sergio Durigan Junior, John Darrington; +Cc: gdb-patches On 2018-10-19 3:43 p.m., Sergio Durigan Junior wrote: > On Friday, October 19 2018, John Darrington wrote: > >> On Thu, Oct 18, 2018 at 04:18:48PM -0400, Sergio Durigan Junior wrote: >> >> > + bool is_unix = hint->ai_family == AF_UNIX; >> >> No need for a newline between the declarations of is_ipv6 and is_unix. >> >> Here, and everywhere else, AF_UNIX may be undefined if building GDB on a >> non-UNIX environment. I'm afraid you may have to guard this code with >> "HAVE_SYS_UN_H". >> >> >> This is true. But a quick experiment showed me that there are quite a >> few other places in gdb which has this problem. > > Which places? There are some files that are not compiled on certain > systems, so it's fine to have system-dependent code without the guards. > I don't use proprietary OSes, so the way I test here is to compile GDB > using a mingw compiler. If it passes, then I assume things are OK. > Your patch, for example, broke the compilation (because of > AF_UNIX/AF_LOCAL). > > Cheers, > I just tried compiling with mingw and stumbled on this: CXX common/netstuff.o /home/simark/src/binutils-gdb/gdb/common/netstuff.c: In function âparsed_connection_spec parse_connection_spec(const char*, addrinfo*)â: /home/simark/src/binutils-gdb/gdb/common/netstuff.c:148:18: error: âAF_LOCALâ was not declared in this scope { "unix:", AF_LOCAL, SOCK_STREAM }, ^~~~~~~~ What is the status on this? Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-28 16:20 ` Simon Marchi @ 2018-10-28 18:10 ` John Darrington 2018-10-28 18:51 ` Simon Marchi 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-28 18:10 UTC (permalink / raw) To: Simon Marchi; +Cc: Sergio Durigan Junior, John Darrington, gdb-patches Mea Culpa. I missed that one. I'll try to get a mingw environment set up and check in a fix. J' On Sun, Oct 28, 2018 at 12:20:28PM -0400, Simon Marchi wrote: On 2018-10-19 3:43 p.m., Sergio Durigan Junior wrote: > On Friday, October 19 2018, John Darrington wrote: > >> On Thu, Oct 18, 2018 at 04:18:48PM -0400, Sergio Durigan Junior wrote: >> >> > + bool is_unix = hint->ai_family == AF_UNIX; >> >> No need for a newline between the declarations of is_ipv6 and is_unix. >> >> Here, and everywhere else, AF_UNIX may be undefined if building GDB on a >> non-UNIX environment. I'm afraid you may have to guard this code with >> "HAVE_SYS_UN_H". >> >> >> This is true. But a quick experiment showed me that there are quite a >> few other places in gdb which has this problem. > > Which places? There are some files that are not compiled on certain > systems, so it's fine to have system-dependent code without the guards. > I don't use proprietary OSes, so the way I test here is to compile GDB > using a mingw compiler. If it passes, then I assume things are OK. > Your patch, for example, broke the compilation (because of > AF_UNIX/AF_LOCAL). > > Cheers, > I just tried compiling with mingw and stumbled on this: CXX common/netstuff.o /home/simark/src/binutils-gdb/gdb/common/netstuff.c: In function ???parsed_connection_spec parse_connection_spec(const char*, addrinfo*)???: /home/simark/src/binutils-gdb/gdb/common/netstuff.c:148:18: error: ???AF_LOCAL??? was not declared in this scope { "unix:", AF_LOCAL, SOCK_STREAM }, ^~~~~~~~ What is the status on this? Simon -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-28 18:10 ` John Darrington @ 2018-10-28 18:51 ` Simon Marchi 2018-10-29 8:24 ` John Darrington 0 siblings, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-10-28 18:51 UTC (permalink / raw) To: John Darrington; +Cc: Sergio Durigan Junior, gdb-patches On 2018-10-28 14:10, John Darrington wrote: > Mea Culpa. > > I missed that one. I'll try to get a mingw environment set up and > check in a fix. Ok. If you are on Linux, it shouldn't be too hard. For example, I use these packages (on two different machines): https://aur.archlinux.org/packages/mingw-w64-gcc/ https://packages.ubuntu.com/en/bionic/gcc-mingw-w64 and configure with --host=x86_64-w64-mingw32. I think that's all. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-28 18:51 ` Simon Marchi @ 2018-10-29 8:24 ` John Darrington 2018-10-29 9:13 ` Rainer Orth 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-29 8:24 UTC (permalink / raw) To: Simon Marchi; +Cc: John Darrington, Sergio Durigan Junior, gdb-patches On Sun, Oct 28, 2018 at 02:51:50PM -0400, Simon Marchi wrote: On 2018-10-28 14:10, John Darrington wrote: > Mea Culpa. > > I missed that one. I'll try to get a mingw environment set up and > check in a fix. Ok. If you are on Linux, it shouldn't be too hard. For example, I use these packages (on two different machines): https://aur.archlinux.org/packages/mingw-w64-gcc/ https://packages.ubuntu.com/en/bionic/gcc-mingw-w64 and configure with --host=x86_64-w64-mingw32. I think that's all. For some reason, when I try to crossbuild in this way, there are many failures (unrelated to this issue). However I've checked in a fix for this issue, and tested it by building natively with a hacked set of standard include headers. J' -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 8:24 ` John Darrington @ 2018-10-29 9:13 ` Rainer Orth 2018-10-29 9:38 ` Rainer Orth 2018-10-29 15:52 ` Simon Marchi 0 siblings, 2 replies; 40+ messages in thread From: Rainer Orth @ 2018-10-29 9:13 UTC (permalink / raw) To: John Darrington; +Cc: Simon Marchi, Sergio Durigan Junior, gdb-patches Hi John, > However I've checked in a fix for this issue, and tested it by building > natively with a hacked set of standard include headers. you always need to post patches here, if only for reference. Besides, we're currently very inconsistent here (haven't checked which part of that is due to your code): most places use AF_UNIX, only two use AF_LOCAL instead (common/netstuff.c, gdbserver/remote-utils.c), and your configure check only checks for AF_LOCAL. I believe we should canonicalize for one of the two and allow for systems that define only one or the other. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 9:13 ` Rainer Orth @ 2018-10-29 9:38 ` Rainer Orth 2018-10-29 15:52 ` Simon Marchi 1 sibling, 0 replies; 40+ messages in thread From: Rainer Orth @ 2018-10-29 9:38 UTC (permalink / raw) To: John Darrington; +Cc: Simon Marchi, Sergio Durigan Junior, gdb-patches Hi John, >> However I've checked in a fix for this issue, and tested it by building >> natively with a hacked set of standard include headers. > > you always need to post patches here, if only for reference. I also noticed that your ChangeLog entry had a couple of formatting issues. I've fixed them with my last commit. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 9:13 ` Rainer Orth 2018-10-29 9:38 ` Rainer Orth @ 2018-10-29 15:52 ` Simon Marchi 2018-10-29 16:26 ` John Darrington 1 sibling, 1 reply; 40+ messages in thread From: Simon Marchi @ 2018-10-29 15:52 UTC (permalink / raw) To: Rainer Orth, John Darrington Cc: Simon Marchi, Sergio Durigan Junior, gdb-patches On 2018-10-29 5:11 a.m., Rainer Orth wrote: > Hi John, > >> However I've checked in a fix for this issue, and tested it by building >> natively with a hacked set of standard include headers. > > you always need to post patches here, if only for reference. Not only should you post here the patches you push as obvious, but I don't think that this: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=98a17ece013cb94cd602496b9efb92b8816b3953 falls under the obvious rule: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/MAINTAINERS;h=d5154d394206861ef5471b4984a4762589c34fc4;hb=HEAD#l82 I can't judge whether the patch is right or not with a quick glance, but it certainly is complex enough to warrant a discussion (as Rainer's reply below shows). Additionally, it seems like the initial 4-patch series was pushed without explicit approval from a maintainer (at least I can't find any). Next time, please wait to have an approval before pushing. If you are not sure whether a reply constitutes an a approval, it's better to ask the maintainer to clarify. > Besides, we're currently very inconsistent here (haven't checked which > part of that is due to your code): most places use AF_UNIX, only two use > AF_LOCAL instead (common/netstuff.c, gdbserver/remote-utils.c), and your > configure check only checks for AF_LOCAL. I believe we should > canonicalize for one of the two and allow for systems that define only > one or the other. mingw defines AF_UNIX, so I would tend to go towards that route. Any of you knows what happens at runtime when you try to bind a AF_UNIX socket on mingw/Windows? Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 15:52 ` Simon Marchi @ 2018-10-29 16:26 ` John Darrington 2018-10-29 16:42 ` Sergio Durigan Junior 0 siblings, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-29 16:26 UTC (permalink / raw) To: Simon Marchi Cc: Rainer Orth, John Darrington, Simon Marchi, Sergio Durigan Junior, gdb-patches On Mon, Oct 29, 2018 at 03:51:55PM +0000, Simon Marchi wrote: On 2018-10-29 5:11 a.m., Rainer Orth wrote: > Hi John, > >> However I've checked in a fix for this issue, and tested it by building >> natively with a hacked set of standard include headers. > > you always need to post patches here, if only for reference. Doesn't that make the gdb-cvs list completely redundant? Not only should you post here the patches you push as obvious, but I don't think that this: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=98a17ece013cb94cd602496b9efb92b8816b3953 falls under the obvious rule: But a number of people had complained that their build was broken, and this was a fix for that. I judged that in consideration of those people fixding their problem was more important than strict observance of protocol. I can't judge whether the patch is right or not with a quick glance, but it certainly is complex enough to warrant a discussion (as Rainer's reply below shows). Additionally, it seems like the initial 4-patch series was pushed without explicit approval from a maintainer (at least I can't find any). Next time, please wait to have an approval before pushing. If you are not sure whether a reply constitutes an a approval, it's better to ask the maintainer to clarify. All of those patches were certainly discussed. In the past, when I've followed up a person who has commented on a patch, but been vague about approval, I have had either a piqued response; or no response at all. If you think it necesary however I can revert anything you think hasn't had enough discussion. > Besides, we're currently very inconsistent here (haven't checked which > part of that is due to your code): most places use AF_UNIX, only two use > AF_LOCAL instead (common/netstuff.c, gdbserver/remote-utils.c), and your > configure check only checks for AF_LOCAL. I believe we should > canonicalize for one of the two and allow for systems that define only > one or the other. mingw defines AF_UNIX, so I would tend to go towards that route. Any of you knows what happens at runtime when you try to bind a AF_UNIX socket on mingw/Windows? AS I understand it, it depends upon which version of windows. Apparently recent versions have local ssockets, but older ones do not. See https://blogs.msdn.microsoft.com/commandline/2017/12/19/af_unix-comes-to-windows/ J' ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 16:26 ` John Darrington @ 2018-10-29 16:42 ` Sergio Durigan Junior 2018-10-29 17:34 ` John Darrington 2018-10-29 18:32 ` Simon Marchi 0 siblings, 2 replies; 40+ messages in thread From: Sergio Durigan Junior @ 2018-10-29 16:42 UTC (permalink / raw) To: John Darrington; +Cc: Simon Marchi, Rainer Orth, Simon Marchi, gdb-patches On Monday, October 29 2018, John Darrington wrote: > On Mon, Oct 29, 2018 at 03:51:55PM +0000, Simon Marchi wrote: > On 2018-10-29 5:11 a.m., Rainer Orth wrote: > > Hi John, > > > >> However I've checked in a fix for this issue, and tested it by building > >> natively with a hacked set of standard include headers. > > > > you always need to post patches here, if only for reference. > > Doesn't that make the gdb-cvs list completely redundant? > > Not only should you post here the patches you push as obvious, but I don't > think that this: > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=98a17ece013cb94cd602496b9efb92b8816b3953 > > falls under the obvious rule: > > But a number of people had complained that their build was broken, and > this was a fix for that. I judged that in consideration of those > people fixding their problem was more important than strict observance > of protocol. > > I can't judge whether the patch is right or not with a quick glance, but it > certainly is complex enough to warrant a discussion (as Rainer's reply below > shows). > > > Additionally, it seems like the initial 4-patch series was pushed without > explicit approval from a maintainer (at least I can't find any). Next time, > please wait to have an approval before pushing. If you are not sure whether > a reply constitutes an a approval, it's better to ask the maintainer to > clarify. > > All of those patches were certainly discussed. In the past, when I've > followed up a person who has commented on a patch, but been vague about > approval, I have had either a piqued response; or no response at all. We were certainly discussing the patches, but they were not approved by anyone. It's also worth mentioning that I raised various points that were not addressed (even though we discussed them). It is still a requirement that the patches need to be approved by at least one maintainer before it is pushed to the repository. > If you think it necesary however I can revert anything you think hasn't > had enough discussion. Yes, please. The patch series is not ready to be pushed yet; there are a bunch of points I raised that were not addressed (and are now causing the failures). I am not a global maintainer, but it is my opinion that the patch series should be reverted for now. We can continue discussing and fixing things with it here in the list. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 16:42 ` Sergio Durigan Junior @ 2018-10-29 17:34 ` John Darrington 2018-10-31 13:54 ` Simon Marchi 2018-10-29 18:32 ` Simon Marchi 1 sibling, 1 reply; 40+ messages in thread From: John Darrington @ 2018-10-29 17:34 UTC (permalink / raw) To: Sergio Durigan Junior Cc: John Darrington, Simon Marchi, Rainer Orth, Simon Marchi, gdb-patches On Mon, Oct 29, 2018 at 12:42:43PM -0400, Sergio Durigan Junior wrote: > If you think it necesary however I can revert anything you think hasn't > had enough discussion. Yes, please. The patch series is not ready to be pushed yet; there are a bunch of points I raised that were not addressed (and are now causing the failures). I am not a global maintainer, but it is my opinion that the patch series should be reverted for now. We can continue discussing and fixing things with it here in the list. Done. Start discussing. J' ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 17:34 ` John Darrington @ 2018-10-31 13:54 ` Simon Marchi 0 siblings, 0 replies; 40+ messages in thread From: Simon Marchi @ 2018-10-31 13:54 UTC (permalink / raw) To: John Darrington, Sergio Durigan Junior Cc: Rainer Orth, Simon Marchi, gdb-patches On 2018-10-29 1:34 p.m., John Darrington wrote: > On Mon, Oct 29, 2018 at 12:42:43PM -0400, Sergio Durigan Junior wrote: > > > If you think it necesary however I can revert anything you think hasn't > > had enough discussion. > > Yes, please. The patch series is not ready to be pushed yet; there are > a bunch of points I raised that were not addressed (and are now causing > the failures). I am not a global maintainer, but it is my opinion that > the patch series should be reverted for now. We can continue discussing > and fixing things with it here in the list. > > Done. > > Start discussing. Well, there are these two loose ends. https://sourceware.org/ml/gdb-patches/2018-10/msg00438.html https://sourceware.org/ml/gdb-patches/2018-10/msg00465.html Also, please check why this series made the buildbot go crazy in the native-extended-gdbserver configuration: https://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m64/builds/11314 It was suggested on IRC that the scoped_free_addrinfo in remote-utils.c is at a wrong place, causing something to be used after free. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested. 2018-10-29 16:42 ` Sergio Durigan Junior 2018-10-29 17:34 ` John Darrington @ 2018-10-29 18:32 ` Simon Marchi 1 sibling, 0 replies; 40+ messages in thread From: Simon Marchi @ 2018-10-29 18:32 UTC (permalink / raw) To: Sergio Durigan Junior, John Darrington Cc: Rainer Orth, Simon Marchi, gdb-patches On 2018-10-29 12:42 p.m., Sergio Durigan Junior wrote: > On Monday, October 29 2018, John Darrington wrote: > >> On Mon, Oct 29, 2018 at 03:51:55PM +0000, Simon Marchi wrote: >> On 2018-10-29 5:11 a.m., Rainer Orth wrote: >> > Hi John, >> > >> >> However I've checked in a fix for this issue, and tested it by building >> >> natively with a hacked set of standard include headers. >> > >> > you always need to post patches here, if only for reference. >> >> Doesn't that make the gdb-cvs list completely redundant? Perhaps, I don't know. Personally, I am not subscribed to gdb-cvs. And generally, when posting on gdb-patches, you'll mention that the patch was pushed as "obvious", which the commit message won't necessarily contain. It's also sometimes necessary to discuss a patch that was initially committed as obvious. The post on gdb-patches would be the place for that. >> Not only should you post here the patches you push as obvious, but I don't >> think that this: >> >> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=98a17ece013cb94cd602496b9efb92b8816b3953 >> >> falls under the obvious rule: >> >> But a number of people had complained that their build was broken, and >> this was a fix for that. I judged that in consideration of those >> people fixding their problem was more important than strict observance >> of protocol. Let me reassure you, I completely understand that the intention was good. I don't want you to feel bad in any way for trying to fix things up! Next time, post the patch to the mailing list for review. If the patch title contains "Fix build for...", it is likely that it will be reviewed quite quickly. >> >> I can't judge whether the patch is right or not with a quick glance, but it >> certainly is complex enough to warrant a discussion (as Rainer's reply below >> shows). >> >> >> Additionally, it seems like the initial 4-patch series was pushed without >> explicit approval from a maintainer (at least I can't find any). Next time, >> please wait to have an approval before pushing. If you are not sure whether >> a reply constitutes an a approval, it's better to ask the maintainer to >> clarify. >> >> All of those patches were certainly discussed. In the past, when I've >> followed up a person who has commented on a patch, but been vague about >> approval, I have had either a piqued response; or no response at all. > > We were certainly discussing the patches, but they were not approved by > anyone. It's also worth mentioning that I raised various points that > were not addressed (even though we discussed them). It is still a > requirement that the patches need to be approved by at least one > maintainer before it is pushed to the repository. Yep. >> If you think it necesary however I can revert anything you think hasn't >> had enough discussion. > > Yes, please. The patch series is not ready to be pushed yet; there are > a bunch of points I raised that were not addressed (and are now causing > the failures). I am not a global maintainer, but it is my opinion that > the patch series should be reverted for now. We can continue discussing > and fixing things with it here in the list. I have reverted the initial series as well as the fixup. Sergio has mentioned on IRC that it causes some test failures on the buildbot that cause the test time very long. So this needs more testing before we push it again. Simon ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2018-10-31 13:54 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-09 17:33 Gdbserver can listen on local domain sockets John Darrington 2018-10-09 17:33 ` [PATCH] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested John Darrington 2018-10-09 17:56 ` Eli Zaretskii 2018-10-09 18:02 ` Pedro Alves 2018-10-09 18:41 ` John Darrington 2018-10-09 18:53 ` Pedro Alves 2018-10-09 19:00 ` John Darrington 2018-10-09 19:06 ` Pedro Alves 2018-10-13 17:58 ` [PATCH 1/4] " John Darrington 2018-10-13 17:58 ` [PATCH 3/4] GDB: Fix documentation for invoking GDBSERVER John Darrington 2018-10-13 18:10 ` Eli Zaretskii 2018-10-18 20:27 ` Sergio Durigan Junior 2018-10-19 7:05 ` John Darrington 2018-10-19 20:45 ` Sergio Durigan Junior 2018-10-21 7:33 ` John Darrington 2018-10-21 16:47 ` Sergio Durigan Junior 2018-10-23 18:25 ` John Darrington 2018-10-23 18:58 ` Sergio Durigan Junior 2018-10-13 17:58 ` [PATCH 4/4] GDB: Remote target can now accept the form unix::/path/to/socket John Darrington 2018-10-13 17:58 ` [PATCH 2/4] GDB: Document the unix::/path/to/socket of remote connection John Darrington 2018-10-13 18:11 ` Eli Zaretskii 2018-10-15 9:31 ` Simon Tatham 2018-10-15 11:28 ` John Darrington 2018-10-18 20:21 ` Sergio Durigan Junior 2018-10-13 18:12 ` [PATCH 1/4] GDBSERVER: Listen on a unix domain (instead of TCP) socket if requested Eli Zaretskii 2018-10-18 20:18 ` Sergio Durigan Junior 2018-10-19 18:55 ` John Darrington 2018-10-19 19:43 ` Sergio Durigan Junior 2018-10-28 16:20 ` Simon Marchi 2018-10-28 18:10 ` John Darrington 2018-10-28 18:51 ` Simon Marchi 2018-10-29 8:24 ` John Darrington 2018-10-29 9:13 ` Rainer Orth 2018-10-29 9:38 ` Rainer Orth 2018-10-29 15:52 ` Simon Marchi 2018-10-29 16:26 ` John Darrington 2018-10-29 16:42 ` Sergio Durigan Junior 2018-10-29 17:34 ` John Darrington 2018-10-31 13:54 ` Simon Marchi 2018-10-29 18:32 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).