From patchwork Sun Jan 12 19:57:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 37334 Received: (qmail 111972 invoked by alias); 12 Jan 2020 19:57:47 -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 111963 invoked by uid 89); 12 Jan 2020 19:57:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=8088, Needed, iteration, sk:insert_ X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 12 Jan 2020 19:57:44 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id AFEEF1E4C2; Sun, 12 Jan 2020 14:57:42 -0500 (EST) Subject: Re: [PATCH v2 18/24] Multi-target support To: Pedro Alves , gdb-patches@sourceware.org, Andrew Burgess References: <20191017225026.30496-1-palves@redhat.com> <20191017225026.30496-19-palves@redhat.com> From: Simon Marchi Message-ID: <113c6953-3621-c61b-e318-19806805e878@simark.ca> Date: Sun, 12 Jan 2020 14:57:41 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <20191017225026.30496-19-palves@redhat.com> On 2019-10-17 6:50 p.m., Pedro Alves wrote: > This commit adds multi-target support to GDB. What this means is that > with this commit, GDB can now be connected to different targets at the > same time. E.g., you can debug a live native process and a core dump > at the same time, connect to multiple gdbservers, etc. > > ... remote-sim.c needs to be updated to. The patch below makes it build, although I have not tried it (I don't have time right now to re-figure out again how do do it :)). From 3342486f68499477f38d90c775f496846cb0d65e Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sun, 12 Jan 2020 14:30:18 -0500 Subject: [PATCH] gdb: adjust remote-sim.c to multi-target The remote-sim.c file doesn't build since the main multi-target patch (5b6d1e4f, "Multi-target support"), this patch is an attempt to fix it. I have only build-tested it, so I'm not sure it runs fine, but it should get us close at least. I made these functions methods of the gdbsim_target, because they need to pass the target down to some GDB core functions, like find_inferior_ptid: - get_sim_inferior_data_by_ptid (renamed to get_inferior_data_by_ptid) - gdbsim_resume_inferior (renamed to resume_one_inferior) - gdbsim_close_inferior (renamed to close_one_inferior) In the last two, I changed iterate_over_inferiors to a range-based for, since that gives simpler code (no need to pass data through the void pointer). The next_pid variable, INITIAL_PID macro and sim_inferior_data structure are simply moved up in the file, above gdbsim_target. gdb/ChangeLog: * remote-sim.c (next_pid, INITIAL_PID, sim_inferior_data): Move up. (gdbsim_target) : New methods. (get_sim_inferior_data_by_ptid): Move to gdbsim_target, pass down target to find_inferior_pid. (gdbsim_target::fetch_registers, gdbsim_target::store_registers): Pass down target to find_inferior_ptid. (gdbsim_target::create_inferior): Pass down target to add_thread_silent. (gdbsim_close_inferior): Move to gdbsim_close_inferior, pass target down to find_inferior_ptid and switch_to_thread. (gdbsim_target::close): Update to call close_one_inferior. (struct resume_data): Remove. (gdbsim_resume_inferior): Move to gdbsim_target. Take arguments directly, rather than through a void pointer. (gdbsim_target::resume): Update to call resume_one_inferior. --- gdb/remote-sim.c | 146 +++++++++++++++++++++++------------------------ 1 file changed, 70 insertions(+), 76 deletions(-) diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c index 582d206d60c7..533c0d6a9e12 100644 --- a/gdb/remote-sim.c +++ b/gdb/remote-sim.c @@ -74,6 +74,43 @@ static void gdb_os_error (host_callback *, const char *, ...) sim_* are the interface to the simulator (see remote-sim.h). gdbsim_* are stuff which is internal to gdb. */ +/* Value of the next pid to allocate for an inferior. As indicated + elsewhere, its initial value is somewhat arbitrary; it's critical + though that it's not zero or negative. */ +static int next_pid; +#define INITIAL_PID 42000 + +/* Simulator-specific, per-inferior state. */ +struct sim_inferior_data { + explicit sim_inferior_data (SIM_DESC desc) + : gdbsim_desc (desc), + remote_sim_ptid (next_pid, 0, next_pid) + { + ++next_pid; + } + + ~sim_inferior_data (); + + /* Flag which indicates whether or not the program has been loaded. */ + int program_loaded = 0; + + /* Simulator descriptor for this inferior. */ + SIM_DESC gdbsim_desc; + + /* This is the ptid we use for this particular simulator instance. Its + value is somewhat arbitrary, as the simulator target don't have a + notion of tasks or threads, but we need something non-null to place + in inferior_ptid. For simulators which permit multiple instances, + we also need a unique identifier to use for each inferior. */ + ptid_t remote_sim_ptid; + + /* Signal with which to resume. */ + enum gdb_signal resume_siggnal = GDB_SIGNAL_0; + + /* Flag which indicates whether resume should step or not. */ + int resume_step = 0; +}; + static const target_info gdbsim_target_info = { "sim", N_("simulator"), @@ -126,47 +163,16 @@ struct gdbsim_target final bool has_all_memory () override; bool has_memory () override; + +private: + sim_inferior_data *get_inferior_data_by_ptid (ptid_t ptid, + int sim_instance_needed); + void resume_one_inferior (inferior *inf, bool step, gdb_signal siggnal); + void close_one_inferior (inferior *inf); }; static struct gdbsim_target gdbsim_ops; -/* Value of the next pid to allocate for an inferior. As indicated - elsewhere, its initial value is somewhat arbitrary; it's critical - though that it's not zero or negative. */ -static int next_pid; -#define INITIAL_PID 42000 - -/* Simulator-specific, per-inferior state. */ -struct sim_inferior_data { - explicit sim_inferior_data (SIM_DESC desc) - : gdbsim_desc (desc), - remote_sim_ptid (next_pid, 0, next_pid) - { - ++next_pid; - } - - ~sim_inferior_data (); - - /* Flag which indicates whether or not the program has been loaded. */ - int program_loaded = 0; - - /* Simulator descriptor for this inferior. */ - SIM_DESC gdbsim_desc; - - /* This is the ptid we use for this particular simulator instance. Its - value is somewhat arbitrary, as the simulator target don't have a - notion of tasks or threads, but we need something non-null to place - in inferior_ptid. For simulators which permit multiple instances, - we also need a unique identifier to use for each inferior. */ - ptid_t remote_sim_ptid; - - /* Signal with which to resume. */ - enum gdb_signal resume_siggnal = GDB_SIGNAL_0; - - /* Flag which indicates whether resume should step or not. */ - int resume_step = 0; -}; - static inferior_key sim_inferior_data_key; /* Flag indicating the "open" status of this module. It's set to 1 @@ -262,8 +268,9 @@ get_sim_inferior_data (struct inferior *inf, int sim_instance_needed) inferior in question. Return NULL when no inferior is found or when ptid has a zero or negative pid component. */ -static struct sim_inferior_data * -get_sim_inferior_data_by_ptid (ptid_t ptid, int sim_instance_needed) +sim_inferior_data * +gdbsim_target::get_inferior_data_by_ptid (ptid_t ptid, + int sim_instance_needed) { struct inferior *inf; int pid = ptid.pid (); @@ -271,7 +278,7 @@ get_sim_inferior_data_by_ptid (ptid_t ptid, int sim_instance_needed) if (pid <= 0) return NULL; - inf = find_inferior_pid (pid); + inf = find_inferior_pid (this, pid); if (inf) return get_sim_inferior_data (inf, sim_instance_needed); @@ -441,7 +448,7 @@ void gdbsim_target::fetch_registers (struct regcache *regcache, int regno) { struct gdbarch *gdbarch = regcache->arch (); - struct inferior *inf = find_inferior_ptid (regcache->ptid ()); + struct inferior *inf = find_inferior_ptid (this, regcache->ptid ()); struct sim_inferior_data *sim_data = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED); @@ -510,7 +517,7 @@ void gdbsim_target::store_registers (struct regcache *regcache, int regno) { struct gdbarch *gdbarch = regcache->arch (); - struct inferior *inf = find_inferior_ptid (regcache->ptid ()); + struct inferior *inf = find_inferior_ptid (this, regcache->ptid ()); struct sim_inferior_data *sim_data = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED); @@ -656,7 +663,7 @@ gdbsim_target::create_inferior (const char *exec_file, inferior_ptid = sim_data->remote_sim_ptid; inferior_appeared (current_inferior (), inferior_ptid.pid ()); - add_thread_silent (inferior_ptid); + add_thread_silent (this, inferior_ptid); insert_breakpoints (); /* Needed to get correct instruction in cache. */ @@ -770,8 +777,8 @@ gdbsim_target_open (const char *args, int from_tty) /* Callback for iterate_over_inferiors. Called (indirectly) by gdbsim_close(). */ -static int -gdbsim_close_inferior (struct inferior *inf, void *arg) +void +gdbsim_target::close_one_inferior (inferior *inf) { struct sim_inferior_data *sim_data = sim_inferior_data_key.get (inf); if (sim_data != NULL) @@ -785,14 +792,12 @@ gdbsim_close_inferior (struct inferior *inf, void *arg) Thus we need to verify the existence of an inferior using the pid in question before setting inferior_ptid via switch_to_thread() or mourning the inferior. */ - if (find_inferior_ptid (ptid) != NULL) + if (find_inferior_ptid (this, ptid) != NULL) { - switch_to_thread (ptid); + switch_to_thread (this, ptid); generic_mourn_inferior (); } } - - return 0; } /* Close out all files and local state before this target loses control. */ @@ -803,7 +808,8 @@ gdbsim_target::close () if (remote_debug) fprintf_unfiltered (gdb_stdlog, "gdbsim_close\n"); - iterate_over_inferiors (gdbsim_close_inferior, NULL); + for (inferior *inf : all_inferiors (this)) + close_one_inferior (inf); if (sim_argv != NULL) { @@ -839,45 +845,30 @@ gdbsim_target::detach (inferior *inf, int from_tty) or to run free; SIGGNAL is the signal value (e.g. SIGINT) to be given to the target, or zero for no signal. */ -struct resume_data -{ - enum gdb_signal siggnal; - int step; -}; - -static int -gdbsim_resume_inferior (struct inferior *inf, void *arg) +void +gdbsim_target::resume_one_inferior (inferior *inf, bool step, + gdb_signal siggnal) { struct sim_inferior_data *sim_data = get_sim_inferior_data (inf, SIM_INSTANCE_NOT_NEEDED); - struct resume_data *rd = (struct resume_data *) arg; if (sim_data) { - sim_data->resume_siggnal = rd->siggnal; - sim_data->resume_step = rd->step; + sim_data->resume_siggnal = siggnal; + sim_data->resume_step = step; if (remote_debug) fprintf_unfiltered (gdb_stdlog, _("gdbsim_resume: pid %d, step %d, signal %d\n"), - inf->pid, rd->step, rd->siggnal); + inf->pid, step, siggnal); } - - /* When called from iterate_over_inferiors, a zero return causes the - iteration process to proceed until there are no more inferiors to - consider. */ - return 0; } void gdbsim_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal) { - struct resume_data rd; struct sim_inferior_data *sim_data - = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED); - - rd.siggnal = siggnal; - rd.step = step; + = get_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED); /* We don't access any sim_data members within this function. What's of interest is whether or not the call to @@ -887,9 +878,12 @@ gdbsim_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal) either have multiple inferiors to resume or an error condition. */ if (sim_data) - gdbsim_resume_inferior (find_inferior_ptid (ptid), &rd); + resume_one_inferior (find_inferior_ptid (this, ptid), step, siggnal); else if (ptid == minus_one_ptid) - iterate_over_inferiors (gdbsim_resume_inferior, &rd); + { + for (inferior *inf : all_inferiors (this)) + resume_one_inferior (inf, step, siggnal); + } else error (_("The program is not being run.")); } @@ -969,7 +963,7 @@ gdbsim_target::wait (ptid_t ptid, struct target_waitstatus *status, int options) SIM_INSTANCE_NEEDED); else { - sim_data = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NEEDED); + sim_data = get_inferior_data_by_ptid (ptid, SIM_INSTANCE_NEEDED); if (sim_data == NULL) error (_("Unable to wait for pid %d. Inferior not found."), ptid.pid ()); @@ -1248,7 +1242,7 @@ bool gdbsim_target::thread_alive (ptid_t ptid) { struct sim_inferior_data *sim_data - = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED); + = get_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED); if (sim_data == NULL) return false;