* Remote debugging over local domain sockets? @ 2018-08-29 7:05 John Darrington 2018-08-29 15:17 ` Tom Tromey 0 siblings, 1 reply; 16+ messages in thread From: John Darrington @ 2018-08-29 7:05 UTC (permalink / raw) To: gdb-patches So far as I'm aware, it's not possible to run a remote target over a local domain (AF_UNIX) socket. Would you accept a patch which changes the behaviour of "target remote /foo/bar" such that if /foo/bar is a socket, gdb will connect to that socket instead of trying to open it as a device? J' ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Remote debugging over local domain sockets? 2018-08-29 7:05 Remote debugging over local domain sockets? John Darrington @ 2018-08-29 15:17 ` Tom Tromey 2018-08-29 15:28 ` John Darrington 0 siblings, 1 reply; 16+ messages in thread From: Tom Tromey @ 2018-08-29 15:17 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes: John> So far as I'm aware, it's not possible to run a remote target over John> a local domain (AF_UNIX) socket. John> Would you accept a patch which changes the behaviour of "target John> remote /foo/bar" such that if /foo/bar is a socket, gdb will John> connect to that socket instead of trying to open it as a device? How about stat'ing the path and doing the right thing depending on whether it is a device or a socket? Alternatively, how about new syntax instead? "target remote /path" has meant to open a serial device since forever. However there's also syntax like "target remote udp:host:port". So maybe "target remote socket:/path"? Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Remote debugging over local domain sockets? 2018-08-29 15:17 ` Tom Tromey @ 2018-08-29 15:28 ` John Darrington 2018-08-29 15:39 ` Tom Tromey 0 siblings, 1 reply; 16+ messages in thread From: John Darrington @ 2018-08-29 15:28 UTC (permalink / raw) To: Tom Tromey; +Cc: John Darrington, gdb-patches On Wed, Aug 29, 2018 at 09:17:00AM -0600, Tom Tromey wrote: >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes: John> So far as I'm aware, it's not possible to run a remote target over John> a local domain (AF_UNIX) socket. John> Would you accept a patch which changes the behaviour of "target John> remote /foo/bar" such that if /foo/bar is a socket, gdb will John> connect to that socket instead of trying to open it as a device? How about stat'ing the path and doing the right thing depending on whether it is a device or a socket? That was my idea. Alternatively, how about new syntax instead? "target remote /path" has meant to open a serial device since forever. However there's also syntax like "target remote udp:host:port". So maybe "target remote socket:/path"? You mean where 'socket' is the literal string "socket" ? That might also work, but one would have to consider what would happen if somebody wanted to connect to a remote host called "socket" . I prefer the first idea. -- 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] 16+ messages in thread
* Re: Remote debugging over local domain sockets? 2018-08-29 15:28 ` John Darrington @ 2018-08-29 15:39 ` Tom Tromey 2018-08-31 10:18 ` [PATCH] Allow remote debugging over a local domain socket John Darrington 0 siblings, 1 reply; 16+ messages in thread From: Tom Tromey @ 2018-08-29 15:39 UTC (permalink / raw) To: John Darrington; +Cc: Tom Tromey, gdb-patches >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes: > How about stat'ing the path and doing the right thing depending on > whether it is a device or a socket? John> That was my idea. Hah, sorry. I completely misread that sentence. I think this would be fine. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] Allow remote debugging over a local domain socket 2018-08-29 15:39 ` Tom Tromey @ 2018-08-31 10:18 ` John Darrington 2018-08-31 14:59 ` Eli Zaretskii ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: John Darrington @ 2018-08-31 10:18 UTC (permalink / raw) To: gdb-patches; +Cc: John Darrington Extend the "target remote" and "target extended-remote" commands such that if the filename provided is a unix domain (AF_UNIX) socket, then it'll be treated as such, instead of trying to open it as if it were a character device. gdb/ChangeLog: * gdb/NEWS: Mention changed commands. * gdb/configure.ac (SER_HARDWIRE): Add ser-socket.o * gdb/doc/gdb.texinfo (Remote Connection Commands): Describe changed commands. * gdb/ser-socket.c: New file. * gdb/ser-socket.h: New file. * gdb/Makefile.in: Add new files. * gdb/serial.c (serial_open): Check if filename is a socket and lookup the appropriate interface accordingly. --- gdb/Makefile.in | 2 + gdb/NEWS | 5 ++ gdb/configure.ac | 2 +- gdb/doc/gdb.texinfo | 19 +++++++- gdb/ser-socket.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/ser-socket.h | 31 ++++++++++++ gdb/serial.c | 10 +++- 7 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 gdb/ser-socket.c create mode 100644 gdb/ser-socket.h diff --git a/gdb/Makefile.in b/gdb/Makefile.in index f448d1ee19..cb98f987da 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1355,6 +1355,7 @@ HFILES_NO_SRCDIR = \ sentinel-frame.h \ ser-base.h \ ser-event.h \ + ser-socket.h \ ser-tcp.h \ ser-unix.h \ serial.h \ @@ -2324,6 +2325,7 @@ ALLDEPFILES = \ ser-go32.c \ ser-mingw.c \ ser-pipe.c \ + ser-socket.c \ ser-tcp.c \ sh-nbsd-nat.c \ sh-nbsd-tdep.c \ diff --git a/gdb/NEWS b/gdb/NEWS index c46056525a..c7436e0ced 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -40,6 +40,11 @@ maint show dwarf unwinders * Changed commands +target remote FILENAME +target extended-remote FILENAME + If FILENAME is a unix domain socket gdb will attempt to connect + to this socket instead of opening FILENAME as a character device. + thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND The 'thread apply' command accepts new FLAG arguments. FLAG arguments allow to control what output to produce and how to handle diff --git a/gdb/configure.ac b/gdb/configure.ac index 13bc5f9a8f..f6735858d7 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -1870,7 +1870,7 @@ lose dnl Figure out which of the many generic ser-*.c files the _host_ supports. -SER_HARDWIRE="ser-base.o ser-unix.o ser-pipe.o ser-tcp.o" +SER_HARDWIRE="ser-base.o ser-unix.o ser-pipe.o ser-socket.o ser-tcp.o" case ${host} in *go32* ) SER_HARDWIRE=ser-go32.o ;; *djgpp* ) SER_HARDWIRE=ser-go32.o ;; diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index e4ecd57a9e..5ae53e0582 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20703,7 +20703,8 @@ programs. @subsection Remote Connection Commands @cindex remote connection commands -@value{GDBN} can communicate with the target over a serial line, or +@value{GDBN} can communicate with the target over a serial line, a +local socket, or over an @acronym{IP} network using @acronym{TCP} or @acronym{UDP}. In each case, @value{GDBN} uses the same protocol for debugging your program; only the medium carrying the debugging packets varies. The @@ -20728,6 +20729,22 @@ If you're using a serial line, you may want to give @value{GDBN} the (@pxref{Remote Configuration, set serial baud}) before the @code{target} command. + +@item target remote @var{local-socket} +@itemx target extended-remote @var{local-socket} +@cindex local socket, @code{target remote} +Use @var{local-socket} to communicate with the target. For example, +to use a local socket bound to the file system entry @file{/tmp/gdb-socket0}: + +@smallexample +target remote /tmp/gdb-socket0 +@end smallexample + +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. + @item target remote @code{@var{host}:@var{port}} @itemx target remote @code{@var{[host]}:@var{port}} @itemx target remote @code{tcp:@var{host}:@var{port}} diff --git a/gdb/ser-socket.c b/gdb/ser-socket.c new file mode 100644 index 0000000000..54d14f36ad --- /dev/null +++ b/gdb/ser-socket.c @@ -0,0 +1,132 @@ +/* Serial interface for local domain connections on Un*x like systems. + + Copyright (C) 1992-2018 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include "defs.h" +#include "serial.h" +#include "ser-base.h" +#include "ser-socket.h" + +#include <sys/socket.h> +#include <sys/un.h> + + +/* Open a AF_UNIX socket. */ +int +socket_open (struct serial *scb, const char *name) +{ + struct sockaddr_un addr; + + memset (&addr, 0, sizeof addr); + addr.sun_family = AF_UNIX; +#ifndef UNIX_MAX_PATH +# define UNIX_MAX_PATH 108 +#endif + strncpy (addr.sun_path, name, UNIX_MAX_PATH - 1); + + int sock = socket (AF_UNIX, SOCK_STREAM, 0); + + if (connect (sock, (struct sockaddr *) &addr, + sizeof (struct sockaddr_un)) < 0) + { + close (sock); + scb->fd = -1; + return -1; + } + + scb->fd = sock; + + return 0; +} + +void +socket_close (struct serial *scb) +{ + if (scb->fd == -1) + return; + + close (scb->fd); + scb->fd = -1; +} + +int +socket_read_prim (struct serial *scb, size_t count) +{ + /* Need to cast to silence -Wpointer-sign on MinGW, as Winsock's + 'recv' takes 'char *' as second argument, while 'scb->buf' is + 'unsigned char *'. */ + return recv (scb->fd, (char *) scb->buf, count, 0); +} + +int +socket_write_prim (struct serial *scb, const void *buf, size_t count) +{ + /* On Windows, the second parameter to send is a "const char *"; on + UNIX systems it is generally "const void *". The cast to "const + char *" is OK everywhere, since in C++ any data pointer type can + be implicitly converted to "const void *". */ + return send (scb->fd, (const char *) buf, count, 0); +} + +int +ser_socket_send_break (struct serial *scb) +{ + /* Send telnet IAC and BREAK characters. */ + return (serial_write (scb, "\377\363", 2)); +} + +#ifndef USE_WIN32API + +/* The SOCKET ops. */ + +static const struct serial_ops socket_ops = +{ + "socket", + socket_open, + socket_close, + NULL, + ser_base_readchar, + ser_base_write, + ser_base_flush_output, + ser_base_flush_input, + ser_socket_send_break, + ser_base_raw, + ser_base_get_tty_state, + ser_base_copy_tty_state, + ser_base_set_tty_state, + ser_base_print_tty_state, + ser_base_setbaudrate, + ser_base_setstopbits, + ser_base_setparity, + ser_base_drain_output, + ser_base_async, + socket_read_prim, + socket_write_prim +}; + +#endif /* USE_WIN32API */ + +void +_initialize_ser_socket (void) +{ +#ifdef USE_WIN32API + /* Do nothing; Windoze does not have local domain sockets. */ +#else + serial_add_interface (&socket_ops); +#endif /* USE_WIN32API */ +} diff --git a/gdb/ser-socket.h b/gdb/ser-socket.h new file mode 100644 index 0000000000..58509302d6 --- /dev/null +++ b/gdb/ser-socket.h @@ -0,0 +1,31 @@ +/* Serial interface for raw TCP connections on Un*x like systems. + + Copyright (C) 2006-2018 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#ifndef SER_SOCKET_H +#define SER_SOCKET_H + +struct serial; + +extern int socket_open (struct serial *scb, const char *name); +extern void socket_close (struct serial *scb); +extern int socket_read_prim (struct serial *scb, size_t count); +extern int socket_write_prim (struct serial *scb, const void *buf, size_t count); +extern int ser_socket_send_break (struct serial *scb); + +#endif diff --git a/gdb/serial.c b/gdb/serial.c index fb2b212918..13b1af3873 100644 --- a/gdb/serial.c +++ b/gdb/serial.c @@ -213,7 +213,15 @@ serial_open (const char *name) else if (strchr (name, ':')) ops = serial_interface_lookup ("tcp"); else - ops = serial_interface_lookup ("hardwire"); + { + /* Check to see if name is a socket. If it is, then treat is + as such. Otherwise assume that it's a character device. */ + struct stat sb; + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) + ops = serial_interface_lookup ("socket"); + else + ops = serial_interface_lookup ("hardware"); + } if (!ops) return NULL; -- 2.11.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 10:18 ` [PATCH] Allow remote debugging over a local domain socket John Darrington @ 2018-08-31 14:59 ` Eli Zaretskii 2018-08-31 15:10 ` John Darrington 2018-08-31 15:10 ` Tom Tromey 2018-08-31 16:01 ` Pedro Alves 2 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2018-08-31 14:59 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: Fri, 31 Aug 2018 12:18:18 +0200 > > Extend the "target remote" and "target extended-remote" commands > such that if the filename provided is a unix domain (AF_UNIX) > socket, then it'll be treated as such, instead of trying to open > it as if it were a character device. Thanks. > +target remote FILENAME > +target extended-remote FILENAME > + If FILENAME is a unix domain socket gdb will attempt to connect ^ Missing comma. Also, I believe "unix" should be "Unix" and "gdb" should be "GDB". > @subsection Remote Connection Commands > @cindex remote connection commands > -@value{GDBN} can communicate with the target over a serial line, or > +@value{GDBN} can communicate with the target over a serial line, a > +local socket, or I'd prefer to say "local Unix domain socket" here. > +@item target remote @var{local-socket} > +@itemx target extended-remote @var{local-socket} > +@cindex local socket, @code{target remote} > +Use @var{local-socket} to communicate with the target. For example, > +to use a local socket bound to the file system entry @file{/tmp/gdb-socket0}: Likewise here. Alternatively, add a note that this only works on systems that support Unix domain sockets. The documentation parts are approved with these fixed. > +int > +socket_read_prim (struct serial *scb, size_t count) > +{ > + /* Need to cast to silence -Wpointer-sign on MinGW, as Winsock's > + 'recv' takes 'char *' as second argument, while 'scb->buf' is > + 'unsigned char *'. */ > + return recv (scb->fd, (char *) scb->buf, count, 0); > +} > + > +int > +socket_write_prim (struct serial *scb, const void *buf, size_t count) > +{ > + /* On Windows, the second parameter to send is a "const char *"; on > + UNIX systems it is generally "const void *". The cast to "const > + char *" is OK everywhere, since in C++ any data pointer type can > + be implicitly converted to "const void *". */ > + return send (scb->fd, (const char *) buf, count, 0); > +} I'm confused: why does this mention MinGW and Windows, when Windows doesn't support AF_UNIX (AFAIK)? Should this stuff even be compiled on Windows? > +#ifdef USE_WIN32API > + /* Do nothing; Windoze does not have local domain sockets. */ Exactly! > - ops = serial_interface_lookup ("hardwire"); > + { > + /* Check to see if name is a socket. If it is, then treat is > + as such. Otherwise assume that it's a character device. */ > + struct stat sb; > + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) AFAIK, S_IFSOCK is not defined in the MinGW headers, so we need some replacement definition, or we need to ifdef this away in the Windows build. Thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 14:59 ` Eli Zaretskii @ 2018-08-31 15:10 ` John Darrington 2018-08-31 17:50 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: John Darrington @ 2018-08-31 15:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: John Darrington, gdb-patches On Fri, Aug 31, 2018 at 05:58:47PM +0300, Eli Zaretskii wrote: > +int > +socket_read_prim (struct serial *scb, size_t count) > +{ > + /* Need to cast to silence -Wpointer-sign on MinGW, as Winsock's > + 'recv' takes 'char *' as second argument, while 'scb->buf' is > + 'unsigned char *'. */ > + return recv (scb->fd, (char *) scb->buf, count, 0); > +} > + > +int > +socket_write_prim (struct serial *scb, const void *buf, size_t count) > +{ > + /* On Windows, the second parameter to send is a "const char *"; on > + UNIX systems it is generally "const void *". The cast to "const > + char *" is OK everywhere, since in C++ any data pointer type can > + be implicitly converted to "const void *". */ > + return send (scb->fd, (const char *) buf, count, 0); > +} I'm confused: why does this mention MinGW and Windows, when Windows doesn't support AF_UNIX (AFAIK)? Should this stuff even be compiled on Windows? > +#ifdef USE_WIN32API > + /* Do nothing; Windoze does not have local domain sockets. */ Exactly! > - ops = serial_interface_lookup ("hardwire"); > + { > + /* Check to see if name is a socket. If it is, then treat is > + as such. Otherwise assume that it's a character device. */ > + struct stat sb; > + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) AFAIK, S_IFSOCK is not defined in the MinGW headers, so we need some replacement definition, or we need to ifdef this away in the Windows build. I am told, that the recent versions of Windows 10 do indeed have local domain sockets. But if you like, I can conditionally include the file in the SER_HARDWIRE variable. Then the whole issue is moot. 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] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 15:10 ` John Darrington @ 2018-08-31 17:50 ` Eli Zaretskii 0 siblings, 0 replies; 16+ messages in thread From: Eli Zaretskii @ 2018-08-31 17:50 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches > Date: Fri, 31 Aug 2018 17:10:38 +0200 > From: John Darrington <john@darrington.wattle.id.au> > Cc: John Darrington <john@darrington.wattle.id.au>, gdb-patches@sourceware.org > > AFAIK, S_IFSOCK is not defined in the MinGW headers, so we need some > replacement definition, or we need to ifdef this away in the Windows > build. > > I am told, that the recent versions of Windows 10 do indeed have local > domain sockets. That's OK, but even the newer versions of MinGW don't yet have S_IFSOCK in their headers, AFAICS, so they will fail to compile this code. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 10:18 ` [PATCH] Allow remote debugging over a local domain socket John Darrington 2018-08-31 14:59 ` Eli Zaretskii @ 2018-08-31 15:10 ` Tom Tromey 2018-08-31 15:12 ` John Darrington 2018-08-31 16:01 ` Pedro Alves 2 siblings, 1 reply; 16+ messages in thread From: Tom Tromey @ 2018-08-31 15:10 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes: John> Extend the "target remote" and "target extended-remote" commands John> such that if the filename provided is a unix domain (AF_UNIX) John> socket, then it'll be treated as such, instead of trying to open John> it as if it were a character device. Thanks for the patch. This looks essentially reasonable to me. John> +/* Open a AF_UNIX socket. */ John> +int John> +socket_open (struct serial *scb, const char *name) John> +{ It seems to me that all the functions in this file could be static. This might necessitate wrapping many of them in "#ifndef USE_WIN32API" to avoid warnings about unused code, but that seems like an improvement as well. John> +int John> +ser_socket_send_break (struct serial *scb) John> +{ John> + /* Send telnet IAC and BREAK characters. */ John> + return (serial_write (scb, "\377\363", 2)); John> +} I don't really know what's expected here, but is this correct? John> diff --git a/gdb/ser-socket.h b/gdb/ser-socket.h John> new file mode 100644 John> index 0000000000..58509302d6 John> --- /dev/null John> +++ b/gdb/ser-socket.h You could just drop this file entirely. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 15:10 ` Tom Tromey @ 2018-08-31 15:12 ` John Darrington 0 siblings, 0 replies; 16+ messages in thread From: John Darrington @ 2018-08-31 15:12 UTC (permalink / raw) To: Tom Tromey; +Cc: John Darrington, gdb-patches On Fri, Aug 31, 2018 at 09:09:23AM -0600, Tom Tromey wrote: >>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes: John> Extend the "target remote" and "target extended-remote" commands John> such that if the filename provided is a unix domain (AF_UNIX) John> socket, then it'll be treated as such, instead of trying to open John> it as if it were a character device. Thanks for the patch. This looks essentially reasonable to me. Thanks John> +/* Open a AF_UNIX socket. */ John> +int John> +socket_open (struct serial *scb, const char *name) John> +{ It seems to me that all the functions in this file could be static. This might necessitate wrapping many of them in "#ifndef USE_WIN32API" to avoid warnings about unused code, but that seems like an improvement as well. OK John> +int John> +ser_socket_send_break (struct serial *scb) John> +{ John> + /* Send telnet IAC and BREAK characters. */ John> + return (serial_write (scb, "\377\363", 2)); John> +} I don't really know what's expected here, but is this correct? I wondererd about that too. This bit I copied from the ser-tcp.c file. John> diff --git a/gdb/ser-socket.h b/gdb/ser-socket.h John> new file mode 100644 John> index 0000000000..58509302d6 John> --- /dev/null John> +++ b/gdb/ser-socket.h You could just drop this file entirely. OK. -- 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] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 10:18 ` [PATCH] Allow remote debugging over a local domain socket John Darrington 2018-08-31 14:59 ` Eli Zaretskii 2018-08-31 15:10 ` Tom Tromey @ 2018-08-31 16:01 ` Pedro Alves 2018-08-31 16:40 ` John Darrington 2 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2018-08-31 16:01 UTC (permalink / raw) To: John Darrington, gdb-patches On 08/31/2018 11:18 AM, John Darrington wrote: > Extend the "target remote" and "target extended-remote" commands > such that if the filename provided is a unix domain (AF_UNIX) > socket, then it'll be treated as such, instead of trying to open > it as if it were a character device. What server are you using this against? It'd be great to add support for gdbserver as well. Were you planning on doing it? If we had that, then I think we could add unix domain socket testing to gdb/testsuite/gdb.server/server-connect.exp (assuming it'd be easy enough to create a socket from tcl). > > gdb/ChangeLog: > * gdb/NEWS: Mention changed commands. > * gdb/configure.ac (SER_HARDWIRE): Add ser-socket.o > * gdb/ser-socket.c: New file. > * gdb/ser-socket.h: New file. > * gdb/Makefile.in: Add new files. > * gdb/serial.c (serial_open): Check if filename is a socket > and lookup the appropriate interface accordingly. Remove leading "gdb/" from all these entries. The paths in the entry are relative to the ChangeLog file. > * gdb/doc/gdb.texinfo (Remote Connection Commands): Describe > changed commands. gdb/doc/ has its own ChangeLog file. Move it there, and remove the "gdb/doc/" prefix. Update "Describe changed commands" to be a bit more standalone, since it won't have the gdb/ changes context at hand. Also please fix the double-space, missing period, and check tab vs spaces in the entry. > + > + > +/* Open a AF_UNIX socket. */ > +int > +socket_open (struct serial *scb, const char *name) > +{ > + struct sockaddr_un addr; > + > + memset (&addr, 0, sizeof addr); > + addr.sun_family = AF_UNIX; > +#ifndef UNIX_MAX_PATH > +# define UNIX_MAX_PATH 108 > +#endif gdbserver/tracepoint.c does: #ifndef UNIX_PATH_MAX #define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path) #endif > diff --git a/gdb/serial.c b/gdb/serial.c > index fb2b212918..13b1af3873 100644 > --- a/gdb/serial.c > +++ b/gdb/serial.c > @@ -213,7 +213,15 @@ serial_open (const char *name) > else if (strchr (name, ':')) > ops = serial_interface_lookup ("tcp"); > else > - ops = serial_interface_lookup ("hardwire"); > + { > + /* Check to see if name is a socket. If it is, then treat is > + as such. Otherwise assume that it's a character device. */ > + struct stat sb; > + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) > + ops = serial_interface_lookup ("socket"); > + else > + ops = serial_interface_lookup ("hardware"); Nit: maybe it's just me, but I find "socket" ambiguous -- is it a unix domain socket, a tcp socket, a udp socket, other? I'd have picked "unix" or "uds" instead, and likewise have named the file ser-unix.c and functions unix_foo instead of serial. We already have ser-unix.c, but since this is only for unix really, then we could add the new code in that file instead? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 16:01 ` Pedro Alves @ 2018-08-31 16:40 ` John Darrington 2018-09-03 13:19 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: John Darrington @ 2018-08-31 16:40 UTC (permalink / raw) To: Pedro Alves; +Cc: John Darrington, gdb-patches On Fri, Aug 31, 2018 at 05:00:55PM +0100, Pedro Alves wrote: What server are you using this against? It'd be great to add support for gdbserver as well. Were you planning on doing it? I'm using upad [1], but a version which has not yet been released (the released one doesn't have the necessary features). I wasn't planning to update gdbserver but I could do that when ... If we had that, then I think we could add unix domain socket testing to gdb/testsuite/gdb.server/server-connect.exp (assuming it'd be easy enough to create a socket from tcl). Yes. One of the big advantages of using local sockets in testing as opposed to tcp sockets is that parallel tests become a lot easier. You don't have to worry about port number conflicts or wait times. However to do it right is not altogether straightforward. You'd need gdbserver to have some kind of daemon mode, for example tempdir=$(mkdir -d) gdbserver --socket=$tempdir/mysock --start gdb --iex="target remote $tempdir/mysock" ... gdbserver --socket=$tempdir/mysock --stop rm -rf $tempdir This is because there is a race condition in a server between the bind and listen syscalls. GDB must not attempt to connect until listen has returned successfully. [1] http://www.nongnu.org/micropad gdbserver/tracepoint.c does: #ifndef UNIX_PATH_MAX #define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path) #endif I'm not sure if that's entirely safe. I believe some systems define sun_path as a pointer into a static buffer in the kernel. But if some higher authority can say otherwise I'll defer to them. > diff --git a/gdb/serial.c b/gdb/serial.c > index fb2b212918..13b1af3873 100644 > --- a/gdb/serial.c > +++ b/gdb/serial.c > @@ -213,7 +213,15 @@ serial_open (const char *name) > else if (strchr (name, ':')) > ops = serial_interface_lookup ("tcp"); > else > - ops = serial_interface_lookup ("hardwire"); > + { > + /* Check to see if name is a socket. If it is, then treat is > + as such. Otherwise assume that it's a character device. */ > + struct stat sb; > + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) > + ops = serial_interface_lookup ("socket"); > + else > + ops = serial_interface_lookup ("hardware"); Nit: maybe it's just me, but I find "socket" ambiguous -- is it a unix domain socket, a tcp socket, a udp socket, other? I'd have picked "unix" or "uds" instead, and likewise have named the file ser-unix.c and functions unix_foo instead of serial. We already have ser-unix.c, but since this is only for unix really, then we could add the new code in that file instead? Let's face it, the names of these files are totally anachronistic. ser-tcp.c has nothing to do with serial ports and serial.c does only tangentially. It could use a big rename action ... 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] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-08-31 16:40 ` John Darrington @ 2018-09-03 13:19 ` Pedro Alves 2018-09-03 18:49 ` John Darrington 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2018-09-03 13:19 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On 08/31/2018 05:40 PM, John Darrington wrote: > On Fri, Aug 31, 2018 at 05:00:55PM +0100, Pedro Alves wrote: > > What server are you using this against? It'd be great to add > support for gdbserver as well. Were you planning on doing it? > > I'm using upad [1], but a version which has not yet been released (the > released one doesn't have the necessary features). > I wasn't planning to update gdbserver but I could do that when ... > > If we had that, then I think we could add unix domain socket testing > to gdb/testsuite/gdb.server/server-connect.exp (assuming it'd be easy > enough to create a socket from tcl). > > Yes. One of the big advantages of using local sockets in testing as > opposed to tcp sockets is that parallel tests become a lot easier. That's not what I suggested. The server-connect.exp test does this: # Test multiple types of connection (IPv4, IPv6, TCP, UDP) and make # sure both gdbserver and GDB work. so what I meant was that we could add unix socket testing to that file in order to smoke test unix socket work and continue working, regardless of how the rest of the testsuite is being run. > You don't have to worry about port number conflicts or wait times. > > However to do it right is not altogether straightforward. You'd need > gdbserver to have some kind of daemon mode, for example > > tempdir=$(mkdir -d) > gdbserver --socket=$tempdir/mysock --start > gdb --iex="target remote $tempdir/mysock" ... > gdbserver --socket=$tempdir/mysock --stop > rm -rf $tempdir > > This is because there is a race condition in a server between the > bind and listen syscalls. GDB must not attempt to connect until > listen has returned successfully. Can you clarify how unix sockets are different from tcp sockets here? > > > [1] http://www.nongnu.org/micropad > > > gdbserver/tracepoint.c does: > > #ifndef UNIX_PATH_MAX > #define UNIX_PATH_MAX sizeof(((struct sockaddr_un *) NULL)->sun_path) > #endif > > I'm not sure if that's entirely safe. I believe some systems define > sun_path as a pointer into a static buffer in the kernel. How can userspace have a pointer into kernel memory? > But if some higher authority can say otherwise I'll defer to them. What do you mean by "higher authority"? If you google around a bit, you find references to kernels other than Linux having a limit different than 108. E.g., from: https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars OpenBSD: 104 characters FreeBSD: 104 characters Mac OS X 10.9: 104 characters So hardcoding to 108 seems worse to me. Regardless, seems odd to have different parts of gdb use different fallbacks. Ideally, we'd move the fallback definition to a single place used by both users. > > > diff --git a/gdb/serial.c b/gdb/serial.c > > index fb2b212918..13b1af3873 100644 > > --- a/gdb/serial.c > > +++ b/gdb/serial.c > > @@ -213,7 +213,15 @@ serial_open (const char *name) > > else if (strchr (name, ':')) > > ops = serial_interface_lookup ("tcp"); > > else > > - ops = serial_interface_lookup ("hardwire"); > > + { > > + /* Check to see if name is a socket. If it is, then treat is > > + as such. Otherwise assume that it's a character device. */ > > + struct stat sb; > > + if (0 == stat (name, &sb) && ((sb.st_mode & S_IFMT) == S_IFSOCK)) > > + ops = serial_interface_lookup ("socket"); > > + else > > + ops = serial_interface_lookup ("hardware"); > > > Nit: maybe it's just me, but I find "socket" ambiguous -- is it > a unix domain socket, a tcp socket, a udp socket, other? > I'd have picked "unix" or "uds" instead, and likewise have > named the file ser-unix.c and functions unix_foo instead > of serial. We already have ser-unix.c, but since this is > only for unix really, then we could add the new code in > that file instead? > > Let's face it, the names of these files are totally anachronistic. > ser-tcp.c has nothing to do with serial ports and serial.c does only > tangentially. All these files provide different implementations of serial transports (as opposed to parallel), abstracted behind "struct serial", and to be used with the "remote SERIAL protocol". It's not that tangential. We have three host-specific files named such that their name indicates the host which they are for: "ser-unix.c", "ser-go32.c" and "ser-mingw.c". Then we have host-independent files that are named wrt to the transport they implement internally: "ser-event.c", "ser-pipe.c", "ser-tcp.c". ser-event.c is a bit of an outlier, if you'd like to pick on some case. > It could use a big rename action ... Sure, it could be better. But, is "socket" your ideal choice? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-09-03 13:19 ` Pedro Alves @ 2018-09-03 18:49 ` John Darrington 2018-10-01 19:45 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: John Darrington @ 2018-09-03 18:49 UTC (permalink / raw) To: Pedro Alves; +Cc: John Darrington, gdb-patches Hi Pedro, On Mon, Sep 03, 2018 at 02:18:55PM +0100, Pedro Alves wrote: > Yes. One of the big advantages of using local sockets in testing as > opposed to tcp sockets is that parallel tests become a lot easier. That's not what I suggested. The server-connect.exp test does this: # Test multiple types of connection (IPv4, IPv6, TCP, UDP) and make # sure both gdbserver and GDB work. so what I meant was that we could add unix socket testing to that file in order to smoke test unix socket work and continue working, regardless of how the rest of the testsuite is being run. Oh right. Yes that could be done, but like you said it would involve modifying gdbserver. Can you clarify how unix sockets are different from tcp sockets here? 1. Unix sockets have a (almost) infinite choice of connection points. Whereas TCP sockets you're limited by the port number (usually 16 bits). 2. One can never be sure that a TCP port doesn't already have something listening on it. So starting a server cannot be guaranteed to succeed. Whereas with a Unix socket you can create the directory where the file entry is to reside and know it'll be empty. 3. If a server listening on a TCP socket crashes, then that port number will be unavailable until the kernel's timeout expires (usually after ~ 2 minutes). You cannot restart the server until that happens. There is no way (outside of unloading the kernel's TCP/IP stack) to force the port to become a available again. With Unix sockets, you simply unlink the filename. 4. Unix sockets can only be used for communication between processes on the same host. > > I'm not sure if that's entirely safe. I believe some systems define > sun_path as a pointer into a static buffer in the kernel. How can userspace have a pointer into kernel memory? I recall from Stevens or somewhere that some systems used to define struct socket_un like { int sun_family; char *sun_path; } or something. I don't remember the details of which system it was or how it worked. OpenBSD: 104 characters FreeBSD: 104 characters Mac OS X 10.9: 104 characters So hardcoding to 108 seems worse to me. I thought posix required a minimum of 108. Regardless, seems odd to have different parts of gdb use different fallbacks. Ideally, we'd move the fallback definition to a single place used by both users. I don't mind. If you want me to copy the macro from there, I can do that. All these files provide different implementations of serial transports (as opposed to parallel), abstracted behind "struct serial", and to be used with the "remote SERIAL protocol". It's not that tangential. We have three host-specific files named such that their name indicates the host which they are for: "ser-unix.c", "ser-go32.c" and "ser-mingw.c". Then we have host-independent files that are named wrt to the transport they implement internally: "ser-event.c", "ser-pipe.c", "ser-tcp.c". ser-event.c is a bit of an outlier, if you'd like to pick on some case. > It could use a big rename action ... Sure, it could be better. But, is "socket" your ideal choice? My first choice was ser-unix.c but that is already taken. I really don't have a preference. What would you prefer? ser-local.c perhaps? Thanks for the review. J' ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-09-03 18:49 ` John Darrington @ 2018-10-01 19:45 ` Pedro Alves 2018-10-02 10:16 ` John Darrington 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2018-10-01 19:45 UTC (permalink / raw) To: John Darrington; +Cc: gdb-patches On 09/03/2018 07:48 PM, John Darrington wrote: > Hi Pedro, > > On Mon, Sep 03, 2018 at 02:18:55PM +0100, Pedro Alves wrote: > > > Yes. One of the big advantages of using local sockets in testing as > > opposed to tcp sockets is that parallel tests become a lot easier. > > That's not what I suggested. The server-connect.exp test does this: > > # Test multiple types of connection (IPv4, IPv6, TCP, UDP) and make > # sure both gdbserver and GDB work. > > so what I meant was that we could add unix socket testing to that file > in order to smoke test unix socket work and continue working, regardless > of how the rest of the testsuite is being run. > > Oh right. Yes that could be done, but like you said it would involve > modifying gdbserver. > > Can you clarify how unix sockets are different from tcp sockets here? > > 1. Unix sockets have a (almost) infinite choice of connection points. > Whereas TCP sockets you're limited by the port number (usually 16 bits). > > 2. One can never be sure that a TCP port doesn't already have something > listening on it. So starting a server cannot be guaranteed to succeed. > Whereas with a Unix socket you can create the directory where the file > entry is to reside and know it'll be empty. > > 3. If a server listening on a TCP socket crashes, then that port number > will be unavailable until the kernel's timeout expires (usually after ~ > 2 minutes). You cannot restart the server until that happens. There > is no way (outside of unloading the kernel's TCP/IP stack) to force the > port to become a available again. With Unix sockets, you simply unlink > the filename. > > 4. Unix sockets can only be used for communication between processes on > the same host. What I meant with "different ... here" was, how are unix domain sockets different from tcp sockets with respect to: "This is because there is a race condition in a server between the bind and listen syscalls. GDB must not attempt to connect until listen has returned successfully." If GDB attempts to connect to a tcp gdbserver before it listens/binds, then the connection will fail too. Except, GDB retries the connection, but that would be the case with unix domain sockets too, no? Re. your point 3 above, I don't observe that with gdbserver, maybe because it enables fast reuse with SO_REUSEADDR. > > > > > > I'm not sure if that's entirely safe. I believe some systems define > > sun_path as a pointer into a static buffer in the kernel. > > How can userspace have a pointer into kernel memory? > > I recall from Stevens or somewhere that some systems used to > define struct socket_un like > { > int sun_family; > char *sun_path; > } > > or something. I don't remember the details of which system it was or > how it worked. > > > OpenBSD: 104 characters > FreeBSD: 104 characters > Mac OS X 10.9: 104 characters > > So hardcoding to 108 seems worse to me. > > I thought posix required a minimum of 108. > > Regardless, seems odd to have different parts of gdb use different > fallbacks. Ideally, we'd move the fallback definition to a single > place used by both users. > > I don't mind. If you want me to copy the macro from there, I can do > that. Please do. > > All these files provide different implementations of serial transports > (as opposed to parallel), abstracted behind "struct serial", and to > be used with the "remote SERIAL protocol". It's not that tangential. > > We have three host-specific files named such that their name indicates > the host which they are for: > > "ser-unix.c", "ser-go32.c" and "ser-mingw.c". > > Then we have host-independent files that are named wrt to the > transport they implement internally: > > "ser-event.c", "ser-pipe.c", "ser-tcp.c". > > ser-event.c is a bit of an outlier, if you'd like to > pick on some case. > > > It could use a big rename action ... > > Sure, it could be better. > > But, is "socket" your ideal choice? > > My first choice was ser-unix.c but that is already taken. I really > don't have a preference. What would you prefer? ser-local.c perhaps? > As I had mentioned before, if "unix" is taken, I'd prefer "ser-uds.c", for "Unix Domain Socket", and uds_ as function/symbol prefix. I think UDS is quite established and clearer than "local". I get where it comes from, but note how "local" isn't even ever mentioned anywhere in <https://en.wikipedia.org/wiki/Unix_domain_socket>, for example. I'll take a look at your v3 patch, see if I have extra comments. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Allow remote debugging over a local domain socket 2018-10-01 19:45 ` Pedro Alves @ 2018-10-02 10:16 ` John Darrington 0 siblings, 0 replies; 16+ messages in thread From: John Darrington @ 2018-10-02 10:16 UTC (permalink / raw) To: Pedro Alves; +Cc: John Darrington, gdb-patches Hi Pedro, Nice to hear from you. On Mon, Oct 01, 2018 at 08:45:52PM +0100, Pedro Alves wrote: > Can you clarify how unix sockets are different from tcp sockets here? > > 1. Unix sockets have a (almost) infinite choice of connection points. > Whereas TCP sockets you're limited by the port number (usually 16 bits). > > 2. One can never be sure that a TCP port doesn't already have something > listening on it. So starting a server cannot be guaranteed to succeed. > Whereas with a Unix socket you can create the directory where the file > entry is to reside and know it'll be empty. > > 3. If a server listening on a TCP socket crashes, then that port number > will be unavailable until the kernel's timeout expires (usually after ~ > 2 minutes). You cannot restart the server until that happens. There > is no way (outside of unloading the kernel's TCP/IP stack) to force the > port to become a available again. With Unix sockets, you simply unlink > the filename. > > 4. Unix sockets can only be used for communication between processes on > the same host. What I meant with "different ... here" was, how are unix domain sockets different from tcp sockets with respect to: "This is because there is a race condition in a server between the bind and listen syscalls. GDB must not attempt to connect until listen has returned successfully." If GDB attempts to connect to a tcp gdbserver before it listens/binds, then the connection will fail too. Except, GDB retries the connection, but that would be the case with unix domain sockets too, no? You are right. There is no difference in this respect. My only reason for mentioning it was that it is the only problem affecting concurrent gdb/gdbserver sessions which is *not* solved by the use of unix domain sockets. > > I don't mind. If you want me to copy the macro from there, I can do > that. Please do. Done. > My first choice was ser-unix.c but that is already taken. I really > don't have a preference. What would you prefer? ser-local.c perhaps? > As I had mentioned before, if "unix" is taken, I'd prefer "ser-uds.c", for "Unix Domain Socket", and uds_ as function/symbol prefix. I think UDS is quite established and clearer than "local". I get where it comes from, but note how "local" isn't even ever mentioned anywhere in <https://en.wikipedia.org/wiki/Unix_domain_socket>, for example. I don't remember you suggesting ser-uds.c before. But no problem - I will make that change. Thanks for your comments. I will attend to them all tonight and check it in. J' ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-10-02 10:16 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-29 7:05 Remote debugging over local domain sockets? John Darrington 2018-08-29 15:17 ` Tom Tromey 2018-08-29 15:28 ` John Darrington 2018-08-29 15:39 ` Tom Tromey 2018-08-31 10:18 ` [PATCH] Allow remote debugging over a local domain socket John Darrington 2018-08-31 14:59 ` Eli Zaretskii 2018-08-31 15:10 ` John Darrington 2018-08-31 17:50 ` Eli Zaretskii 2018-08-31 15:10 ` Tom Tromey 2018-08-31 15:12 ` John Darrington 2018-08-31 16:01 ` Pedro Alves 2018-08-31 16:40 ` John Darrington 2018-09-03 13:19 ` Pedro Alves 2018-09-03 18:49 ` John Darrington 2018-10-01 19:45 ` Pedro Alves 2018-10-02 10:16 ` John Darrington
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).