From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128021 invoked by alias); 3 Sep 2018 13:19:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 128000 invoked by uid 89); 3 Sep 2018 13:19:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=authority, picked, listen, uds X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Sep 2018 13:18:59 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 762164023ECB; Mon, 3 Sep 2018 13:18:57 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2AFA22026D6B; Mon, 3 Sep 2018 13:18:56 +0000 (UTC) Subject: Re: [PATCH] Allow remote debugging over a local domain socket To: John Darrington References: <874lfd5gjt.fsf@tromey.com> <20180831101818.9175-1-john@darrington.wattle.id.au> <61e78be6-4976-6a28-90d2-e515c0afc2f3@redhat.com> <20180831164001.jcejryh3v7fqtsd3@jocasta.intra> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <39de0b1a-eba6-2325-f76e-9dfb54ebd88d@redhat.com> Date: Mon, 03 Sep 2018 13:19:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180831164001.jcejryh3v7fqtsd3@jocasta.intra> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-09/txt/msg00017.txt.bz2 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