From patchwork Thu Feb 6 19:41:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stan Cox X-Patchwork-Id: 37715 Received: (qmail 119709 invoked by alias); 6 Feb 2020 19:41:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 119653 invoked by uid 89); 6 Feb 2020 19:41:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=manages, xmalloc, disconnected, multi-process X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Feb 2020 19:41:48 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581018106; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=NcMW1RJiPWslk9qenZGiizSPbgUhxGSnia5c8CayQGc=; b=cpULgQfI0Rhdi+r7E8oT7Uu+4NHa3EiXJ5GInxi8k0yJKbcGPsIZfTJjFaC/0MvHsM1Z7+ M/K8l8mFc5++snIbN5S4spvsYnQgwSoZVnH8Kr01HnwFwfSEtaaAMjXKVZq6XdcTE3smfO MIcbyN9omqVtQnDNRGRfrILYg2dGFWc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-82-2jOobvRfNlCfMT6Br3plgw-1; Thu, 06 Feb 2020 14:41:41 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EB7B8100551A; Thu, 6 Feb 2020 19:41:40 +0000 (UTC) Received: from [10.13.129.218] (dhcp129-218.rdu.redhat.com [10.13.129.218]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8AC54857B4; Thu, 6 Feb 2020 19:41:40 +0000 (UTC) Subject: Re: [PATCH] add file desc to gdbserver client_state From: Stan Cox To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20191122233003.211567-1-cbiesinger@google.com> <8448b37f-ebdf-d2d9-829a-87f7f6ea102c@redhat.com> <87fthnvmxz.fsf@tromey.com> <827a8c5f-e797-58a3-62bf-335e7a44cd9a@redhat.com> Message-ID: Date: Thu, 6 Feb 2020 14:41:40 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <827a8c5f-e797-58a3-62bf-335e7a44cd9a@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com This patch adds struct multi_client_states for managing multiple clients and adds gdb_fildes_t support to routines write_prim, read_prim, readchar for handling client specific io,with each client associated with its corresponding file desc. Thispatch adds the corresponding file desc to the client_state and adds afile desc parameter to the gdbserver I/O ops. It also adds structmulti_client_states which currently just manages a primary client_statebut will later allow managing a collection of client_states where each client_state is the state for an individualclient. This patch defines get_client_state using multi_client_statesdefines. Concurrent with this patch is an strace patch that adds a gdbserver backend to strace. Add struct multi_client_states * server.h (struct multi_client_states): New. * server.c (g_client_states) (get_client_states, multi_client_sates::add_client_state): New. (captured_main): Set multi_client_states.client_states * remote-utils.c (gdb_connected, remote_prepare, remote_close) (putpkt_binary_1, getpkt) Use client_state.remote_desc. (handle_accept_event): Set the client state for this remote file desc. (write_prim, read_prim, readchar): Add gdb_fildes_t parameter. Change all callers. Add struct multi_client_states * server.h (struct multi_client_states): New. * server.c (g_client_states) (get_client_states, multi_client_sates::add_client_state): New. (captured_main): Set multi_client_states.client_states * remote-utils.c (gdb_connected, remote_prepare, remote_close) (putpkt_binary_1, getpkt) Use client_state.remote_desc. (handle_accept_event): Set the client state for this remote file desc. (write_prim, read_prim, readchar): Add gdb_fildes_t parameter. Change all callers. --- gdb/gdbserver/ChangeLog | 12 ++++++++++++ gdb/gdbserver/remote-utils.c | 78 +++++++++++++++++++++++++++++++++++++++++++++--------------------------------- gdb/gdbserver/server.c | 28 +++++++++++++++++++++++----- gdb/gdbserver/server.h | 31 +++++++++++++++++++++++++++++-- 4 files changed, 109 insertions(+), 40 deletions(-) diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index b8a8c6576f9..63d3cad9597 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -94,7 +94,7 @@ enum { Either NOT_SCHEDULED or the callback id. */ static int readchar_callback = NOT_SCHEDULED; -static int readchar (void); +static int readchar (gdb_fildes_t); static void reset_readchar (void); static void reschedule (void); @@ -108,7 +108,6 @@ struct sym_cache static int remote_is_stdio = 0; -static gdb_fildes_t remote_desc = INVALID_DESCRIPTOR; static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR; #ifdef USE_WIN32API @@ -119,7 +118,8 @@ static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR; int gdb_connected (void) { - return remote_desc != INVALID_DESCRIPTOR; + client_state &cs = get_client_state (); + return cs.remote_desc != INVALID_DESCRIPTOR; } /* Return true if the remote connection is over stdio. */ @@ -149,6 +149,7 @@ handle_accept_event (int err, gdb_client_data client_data) { struct sockaddr_storage sockaddr; socklen_t len = sizeof (sockaddr); + gdb_fildes_t remote_desc; if (debug_threads) debug_printf ("handling possible accept event\n"); @@ -157,6 +158,8 @@ handle_accept_event (int err, gdb_client_data client_data) if (remote_desc == -1) perror_with_name ("Accept failed"); + get_client_states ().add_client_state (remote_desc); + /* Enable TCP keep alive process. */ socklen_t tmp = 1; setsockopt (remote_desc, SOL_SOCKET, SO_KEEPALIVE, @@ -269,6 +272,8 @@ remote_prepare (const char *name) } #endif + cs.remote_desc = INVALID_DESCRIPTOR; + int r = getaddrinfo (parsed.host_str.c_str (), parsed.port_str.c_str (), &hint, &ainfo); @@ -325,6 +330,7 @@ void remote_open (const char *name) { const char *port_str; + gdb_fildes_t remote_desc; port_str = strchr (name, ':'); #ifdef USE_WIN32API @@ -417,17 +423,19 @@ remote_open (const char *name) void remote_close (void) { - delete_file_handler (remote_desc); + client_state &cs = get_client_state (); + + delete_file_handler (cs.remote_desc); disable_async_io (); #ifdef USE_WIN32API - closesocket (remote_desc); + closesocket (cs.remote_desc); #else if (! remote_connection_is_stdio ()) - close (remote_desc); + close (cs.remote_desc); #endif - remote_desc = INVALID_DESCRIPTOR; + cs.remote_desc = INVALID_DESCRIPTOR; reset_readchar (); } @@ -608,12 +616,12 @@ read_ptid (const char *buf, const char **obuf) This may return less than COUNT. */ static int -write_prim (const void *buf, int count) +write_prim (gdb_fildes_t fd, const void *buf, int count) { if (remote_connection_is_stdio ()) return write (fileno (stdout), buf, count); else - return write (remote_desc, buf, count); + return write (fd, buf, count); } /* Read COUNT bytes from the client and store in BUF. @@ -621,12 +629,12 @@ write_prim (const void *buf, int count) This may return less than COUNT. */ static int -read_prim (void *buf, int count) +read_prim (gdb_fildes_t fd, void *buf, int count) { if (remote_connection_is_stdio ()) return read (fileno (stdin), buf, count); else - return read (remote_desc, buf, count); + return read (fd, buf, count); } /* Send a packet to the remote machine, with error checking. @@ -667,7 +675,7 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif) do { - if (write_prim (buf2, p - buf2) != p - buf2) + if (write_prim (cs.remote_desc, buf2, p - buf2) != p - buf2) { perror ("putpkt(write)"); free (buf2); @@ -680,9 +688,9 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif) if (remote_debug) { if (is_notif) - debug_printf ("putpkt (\"%s\"); [notif]\n", buf2); + debug_printf ("putpkt (%d, \"%s\"); [notif]\n", cs.remote_desc, buf2); else - debug_printf ("putpkt (\"%s\"); [noack mode]\n", buf2); + debug_printf ("putpkt (%d, \"%s\"); [noack mode]\n", cs.remote_desc, buf2); debug_flush (); } break; @@ -690,11 +698,11 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif) if (remote_debug) { - debug_printf ("putpkt (\"%s\"); [looking for ack]\n", buf2); + debug_printf ("putpkt (%d, \"%s\"); [looking for ack]\n", cs.remote_desc, buf2); debug_flush (); } - cc = readchar (); + cc = readchar (cs.remote_desc); if (cc < 0) { @@ -749,6 +757,7 @@ putpkt_notif (char *buf) static void input_interrupt (int unused) { + client_state &cs = get_client_state (); fd_set readset; struct timeval immediate = { 0, 0 }; @@ -756,13 +765,13 @@ input_interrupt (int unused) be a problem under NetBSD 1.4 and 1.5. */ FD_ZERO (&readset); - FD_SET (remote_desc, &readset); - if (select (remote_desc + 1, &readset, 0, 0, &immediate) > 0) + FD_SET (cs.remote_desc, &readset); + if (select (cs.remote_desc + 1, &readset, 0, 0, &immediate) > 0) { int cc; char c = 0; - cc = read_prim (&c, 1); + cc = read_prim (cs.remote_desc, &c, 1); if (cc == 0) { @@ -787,10 +796,11 @@ input_interrupt (int unused) void check_remote_input_interrupt_request (void) { + client_state &cs = get_client_state (); /* This function may be called before establishing communications, therefore we need to validate the remote descriptor. */ - if (remote_desc == INVALID_DESCRIPTOR) + if (cs.remote_desc == INVALID_DESCRIPTOR) return; input_interrupt (0); @@ -816,6 +826,7 @@ block_unblock_async_io (int block) static void nto_comctrl (int enable) { + client_state &cs = get_client_state (); struct sigevent event; if (enable) @@ -825,11 +836,11 @@ nto_comctrl (int enable) event.sigev_code = 0; event.sigev_value.sival_ptr = NULL; event.sigev_priority = -1; - ionotify (remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT, + ionotify (cs.remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT, &event); } else - ionotify (remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL); + ionotify (cs.remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL); } #endif /* __QNX__ */ @@ -892,13 +903,13 @@ static unsigned char *readchar_bufp; /* Returns next char from remote GDB. -1 if error. */ static int -readchar (void) +readchar (gdb_fildes_t fd) { int ch; if (readchar_bufcnt == 0) { - readchar_bufcnt = read_prim (readchar_buf, sizeof (readchar_buf)); + readchar_bufcnt = read_prim (fd, readchar_buf, sizeof (readchar_buf)); if (readchar_bufcnt <= 0) { @@ -973,6 +984,7 @@ getpkt (char *buf) char *bp; unsigned char csum, c1, c2; int c; + gdb_fildes_t fd = cs.remote_desc; while (1) { @@ -980,7 +992,7 @@ getpkt (char *buf) while (1) { - c = readchar (); + c = readchar (fd); /* The '\003' may appear before or after each packet, so check for an input interrupt. */ @@ -1005,7 +1017,7 @@ getpkt (char *buf) bp = buf; while (1) { - c = readchar (); + c = readchar (fd); if (c < 0) return -1; if (c == '#') @@ -1015,8 +1027,8 @@ getpkt (char *buf) } *bp = 0; - c1 = fromhex (readchar ()); - c2 = fromhex (readchar ()); + c1 = fromhex (readchar (fd)); + c2 = fromhex (readchar (fd)); if (csum == (c1 << 4) + c2) break; @@ -1033,7 +1045,7 @@ getpkt (char *buf) fprintf (stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n", (c1 << 4) + c2, csum, buf); - if (write_prim ("-", 1) != 1) + if (write_prim (fd, "-", 1) != 1) return -1; } @@ -1041,11 +1053,11 @@ getpkt (char *buf) { if (remote_debug) { - debug_printf ("getpkt (\"%s\"); [sending ack] \n", buf); + debug_printf ("getpkt (%d, \"%s\"); [sending ack] \n", fd, buf); debug_flush (); } - if (write_prim ("+", 1) != 1) + if (write_prim (fd, "+", 1) != 1) return -1; if (remote_debug) @@ -1058,7 +1070,7 @@ getpkt (char *buf) { if (remote_debug) { - debug_printf ("getpkt (\"%s\"); [no ack sent] \n", buf); + debug_printf ("getpkt (%d, \"%s\"); [no ack sent] \n", fd, buf); debug_flush (); } } @@ -1075,7 +1087,7 @@ getpkt (char *buf) while (readchar_bufcnt > 0 && *readchar_bufp == '\003') { /* Consume the interrupt character in the buffer. */ - readchar (); + readchar (fd); (*the_target->request_interrupt) (); } diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 3fc026f78eb..8a5f23f9824 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -152,13 +152,30 @@ static struct btrace_config current_btrace_conf; /* The client remote protocol state. */ -static client_state g_client_state; +static struct multi_client_states g_client_states; + +multi_client_states & +get_client_states () +{ + return g_client_states; +} + + +/* Add a new client state for a corresponding file descriptor fd */ client_state & -get_client_state () +multi_client_states::add_client_state (gdb_fildes_t fd) { - client_state &cs = g_client_state; - return cs; + if (fd == -1) + /* This is an initial client state setup in captured main */ + client_states[fd] = client_state (); + else + /* Initialize this client state from the initial client state. + * If fd was disconnected / reconnected then it is reloaded. + */ + client_states[fd] = client_states[-1]; + client_states[fd].remote_desc = fd; + return set_current_client (fd); } @@ -3556,7 +3573,8 @@ captured_main (int argc, char *argv[]) #endif current_directory = getcwd (NULL, 0); - client_state &cs = get_client_state (); + + client_state &cs = get_client_states ().add_client_state (-1); if (current_directory == NULL) { diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index 3c286862349..6c2b70e8004 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -107,6 +107,7 @@ extern int in_queued_stop_replies (ptid_t ptid); #include "utils.h" #include "debug.h" #include "gdbsupport/gdb_vecs.h" +#include /* Maximum number of bytes to read/write at once. The value here is chosen to fill up a packet (the headers account for the 32). */ @@ -141,9 +142,11 @@ extern unsigned long signal_pid; struct client_state { client_state (): - own_buf ((char *) xmalloc (PBUFSIZ + 1)) + own_buf ((char *) xmalloc (PBUFSIZ + 1)) {} + gdb_fildes_t remote_desc; + /* The thread set with an `Hc' packet. `Hc' is deprecated in favor of `vCont'. Note the multi-process extensions made `vCont' a requirement, so `Hc pPID.TID' is pretty much undefined. So @@ -202,7 +205,31 @@ struct client_state }; -client_state &get_client_state (); +/* Container of client remote protocol states for all the currently + connected clients. */ + +#define get_client_state() get_client_states ().get_current_client () + +class multi_client_states +{ +private: + /* Mapping from the file descriptor to the associated client state */ + std::map client_states; + /* The file descriptor of the current client we are focused on */ + gdb_fildes_t current_fd; + +public: + /* Return the current client we are focused on. */ + client_state &get_current_client () { return client_states[current_fd]; } + + /* Set the current client we wish to focus on. */ + client_state &set_current_client (gdb_fildes_t fd) { current_fd = fd; return client_states[current_fd]; } + + /* Add the current client we wish to focus on */ + client_state &add_client_state (gdb_fildes_t fd); +}; + +struct multi_client_states &get_client_states (); #include "gdbthread.h" #include "inferiors.h"