Re: [PATCH] Allow remote debugging over a local domain socket

John Darrington
2018-08-31T16:40:11Z

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.