From patchwork Tue Dec 24 04:42:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stan Cox X-Patchwork-Id: 37083 Received: (qmail 50456 invoked by alias); 24 Dec 2019 04:42:54 -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 50447 invoked by uid 89); 24 Dec 2019 04:42:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=1.4, 1.5 X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Dec 2019 04:42:51 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1577162569; 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=azNOL0xO/5tqn2mrBnXz6VF0jrL3Shss3Hpoa9k19mg=; b=TLyenEgOt487ei5OBRBVh3uSrJIMfX4PCV/lL23Q9uBW27ieTsWCRg1WrOKpNiU8OBM7Va X9xJ0AfJdBaYOvbfmiYWQX1ctsX37jRjHXrWEwZMN1nawesnPyp3GwW3Tf1oIDt9nS7Gf2 +MwnKO7Bwdpmv/zo4TpqdqxaenfT/HE= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-144-YM_N-Zq0MnScjTTzJbQ_8Q-1; Mon, 23 Dec 2019 23:42:08 -0500 Received: by mail-qt1-f199.google.com with SMTP id l25so12436101qtu.0 for ; Mon, 23 Dec 2019 20:42:08 -0800 (PST) Return-Path: Received: from ?IPv6:2606:a000:44c6:c900::1? (2606-a000-44c6-c900-0000-0000-0000-0001.inf6.spectrum.com. [2606:a000:44c6:c900::1]) by smtp.gmail.com with ESMTPSA id a24sm6469505qkl.82.2019.12.23.20.42.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Dec 2019 20:42:06 -0800 (PST) Subject: Re: [PATCH] add file desc to gdbserver client_state 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> From: Stan Cox Message-ID: <827a8c5f-e797-58a3-62bf-335e7a44cd9a@redhat.com> Date: Mon, 23 Dec 2019 23:42:05 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 MIME-Version: 1.0 In-Reply-To: <87fthnvmxz.fsf@tromey.com> X-Mimecast-Spam-Score: 0 This patch adds gdb_fildes_t support to gdbserver for later use by the gdbserver multi-client patch. That patch enables multiple clients with each client associated with its corresponding file desc. This patch adds the corresponding file desc to the client_state and adds a file desc parameter to the gdbserver I/O ops. It also adds a minimal multi_client_states which currently just manages a single client_state but later will be expanded to be a collection of multiple client_states where each client_state is the state for an individual client. In contrast to the previous patch, this patch 1) uses client_state.remote_desc in remote-utils.c instead of the static version 2) no longer uses the function get_client_state but instead defines get_client_state using multi_client_states The gdbserver multi-client patch is mentioned in: https://sourceware.org/ml/gdb-patches/2019-08/msg00577.html > Will remote_desc be going away? And hopefully get_remote_desc is also > just a stepping stone? remote_desc is now gone as mentioned in 1 above > Can get_client_state be made static now? > If not, why not? get_client_state is now redefined as mentioned in 2) above Thanks Tom! commit bac0aa21ec (HEAD -> master) Author: Stan Cox Date: Fri Dec 20 15:54:22 2019 -0500 Add struct multi_client_states * remote-utils.c (remote_desc): Move to struct client_state (gdb_connected, handle_accept_event, remote_open, putpkt_binary_1) (putpkt_notif, input_interrupt, block_unblock_async_io) (nto_conctrl, getpkt): Change remote_desc reference. * server.c (captured_main): Move remote_desc initialization to remote_prepare. --- gdb/gdbserver/ChangeLog | 12 ++++++++++++ gdb/gdbserver/remote-utils.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------- gdb/gdbserver/remote-utils.h | 2 ++ gdb/gdbserver/server.c | 15 ++++++++++++++- gdb/gdbserver/server.h | 25 ++++++++++++++++++++++++- 5 files changed, 100 insertions(+), 35 deletions(-) diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 02414385b7..bf43b232f5 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,15 @@ +2019-12-23 Stan Cox + + * server.h (struct multi_client_states): New. + * server.c (g_client_states) + (get_client_states, multi_client_sates::set_client_state): New. + (captured_main): Set the initial multi_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. + 2019-11-22 Tom Tromey * gdb.ada/tasks.exp: Add -ada-task-info regression test. diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index d7da4b7aed..09095d9bbc 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -25,6 +25,7 @@ #include "tdesc.h" #include "debug.h" #include "dll.h" +#include "server.h" #include "gdbsupport/rsp-low.h" #include "gdbsupport/netstuff.h" #include "gdbsupport/filestuff.h" @@ -93,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); @@ -107,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 @@ -115,10 +115,12 @@ static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR; # define write(fd, buf, len) send (fd, (char *) buf, len, 0) #endif + 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. */ @@ -148,6 +150,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"); @@ -156,6 +159,9 @@ handle_accept_event (int err, gdb_client_data client_data) if (remote_desc == -1) perror_with_name ("Accept failed"); + struct multi_client_states &client_states = get_client_states(); + client_states.set_client_state (remote_desc); + /* Enable TCP keep alive process. */ socklen_t tmp = 1; setsockopt (remote_desc, SOL_SOCKET, SO_KEEPALIVE, @@ -268,6 +274,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); @@ -324,6 +332,7 @@ void remote_open (const char *name) { const char *port_str; + gdb_fildes_t remote_desc; port_str = strchr (name, ':'); #ifdef USE_WIN32API @@ -416,17 +425,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 (); } @@ -607,12 +618,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. @@ -620,12 +631,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. @@ -666,7 +677,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); @@ -679,9 +690,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; @@ -689,11 +700,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) { @@ -748,6 +759,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 }; @@ -755,13 +767,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) { @@ -786,10 +798,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); @@ -815,6 +828,7 @@ block_unblock_async_io (int block) static void nto_comctrl (int enable) { + client_state &cs = get_client_state (); struct sigevent event; if (enable) @@ -824,11 +838,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__ */ @@ -891,13 +905,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) { @@ -972,6 +986,7 @@ getpkt (char *buf) char *bp; unsigned char csum, c1, c2; int c; + gdb_fildes_t fd = cs.remote_desc; while (1) { @@ -979,7 +994,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. */ @@ -1004,7 +1019,7 @@ getpkt (char *buf) bp = buf; while (1) { - c = readchar (); + c = readchar (fd); if (c < 0) return -1; if (c == '#') @@ -1014,8 +1029,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; @@ -1032,7 +1047,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; } @@ -1040,11 +1055,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) @@ -1057,7 +1072,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 (); } } @@ -1074,7 +1089,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/remote-utils.h b/gdb/gdbserver/remote-utils.h index 4ca5d9435e..606592852c 100644 --- a/gdb/gdbserver/remote-utils.h +++ b/gdb/gdbserver/remote-utils.h @@ -19,6 +19,8 @@ #ifndef GDBSERVER_REMOTE_UTILS_H #define GDBSERVER_REMOTE_UTILS_H +int get_remote_desc (void); + int gdb_connected (void); #define STDIO_CONNECTION_NAME "stdio" diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index c5f7176cff..40b8ea1b80 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -153,11 +153,21 @@ 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; +} client_state & -get_client_state () +multi_client_states::set_client_state (gdb_fildes_t fd) { client_state &cs = g_client_state; + + cs.remote_desc = fd; + get_client_states().set_current_client (cs); return cs; } @@ -3556,6 +3566,9 @@ captured_main (int argc, char *argv[]) #endif current_directory = getcwd (NULL, 0); + + multi_client_states &client_states = get_client_states (); + client_states.set_client_state (-1); client_state &cs = get_client_state (); if (current_directory == NULL) diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index e01c4f146e..027adbfe63 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -142,6 +142,8 @@ struct client_state 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 @@ -200,7 +202,28 @@ 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: + client_state *current_cs; + +public: + /* Return the current client we are focused on. */ + client_state &get_current_client () { return *current_cs; } + + /* Set the current client we wish to focus on. */ + void set_current_client (client_state &cs) { current_cs = &cs; } + + /* Set, or create if necessary, the current client we wish to focus on */ + client_state &set_client_state (gdb_fildes_t); +}; + +struct multi_client_states &get_client_states (); #include "gdbthread.h" #include "inferiors.h"