* [RFA] remote-sim.c: Add support for multiple sim instances
@ 2010-07-02 22:30 Kevin Buettner
2010-07-08 20:08 ` Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-07-02 22:30 UTC (permalink / raw)
To: gdb-patches
The patch below was motivated by wanting to be able to examine
simulator memory after a "load", but before a "run". For more
detail, see the thread starting with the following post:
http://sourceware.org/ml/gdb-patches/2010-06/msg00656.html
In the course of working towards a solution for that problem, it
became clear to me that in order to solve it, I would need to move
certain static globals in remote-sim.c into a per-inferior data
structure. (My testing of a simpler patch was showing two regressions
in gdb.multi/base.exp, both caused by the fact that the
"program_loaded" state for one inferior did not hold for all
inferiors.) The movement of certain static globals into a per-inferior
data structure, plus addressing some of the fallout in making such a
change, is what this patch does. It does not address the problem of
being able to access sim memory just after a "load". I will submit a
separate patch for that.
A very nice side effect of this patch is that it is now possible to
have more than one runnable simulator instance. This is illustrated
by the following session using programs from the gdb.multi/base.exp
test case:
GNU gdb (GDB) 7.1.50.20100702-cvs
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=i686-pc-linux-gnu --target=mips-elf".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
(gdb) file testsuite/gdb.multi/hello
Reading symbols from /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hello...done.
(gdb) tar sim
Connected to the simulator.
(gdb) load
Loading section .text, size 0x804 vma 0xa0020000
Loading section .init, size 0x38 vma 0xa0020804
Loading section .fini, size 0x28 vma 0xa002083c
Loading section .eh_frame, size 0x4 vma 0xa0020864
Loading section .jcr, size 0x4 vma 0xa0020868
Loading section .ctors, size 0x8 vma 0xa002086c
Loading section .dtors, size 0x8 vma 0xa0020874
Loading section .rodata, size 0x4 vma 0xa002087c
Loading section .data, size 0x428 vma 0xa0020880
Loading section .sdata, size 0x10 vma 0xa0020ca8
Start address 0xa0020004
Transfer rate: 26048 bits in <1 sec.
(gdb) add-inferior -exec testsuite/gdb.multi/hangout
Added inferior 2
Reading symbols from /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hangout...done.
(gdb) add-inferior -exec testsuite/gdb.multi/goodbye
Added inferior 3
Reading symbols from /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/goodbye...done.
(gdb) info inferior
Num Description Executable
3 <null> /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/goodbye
2 <null> /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hangout
* 1 <null> /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hello
(gdb) inferior 1
[Switching to inferior 1 [process 0] (/mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hello)]
(gdb) load
Loading section .text, size 0x804 vma 0xa0020000
Loading section .init, size 0x38 vma 0xa0020804
Loading section .fini, size 0x28 vma 0xa002083c
Loading section .eh_frame, size 0x4 vma 0xa0020864
Loading section .jcr, size 0x4 vma 0xa0020868
Loading section .ctors, size 0x8 vma 0xa002086c
Loading section .dtors, size 0x8 vma 0xa0020874
Loading section .rodata, size 0x4 vma 0xa002087c
Loading section .data, size 0x428 vma 0xa0020880
Loading section .sdata, size 0x10 vma 0xa0020ca8
Start address 0xa0020004
Transfer rate: 26048 bits in <1 sec.
(gdb) inferior 2
[Switching to inferior 2 [process 0] (/mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hangout)]
(gdb) load
Loading section .text, size 0xacc0 vma 0xa0020000
Loading section .init, size 0x38 vma 0xa002acc0
Loading section .fini, size 0x28 vma 0xa002acf8
Loading section .eh_frame, size 0x40 vma 0xa002ad20
Loading section .jcr, size 0x4 vma 0xa002ad60
Loading section .ctors, size 0x8 vma 0xa002ad64
Loading section .dtors, size 0x8 vma 0xa002ad6c
Loading section .rodata, size 0x5e0 vma 0xa002ad78
Loading section .data, size 0x870 vma 0xa002b358
Loading section .sdata, size 0x88 vma 0xa002bbc8
Start address 0xa0020004
Transfer rate: 385632 bits in <1 sec.
(gdb) inferior 3
[Switching to inferior 3 [process 0] (/mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/goodbye)]
(gdb) load
Loading section .text, size 0x938 vma 0xa0020000
Loading section .init, size 0x38 vma 0xa0020938
Loading section .fini, size 0x28 vma 0xa0020970
Loading section .eh_frame, size 0x4 vma 0xa0020998
Loading section .jcr, size 0x4 vma 0xa002099c
Loading section .ctors, size 0x8 vma 0xa00209a0
Loading section .dtors, size 0x8 vma 0xa00209a8
Loading section .rodata, size 0x4 vma 0xa00209b0
Loading section .data, size 0x428 vma 0xa00209b8
Loading section .sdata, size 0x14 vma 0xa0020de0
Start address 0xa0020004
Transfer rate: 28544 bits in <1 sec.
(gdb) b main
Breakpoint 1 at 0xa002040c: file /ironwood1/sourceware-multi-1/mips-elf/../src/gdb/testsuite/gdb.multi/goodbye.c, line 43.
(gdb) run
Starting program: /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/goodbye
Breakpoint 1, main ()
at /ironwood1/sourceware-multi-1/mips-elf/../src/gdb/testsuite/gdb.multi/goodbye.c:43
43 mailand();
(gdb) inferior 1
[Switching to inferior 1 [process 0] (/mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hello)]
(gdb) b main
Breakpoint 2 at 0xa0020378: file /ironwood1/sourceware-multi-1/mips-elf/../src/gdb/testsuite/gdb.multi/hello.c, line 40.
(gdb) run
Starting program: /mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/hello
Breakpoint 2, main ()
at /ironwood1/sourceware-multi-1/mips-elf/../src/gdb/testsuite/gdb.multi/hello.c:40
40 bar();
(gdb) next
41 tmpx = hello(glob);
(gdb) p glob
$1 = 92
(gdb) inferior 3
[Switching to inferior 3 [process 42002] (/mesquite2/sourceware-multi-1/mips-elf/gdb/testsuite/gdb.multi/goodbye)]
[Switching to thread 1 (Thread <main>)]
#0 main ()
at /ironwood1/sourceware-multi-1/mips-elf/../src/gdb/testsuite/gdb.multi/goodbye.c:43
43 mailand();
(gdb) p glob
$2 = 45
(gdb) next
44 foo(glob);
(gdb) p glob
$3 = 46
(gdb)
The above session shows that it's now possible to run two (or more)
simulator sessions simultateously from gdb. Unfortunately, this only
works for certain simulators. The sims that I've tested and know
to work are frv and mips. The v850 sim should work, but doesn't.
I've tested the arm and powerpc sims and neither of those works with
multiple instances. (Nor are they expected to.) I am fairly certain
that neither the rx nor the m32c sim will work either.
Simulators that aren't capable of multiple instantiations are detected
and an error message is printed. E.g., this is what the message looks
like when attempting to create a second sim instance using an arm-elf
GDB:
(gdb) load
Inferior 2 and inferior 1 have identical simulator state.
(This simulator does not support the running of more than one inferior.)
When should a new sim instance be created? That's one of the
questions that I had to answer while working on this patch.
It seems to me that, long term, the right way to do it is to create a
new sim instance for each invocation of "target sim". That, however,
is not what I did.
I've structured the code in this patch so that the first sim instance
is created when "target sim" is invoked. Subsequent sim instances are
created lazily, as needed. A "load" (aka to_load) operation will
usually trigger the creation of a new sim instance, but so will
attempting to run the program without doing a load. Basically, any
operation which fetches the state using SIM_INSTANCE_NEEDED could,
potentially, cause the creation of a new simulator instance. That
assumes that one has not already been created by some earlier operation,
of course. Thus, due to their sequencing, there are certain operations
that will never be able to create a new sim instance.
But, back to the "right way" for a moment. It seems to me that,
ideally, we'd want to be able to have one inferior connected to a
remote target, another to a simulator, another to a different remote
target, yet another to a sim using a different architecture variant,
etc. In order for that to happen, the target vector will need to
become a per-inferior data structure. Since that's not the case at
the moment, it seemed to me that the next best course of action is
to allocate the sims as required.
Comments? Okay to commit?
Kevin
* remote-sim.c (program_loaded, gdbsim_desc, remote_sim_ptid):
Move these static globals...
(struct sim_inferior_data): ...into this new struct.
(sim_inferior_data_key, next_pid, sim_argv): New static globals.
(gdb_callback, callbacks_initialized): Move these globals to
a point earlier in the file.
(check_for_duplicate_sim_descriptor, get_sim_inferior_data)
(sim_inferior_data_cleanup): New functions.
(SIM_INSTANCE_NOT_NEEDED, SIM_INSTANCE_NEEDED): New constants.
(gdbsim_fetch_register, gdbsim_store_register, gdbsim_load)
(gdbsim_create_inferior, gdbsim_open, gdbsim_close, gdbsim_resume)
(gdbsim_stop, gdbsim_cntrl_c, gdbsim_wait)
(gdbsim_xfer_inferior_memory, gdbsim_files_info)
(gdbsim_mourn_inferior, simulator_command, gdbsim_thread_alive,
(gdbsim_pid_to_str): Invoke `get_sim_inferior_data' to set
new local variable `sim_data' in each of these functions. Use
`sim_data' to reference former globals `program_loaded',
`gdbsim_desc', and `remote_sim_ptid'.
(gdbsim_open): Remove local variable `argv'. Put results of call
to `gdb_buildargv' in `sim_argv' rather than in `argv'. Don't
make a cleanup for it. Free it though when a sim instance cannot
be obtained.
(gdbsim_close): Free sim_argv and null it out as appropriate.
(_initialize_remote_sim): Initialize `sim_inferior_data_key'.
Index: remote-sim.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-sim.c,v
retrieving revision 1.96
diff -u -p -r1.96 remote-sim.c
--- remote-sim.c 16 May 2010 21:11:14 -0000 1.96
+++ remote-sim.c 2 Jul 2010 20:15:59 -0000
@@ -101,19 +101,125 @@ void simulator_command (char *args, int
/* Forward data declarations */
extern struct target_ops gdbsim_ops;
-static int program_loaded = 0;
+static const struct inferior_data *sim_inferior_data_key;
-/* We must keep track of whether the simulator has been opened or not because
- GDB can call a target's close routine twice, but sim_close doesn't allow
- this. We also need to record the result of sim_open so we can pass it
- back to the other sim_foo routines. */
-static SIM_DESC gdbsim_desc = 0;
-
-/* This is the ptid we use while we're connected to the simulator.
- Its value is arbitrary, as the simulator target don't have a notion
- or processes or threads, but we need something non-null to place in
- inferior_ptid. */
-static ptid_t remote_sim_ptid;
+/* Simulator-specific, per-inferior state. */
+struct sim_inferior_data {
+ /* Flag which indicates whether or not the program has been loaded. */
+ int program_loaded;
+
+ /* 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;
+};
+
+/* Value of the next pid to allocate for an inferior. */
+static int next_pid = 42000;
+
+/* Argument list to pass to sim_open() (for creating simulator instances). */
+static char **sim_argv = NULL;
+
+/* OS-level callback functions for write, flush, etc. */
+static host_callback gdb_callback;
+static int callbacks_initialized = 0;
+
+/* Callback for iterate_over_inferiors. It checks to see if
+ the sim descriptor of the per-inferior simulator data
+ passed via ARG is the same as that for the inferior
+ designated by INF. Return true if so; false otherwise. */
+
+static int
+check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sd1, *sd2;
+
+ sd1 = arg;
+ sd2 = inferior_data (inf, sim_inferior_data_key);
+
+ return (sd2 != NULL && sd1 != sd2 && sd2->gdbsim_desc != NULL
+ && sd1->gdbsim_desc == sd2->gdbsim_desc);
+}
+
+/* Flags indicating whether or not a sim instance is needed. One of these
+ flags should be passed to get_sim_inferior_data(). */
+
+enum {SIM_INSTANCE_NOT_NEEDED = 0, SIM_INSTANCE_NEEDED = 1};
+
+/* Obtain pointer to per-inferior simulator data, allocating it if necessary.
+ Attempt to open the sim if SIM_INSTANCE_NEEDED is true. */
+
+static struct sim_inferior_data *
+get_sim_inferior_data (int sim_instance_needed)
+{
+ struct inferior *inf = current_inferior ();
+ struct sim_inferior_data *sim_data
+ = inferior_data (inf, sim_inferior_data_key);
+
+ if (sim_data == NULL)
+ {
+ sim_data = XZALLOC(struct sim_inferior_data);
+ set_inferior_data (inf, sim_inferior_data_key, sim_data);
+
+ /* Allocate a ptid for this inferior. */
+ sim_data->remote_sim_ptid = ptid_build (next_pid, 0, next_pid);
+ next_pid++;
+
+ /* Initialize the other instance variables. */
+ sim_data->program_loaded = 0;
+ sim_data->gdbsim_desc = NULL;
+ }
+
+ /* Try to allocate a new sim instance, if needed. */
+ if (sim_instance_needed && sim_data->gdbsim_desc == 0)
+ {
+ struct inferior *idup;
+ sim_data->gdbsim_desc
+ = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
+ if (sim_data->gdbsim_desc == NULL)
+ error (_("Unable to create simulator instance for inferior %d."), inf->num);
+
+ idup = iterate_over_inferiors (check_for_duplicate_sim_descriptor,
+ sim_data);
+ if (idup != NULL)
+ {
+ /* We don't close the descriptor due to the fact that it's shared with
+ some other inferior. If we were to close it, that might needlessly
+ muck up the other inferior. Of course, it's possible that the damage
+ has already been done... But not closing it seems like the safest
+ option. Note that it *will* ultimately be closed during cleanup of
+ the other inferior. */
+ sim_data->gdbsim_desc = NULL;
+ error (_("Inferior %d and inferior %d would have identical simulator state.\n"
+ "(This simulator does not support the running of more than one inferior.)"),
+ inf->num, idup->num); }
+ }
+
+ return sim_data;
+}
+
+/* Free the per-inferior simulator data. */
+
+static void
+sim_inferior_data_cleanup (struct inferior *inf, void *data)
+{
+ struct sim_inferior_data *sim_data = data;
+
+ if (sim_data != NULL)
+ {
+ if (sim_data->gdbsim_desc)
+ {
+ sim_close (sim_data->gdbsim_desc, 0);
+ sim_data->gdbsim_desc = NULL;
+ }
+ xfree (sim_data);
+ }
+}
static void
dump_mem (char *buf, int len)
@@ -142,9 +248,6 @@ dump_mem (char *buf, int len)
}
}
-static host_callback gdb_callback;
-static int callbacks_initialized = 0;
-
/* Initialize gdb_callback. */
static void
@@ -278,6 +381,8 @@ gdbsim_fetch_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
if (regno == -1)
{
@@ -310,7 +415,7 @@ gdbsim_fetch_register (struct target_ops
gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
memset (buf, 0, MAX_REGISTER_SIZE);
- nr_bytes = sim_fetch_register (gdbsim_desc,
+ nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
buf,
@@ -350,6 +455,9 @@ gdbsim_store_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
+
if (regno == -1)
{
for (regno = 0; regno < gdbarch_num_regs (gdbarch); regno++)
@@ -362,7 +470,7 @@ gdbsim_store_register (struct target_ops
int nr_bytes;
regcache_cooked_read (regcache, regno, tmp);
- nr_bytes = sim_store_register (gdbsim_desc,
+ nr_bytes = sim_store_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
tmp, register_size (gdbarch, regno));
@@ -404,6 +512,8 @@ gdbsim_load (char *args, int fromtty)
{
char **argv;
char *prog;
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
if (args == NULL)
error_no_arg (_("program to load"));
@@ -422,13 +532,13 @@ gdbsim_load (char *args, int fromtty)
/* FIXME: We will print two messages on error.
Need error to either not print anything if passed NULL or need
another routine that doesn't take any arguments. */
- if (sim_load (gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
+ if (sim_load (sim_data->gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
error (_("unable to load program"));
/* FIXME: If a load command should reset the targets registers then
a call to sim_create_inferior() should go here. */
- program_loaded = 1;
+ sim_data->program_loaded = 1;
}
@@ -444,12 +554,14 @@ static void
gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args,
char **env, int from_tty)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
int len;
char *arg_buf, **argv;
if (exec_file == 0 || exec_bfd == 0)
warning (_("No executable file specified."));
- if (!program_loaded)
+ if (!sim_data->program_loaded)
warning (_("No program loaded."));
if (remote_debug)
@@ -457,7 +569,7 @@ gdbsim_create_inferior (struct target_op
(exec_file ? exec_file : "(NULL)"),
args);
- if (ptid_equal (inferior_ptid, remote_sim_ptid))
+ if (ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
gdbsim_kill (target);
remove_breakpoints ();
init_wait_for_inferior ();
@@ -475,9 +587,9 @@ gdbsim_create_inferior (struct target_op
}
else
argv = NULL;
- sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
+ sim_create_inferior (sim_data->gdbsim_desc, exec_bfd, argv, env);
- inferior_ptid = remote_sim_ptid;
+ inferior_ptid = sim_data->remote_sim_ptid;
inferior_appeared (current_inferior (), ptid_get_pid (inferior_ptid));
add_thread_silent (inferior_ptid);
@@ -496,7 +608,8 @@ gdbsim_open (char *args, int from_tty)
{
int len;
char *arg_buf;
- char **argv;
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
if (remote_debug)
printf_filtered ("gdbsim_open: args \"%s\"\n", args ? args : "(null)");
@@ -507,7 +620,7 @@ gdbsim_open (char *args, int from_tty)
sim_close to be called if the simulator is already open, but push_target
is called after sim_open! We can't move the call to push_target before
the call to sim_open because sim_open may invoke `error'. */
- if (gdbsim_desc != NULL)
+ if (sim_data->gdbsim_desc != NULL)
unpush_target (&gdbsim_ops);
len = (7 + 1 /* gdbsim */
@@ -543,14 +656,17 @@ gdbsim_open (char *args, int from_tty)
strcat (arg_buf, " "); /* 1 */
strcat (arg_buf, args);
}
- argv = gdb_buildargv (arg_buf);
- make_cleanup_freeargv (argv);
+ sim_argv = gdb_buildargv (arg_buf);
init_callbacks ();
- gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, argv);
+ sim_data->gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
- if (gdbsim_desc == 0)
- error (_("unable to create simulator instance"));
+ if (sim_data->gdbsim_desc == 0)
+ {
+ freeargv (sim_argv);
+ sim_argv = NULL;
+ error (_("unable to create simulator instance"));
+ }
push_target (&gdbsim_ops);
printf_filtered ("Connected to the simulator.\n");
@@ -572,21 +688,30 @@ gdbsim_open (char *args, int from_tty)
static void
gdbsim_close (int quitting)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_close: quitting %d\n", quitting);
- program_loaded = 0;
+ sim_data->program_loaded = 0;
- if (gdbsim_desc != NULL)
+ if (sim_data->gdbsim_desc != NULL)
{
- sim_close (gdbsim_desc, quitting);
- gdbsim_desc = NULL;
+ sim_close (sim_data->gdbsim_desc, quitting);
+ sim_data->gdbsim_desc = NULL;
+ }
+
+ if (sim_argv != NULL)
+ {
+ freeargv (sim_argv);
+ sim_argv = NULL;
}
end_callbacks ();
generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
- delete_inferior_silent (ptid_get_pid (remote_sim_ptid));
+ delete_thread_silent (sim_data->remote_sim_ptid);
+ delete_inferior_silent (ptid_get_pid (sim_data->remote_sim_ptid));
}
/* Takes a program previously attached to and detaches it.
@@ -620,7 +745,10 @@ static void
gdbsim_resume (struct target_ops *ops,
ptid_t ptid, int step, enum target_signal siggnal)
{
- if (!ptid_equal (inferior_ptid, remote_sim_ptid))
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
+ if (!ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
error (_("The program is not being run."));
if (remote_debug)
@@ -642,7 +770,10 @@ gdbsim_resume (struct target_ops *ops,
static void
gdbsim_stop (ptid_t ptid)
{
- if (!sim_stop (gdbsim_desc))
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
+
+ if (!sim_stop (sim_data->gdbsim_desc))
{
quit ();
}
@@ -676,13 +807,18 @@ gdb_os_poll_quit (host_callback *p)
static void
gdbsim_cntrl_c (int signo)
{
- gdbsim_stop (remote_sim_ptid);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
+ gdbsim_stop (sim_data->remote_sim_ptid);
}
static ptid_t
gdbsim_wait (struct target_ops *ops,
ptid_t ptid, struct target_waitstatus *status, int options)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
static RETSIGTYPE (*prev_sigint) ();
int sigrc = 0;
enum sim_stop reason = sim_running;
@@ -702,11 +838,11 @@ gdbsim_wait (struct target_ops *ops,
#else
prev_sigint = signal (SIGINT, gdbsim_cntrl_c);
#endif
- sim_resume (gdbsim_desc, resume_step, resume_siggnal);
+ sim_resume (sim_data->gdbsim_desc, resume_step, resume_siggnal);
signal (SIGINT, prev_sigint);
resume_step = 0;
- sim_stop_reason (gdbsim_desc, &reason, &sigrc);
+ sim_stop_reason (sim_data->gdbsim_desc, &reason, &sigrc);
switch (reason)
{
@@ -764,15 +900,26 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
int write, struct mem_attrib *attrib,
struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
/* If no program is running yet, then ignore the simulator for
memory. Pass the request down to the next target, hopefully
an exec file. */
if (!target_has_execution)
return 0;
- if (!program_loaded)
+ if (!sim_data->program_loaded)
error (_("No program loaded."));
+ /* Note that we obtained the sim_data pointer above using
+ SIM_INSTANCE_NOT_NEEDED. We do this so that we don't needlessly
+ allocate a sim instance prior to loading a program. If we
+ get to this point in the code though, gdbsim_desc should be
+ non-NULL. (Note that a sim instance is needed in order to load
+ the program...) */
+ gdb_assert (sim_data->gdbsim_desc != NULL);
+
if (remote_debug)
{
/* FIXME: Send to something other than STDOUT? */
@@ -786,11 +933,11 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
if (write)
{
- len = sim_write (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_write (sim_data->gdbsim_desc, memaddr, myaddr, len);
}
else
{
- len = sim_read (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_read (sim_data->gdbsim_desc, memaddr, myaddr, len);
if (remote_debug && len > 0)
dump_mem (myaddr, len);
}
@@ -800,6 +947,8 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
static void
gdbsim_files_info (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
const char *file = "nothing";
if (exec_bfd)
@@ -812,7 +961,7 @@ gdbsim_files_info (struct target_ops *ta
{
printf_filtered ("\tAttached to %s running program %s\n",
target_shortname, file);
- sim_info (gdbsim_desc, 0);
+ sim_info (sim_data->gdbsim_desc, 0);
}
}
@@ -821,12 +970,15 @@ gdbsim_files_info (struct target_ops *ta
static void
gdbsim_mourn_inferior (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_mourn_inferior:\n");
remove_breakpoints ();
generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
+ delete_thread_silent (sim_data->remote_sim_ptid);
}
/* Pass the command argument through to the simulator verbatim. The
@@ -835,7 +987,10 @@ gdbsim_mourn_inferior (struct target_ops
void
simulator_command (char *args, int from_tty)
{
- if (gdbsim_desc == NULL)
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
+ if (sim_data->gdbsim_desc == NULL)
{
/* PREVIOUSLY: The user may give a command before the simulator
@@ -851,7 +1006,7 @@ simulator_command (char *args, int from_
error (_("Not connected to the simulator target"));
}
- sim_do_command (gdbsim_desc, args);
+ sim_do_command (sim_data->gdbsim_desc, args);
/* Invalidate the register cache, in case the simulator command does
something funny. */
@@ -863,7 +1018,10 @@ simulator_command (char *args, int from_
static int
gdbsim_thread_alive (struct target_ops *ops, ptid_t ptid)
{
- if (ptid_equal (ptid, remote_sim_ptid))
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
+
+ if (ptid_equal (ptid, sim_data->remote_sim_ptid))
/* The simulators' task is always alive. */
return 1;
@@ -877,8 +1035,10 @@ static char *
gdbsim_pid_to_str (struct target_ops *ops, ptid_t ptid)
{
static char buf[64];
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
- if (ptid_equal (remote_sim_ptid, ptid))
+ if (ptid_equal (sim_data->remote_sim_ptid, ptid))
{
xsnprintf (buf, sizeof buf, "Thread <main>");
return buf;
@@ -934,7 +1094,6 @@ _initialize_remote_sim (void)
add_com ("sim", class_obscure, simulator_command,
_("Send a command to the simulator."));
- /* Yes, 42000 is arbitrary. The only sense out of it, is that it
- isn't 0. */
- remote_sim_ptid = ptid_build (42000, 0, 42000);
+ sim_inferior_data_key
+ = register_inferior_data_with_cleanup (sim_inferior_data_cleanup);
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-07-02 22:30 [RFA] remote-sim.c: Add support for multiple sim instances Kevin Buettner
@ 2010-07-08 20:08 ` Tom Tromey
2010-07-27 23:07 ` Kevin Buettner
2010-08-03 12:05 ` Pedro Alves
2 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2010-07-08 20:08 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
Kevin> A very nice side effect of this patch is that it is now possible to
Kevin> have more than one runnable simulator instance.
Cool.
Kevin> It seems to me that, long term, the right way to do it is to create a
Kevin> new sim instance for each invocation of "target sim". That, however,
Kevin> is not what I did.
[...]
Kevin> But, back to the "right way" for a moment. It seems to me that,
Kevin> ideally, we'd want to be able to have one inferior connected to a
Kevin> remote target, another to a simulator, another to a different remote
Kevin> target, yet another to a sim using a different architecture variant,
Kevin> etc. In order for that to happen, the target vector will need to
Kevin> become a per-inferior data structure.
Yeah, this has come up a few times now. Somewhere, fairly recently,
Pedro mentioned his preferred approach to this.
Kevin> Comments? Okay to commit?
This isn't an area I am comfortable with, but I didn't want to let your
message go un-replied-to.
I think the idea of putting globals into a per-sim struct is great.
This seems unobjectionable to me.
For the rest, I don't really know, though it seems basically reasonable.
Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-07-02 22:30 [RFA] remote-sim.c: Add support for multiple sim instances Kevin Buettner
2010-07-08 20:08 ` Tom Tromey
@ 2010-07-27 23:07 ` Kevin Buettner
2010-07-27 23:14 ` Pedro Alves
2010-08-03 12:05 ` Pedro Alves
2 siblings, 1 reply; 9+ messages in thread
From: Kevin Buettner @ 2010-07-27 23:07 UTC (permalink / raw)
To: gdb-patches
Any other comments on this patch?
http://sourceware.org/ml/gdb-patches/2010-07/msg00059.html
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-07-27 23:07 ` Kevin Buettner
@ 2010-07-27 23:14 ` Pedro Alves
2010-07-27 23:31 ` Kevin Buettner
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-07-27 23:14 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
On Wednesday 28 July 2010 00:07:35, Kevin Buettner wrote:
> Any other comments on this patch?
>
> http://sourceware.org/ml/gdb-patches/2010-07/msg00059.html
I took a quick look at this the other day, and it looked generaly
a good idea, though I thought I saw a couple of things I'd like
to comment on. I'd like to take another look. Ping me again
if I don't come back to this in the next couple of days.
--
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-07-27 23:14 ` Pedro Alves
@ 2010-07-27 23:31 ` Kevin Buettner
0 siblings, 0 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-07-27 23:31 UTC (permalink / raw)
To: gdb-patches
On Wed, 28 Jul 2010 00:14:20 +0100
Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 28 July 2010 00:07:35, Kevin Buettner wrote:
> > Any other comments on this patch?
> >
> > http://sourceware.org/ml/gdb-patches/2010-07/msg00059.html
>
> I took a quick look at this the other day, and it looked generaly
> a good idea, though I thought I saw a couple of things I'd like
> to comment on. I'd like to take another look. Ping me again
> if I don't come back to this in the next couple of days.
Will do. Thanks for looking at it.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-07-02 22:30 [RFA] remote-sim.c: Add support for multiple sim instances Kevin Buettner
2010-07-08 20:08 ` Tom Tromey
2010-07-27 23:07 ` Kevin Buettner
@ 2010-08-03 12:05 ` Pedro Alves
2010-08-06 23:21 ` Kevin Buettner
2 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-08-03 12:05 UTC (permalink / raw)
To: gdb-patches; +Cc: Kevin Buettner
Pong. ;-)
Comments inline below.
On Friday 02 July 2010 23:30:17, Kevin Buettner wrote:
> Index: remote-sim.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote-sim.c,v
> retrieving revision 1.96
> diff -u -p -r1.96 remote-sim.c
> --- remote-sim.c 16 May 2010 21:11:14 -0000 1.96
> +++ remote-sim.c 2 Jul 2010 20:15:59 -0000
> @@ -101,19 +101,125 @@ void simulator_command (char *args, int
> /* Forward data declarations */
> extern struct target_ops gdbsim_ops;
>
> -static int program_loaded = 0;
> +static const struct inferior_data *sim_inferior_data_key;
>
> -/* We must keep track of whether the simulator has been opened or not because
> - GDB can call a target's close routine twice, but sim_close doesn't allow
> - this. We also need to record the result of sim_open so we can pass it
> - back to the other sim_foo routines. */
> -static SIM_DESC gdbsim_desc = 0;
> -
> -/* This is the ptid we use while we're connected to the simulator.
> - Its value is arbitrary, as the simulator target don't have a notion
> - or processes or threads, but we need something non-null to place in
> - inferior_ptid. */
> -static ptid_t remote_sim_ptid;
> +/* Simulator-specific, per-inferior state. */
> +struct sim_inferior_data {
> + /* Flag which indicates whether or not the program has been loaded. */
> + int program_loaded;
> +
> + /* 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;
> +};
> +
> +/* Value of the next pid to allocate for an inferior. */
> +static int next_pid = 42000;
Maybe this should be reset on, say, gdbsim_open.
> +
> +/* Argument list to pass to sim_open() (for creating simulator instances). */
> +static char **sim_argv = NULL;
> +
> +/* OS-level callback functions for write, flush, etc. */
> +static host_callback gdb_callback;
> +static int callbacks_initialized = 0;
> +
> +/* Callback for iterate_over_inferiors. It checks to see if
> + the sim descriptor of the per-inferior simulator data
> + passed via ARG is the same as that for the inferior
> + designated by INF. Return true if so; false otherwise. */
> +
> +static int
> +check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
> +{
> + struct sim_inferior_data *sd1, *sd2;
> +
> + sd1 = arg;
> + sd2 = inferior_data (inf, sim_inferior_data_key);
> +
> + return (sd2 != NULL && sd1 != sd2 && sd2->gdbsim_desc != NULL
> + && sd1->gdbsim_desc == sd2->gdbsim_desc);
> +}
Why doesn't this return true when sd1 == sd2? Or, is that
something that should never happen?
> +
> +/* Flags indicating whether or not a sim instance is needed. One of these
> + flags should be passed to get_sim_inferior_data(). */
> +
> +enum {SIM_INSTANCE_NOT_NEEDED = 0, SIM_INSTANCE_NEEDED = 1};
> +
> +/* Obtain pointer to per-inferior simulator data, allocating it if necessary.
> + Attempt to open the sim if SIM_INSTANCE_NEEDED is true. */
> +
> +static struct sim_inferior_data *
> +get_sim_inferior_data (int sim_instance_needed)
> +{
> + struct inferior *inf = current_inferior ();
> + struct sim_inferior_data *sim_data
> + = inferior_data (inf, sim_inferior_data_key);
> +
> + if (sim_data == NULL)
> + {
> + sim_data = XZALLOC(struct sim_inferior_data);
> + set_inferior_data (inf, sim_inferior_data_key, sim_data);
> +
> + /* Allocate a ptid for this inferior. */
> + sim_data->remote_sim_ptid = ptid_build (next_pid, 0, next_pid);
> + next_pid++;
> +
> + /* Initialize the other instance variables. */
> + sim_data->program_loaded = 0;
> + sim_data->gdbsim_desc = NULL;
> + }
> +
> + /* Try to allocate a new sim instance, if needed. */
> + if (sim_instance_needed && sim_data->gdbsim_desc == 0)
> + {
> + struct inferior *idup;
> + sim_data->gdbsim_desc
> + = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
> + if (sim_data->gdbsim_desc == NULL)
> + error (_("Unable to create simulator instance for inferior %d."), inf->num);
> +
> + idup = iterate_over_inferiors (check_for_duplicate_sim_descriptor,
> + sim_data);
> + if (idup != NULL)
> + {
> + /* We don't close the descriptor due to the fact that it's shared with
> + some other inferior. If we were to close it, that might needlessly
> + muck up the other inferior. Of course, it's possible that the damage
> + has already been done... But not closing it seems like the safest
> + option. Note that it *will* ultimately be closed during cleanup of
> + the other inferior. */
> + sim_data->gdbsim_desc = NULL;
> + error (_("Inferior %d and inferior %d would have identical simulator state.\n"
> + "(This simulator does not support the running of more than one inferior.)"),
> + inf->num, idup->num); }
> + }
These throw an error while leaving the per-inferior data set in
the inferior. The next_pid global is also always incremented,
even on error. I think you should build the sim_data and open
the simulator fully, before setting the per-inferior pointer,
and incrementing that counter.
> +
> + return sim_data;
> +}
> +
> +/* Free the per-inferior simulator data. */
> +
> +static void
> +sim_inferior_data_cleanup (struct inferior *inf, void *data)
> +{
> + struct sim_inferior_data *sim_data = data;
> +
> + if (sim_data != NULL)
> + {
> + if (sim_data->gdbsim_desc)
> + {
> + sim_close (sim_data->gdbsim_desc, 0);
> + sim_data->gdbsim_desc = NULL;
> + }
> + xfree (sim_data);
> + }
> +}
This is only called when the "struct inferior" object is
deleted. Remember that the inferior stays around in a
"proto-inferior" state (debugging the executable), after
kill/detach/program exit, for example, but there's no garantee that
the next target you'll use it with will be the sim. The
user may do "tar remote" at that point, say.
Thus, you should close the sim descriptor and clear the sim
related data from the inferior on other occasions
as well. IIUC, it is desirable to keep the sim descriptor
open (and the program loaded in the sim), even after
kill/detach/program exit, so you'll want to cleanup
all sim instances and per-inferior sim data when the
gdbsim target is closed.
>
> static void
> dump_mem (char *buf, int len)
> @@ -142,9 +248,6 @@ dump_mem (char *buf, int len)
> }
> }
>
> -static host_callback gdb_callback;
> -static int callbacks_initialized = 0;
> -
> /* Initialize gdb_callback. */
>
> static void
> @@ -278,6 +381,8 @@ gdbsim_fetch_register (struct target_ops
> struct regcache *regcache, int regno)
> {
> struct gdbarch *gdbarch = get_regcache_arch (regcache);
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
>
> if (regno == -1)
> {
> @@ -310,7 +415,7 @@ gdbsim_fetch_register (struct target_ops
>
> gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
> memset (buf, 0, MAX_REGISTER_SIZE);
> - nr_bytes = sim_fetch_register (gdbsim_desc,
> + nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
> gdbarch_register_sim_regno
> (gdbarch, regno),
> buf,
> @@ -350,6 +455,9 @@ gdbsim_store_register (struct target_ops
> struct regcache *regcache, int regno)
> {
> struct gdbarch *gdbarch = get_regcache_arch (regcache);
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> +
> if (regno == -1)
> {
> for (regno = 0; regno < gdbarch_num_regs (gdbarch); regno++)
> @@ -362,7 +470,7 @@ gdbsim_store_register (struct target_ops
> int nr_bytes;
>
> regcache_cooked_read (regcache, regno, tmp);
> - nr_bytes = sim_store_register (gdbsim_desc,
> + nr_bytes = sim_store_register (sim_data->gdbsim_desc,
> gdbarch_register_sim_regno
> (gdbarch, regno),
> tmp, register_size (gdbarch, regno));
> @@ -404,6 +512,8 @@ gdbsim_load (char *args, int fromtty)
> {
> char **argv;
> char *prog;
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
>
> if (args == NULL)
> error_no_arg (_("program to load"));
> @@ -422,13 +532,13 @@ gdbsim_load (char *args, int fromtty)
> /* FIXME: We will print two messages on error.
> Need error to either not print anything if passed NULL or need
> another routine that doesn't take any arguments. */
> - if (sim_load (gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
> + if (sim_load (sim_data->gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
> error (_("unable to load program"));
>
> /* FIXME: If a load command should reset the targets registers then
> a call to sim_create_inferior() should go here. */
>
> - program_loaded = 1;
> + sim_data->program_loaded = 1;
> }
>
>
> @@ -444,12 +554,14 @@ static void
> gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args,
> char **env, int from_tty)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> int len;
> char *arg_buf, **argv;
>
> if (exec_file == 0 || exec_bfd == 0)
> warning (_("No executable file specified."));
> - if (!program_loaded)
> + if (!sim_data->program_loaded)
> warning (_("No program loaded."));
>
> if (remote_debug)
> @@ -457,7 +569,7 @@ gdbsim_create_inferior (struct target_op
> (exec_file ? exec_file : "(NULL)"),
> args);
>
> - if (ptid_equal (inferior_ptid, remote_sim_ptid))
> + if (ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
> gdbsim_kill (target);
> remove_breakpoints ();
> init_wait_for_inferior ();
> @@ -475,9 +587,9 @@ gdbsim_create_inferior (struct target_op
> }
> else
> argv = NULL;
> - sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
> + sim_create_inferior (sim_data->gdbsim_desc, exec_bfd, argv, env);
>
> - inferior_ptid = remote_sim_ptid;
> + inferior_ptid = sim_data->remote_sim_ptid;
> inferior_appeared (current_inferior (), ptid_get_pid (inferior_ptid));
> add_thread_silent (inferior_ptid);
>
> @@ -496,7 +608,8 @@ gdbsim_open (char *args, int from_tty)
> {
> int len;
> char *arg_buf;
> - char **argv;
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
>
> if (remote_debug)
> printf_filtered ("gdbsim_open: args \"%s\"\n", args ? args : "(null)");
> @@ -507,7 +620,7 @@ gdbsim_open (char *args, int from_tty)
> sim_close to be called if the simulator is already open, but push_target
> is called after sim_open! We can't move the call to push_target before
> the call to sim_open because sim_open may invoke `error'. */
> - if (gdbsim_desc != NULL)
> + if (sim_data->gdbsim_desc != NULL)
> unpush_target (&gdbsim_ops);
Although the current inferior may not have a gdbsim_desc,
others might. The question "(gdbsim_desc != NULL)" was
answering before was "is the gdbsim target open?". That's
no longer true after that change.
>
> len = (7 + 1 /* gdbsim */
> @@ -543,14 +656,17 @@ gdbsim_open (char *args, int from_tty)
> strcat (arg_buf, " "); /* 1 */
> strcat (arg_buf, args);
> }
> - argv = gdb_buildargv (arg_buf);
> - make_cleanup_freeargv (argv);
> + sim_argv = gdb_buildargv (arg_buf);
>
> init_callbacks ();
> - gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, argv);
> + sim_data->gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
>
> - if (gdbsim_desc == 0)
> - error (_("unable to create simulator instance"));
> + if (sim_data->gdbsim_desc == 0)
> + {
> + freeargv (sim_argv);
> + sim_argv = NULL;
> + error (_("unable to create simulator instance"));
> + }
This is leaving sim_data set on the inferior when a
exception/error is thrown, and the gdbsim target ends
up not opened. I think it'd be better to build the sim_data
on the side, and one assign it in the inferior when the
open was succesful.
>
> push_target (&gdbsim_ops);
> printf_filtered ("Connected to the simulator.\n");
> @@ -572,21 +688,30 @@ gdbsim_open (char *args, int from_tty)
> static void
> gdbsim_close (int quitting)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
> if (remote_debug)
> printf_filtered ("gdbsim_close: quitting %d\n", quitting);
>
> - program_loaded = 0;
> + sim_data->program_loaded = 0;
>
> - if (gdbsim_desc != NULL)
> + if (sim_data->gdbsim_desc != NULL)
> {
> - sim_close (gdbsim_desc, quitting);
> - gdbsim_desc = NULL;
> + sim_close (sim_data->gdbsim_desc, quitting);
> + sim_data->gdbsim_desc = NULL;
> + }
Following up on a comment I made above, when the target is closed,
you no longer have access to any of the inferiors it was
controlling. Thus, you should do the above for all the
inferiors, not just the current.
> +
> + if (sim_argv != NULL)
> + {
> + freeargv (sim_argv);
> + sim_argv = NULL;
> }
Hmm, not sure I understood the lifetime of this object.
I thought it was just a temporary object to pass to
sim_open. Why isn't it released right after calling
sim_open? What happens to it when you run more than
one inferior?
>
> end_callbacks ();
> generic_mourn_inferior ();
> - delete_thread_silent (remote_sim_ptid);
> - delete_inferior_silent (ptid_get_pid (remote_sim_ptid));
> + delete_thread_silent (sim_data->remote_sim_ptid);
> + delete_inferior_silent (ptid_get_pid (sim_data->remote_sim_ptid));
> }
>
> /* Takes a program previously attached to and detaches it.
> @@ -620,7 +745,10 @@ static void
> gdbsim_resume (struct target_ops *ops,
> ptid_t ptid, int step, enum target_signal siggnal)
> {
> - if (!ptid_equal (inferior_ptid, remote_sim_ptid))
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
> + if (!ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
> error (_("The program is not being run."));
You'll want to be able to resume all inferiors here. Try
"set schedule-multiple on", and then "continue". You'll
see that the PTID argument changes from the PID of the
inferior to minus_one_ptid. That is, if you've set
target_supports_multi_process... hmm, oh, you haven't.
>
> if (remote_debug)
> @@ -642,7 +770,10 @@ gdbsim_resume (struct target_ops *ops,
> static void
> gdbsim_stop (ptid_t ptid)
> {
> - if (!sim_stop (gdbsim_desc))
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> +
> + if (!sim_stop (sim_data->gdbsim_desc))
> {
> quit ();
> }
Pedantically, you should stop the inferior specified by
the PTID argument, not the current. When async is supported,
you may see the core calling this callback with minus_one_ptid,
meaning stop-the-world.
(this only applies when you add support for actually resuming
more than one sim simultaneously)
> @@ -676,13 +807,18 @@ gdb_os_poll_quit (host_callback *p)
> static void
> gdbsim_cntrl_c (int signo)
> {
> - gdbsim_stop (remote_sim_ptid);
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
> + gdbsim_stop (sim_data->remote_sim_ptid);
> }
and here I think you should be telling gdbsim_stop that
you want all sims stopped [gdbsim_stop (minus_one_ptid)].
(this only applies when you add support for actually resuming
more than one sim simultaneously)
>
> static ptid_t
> gdbsim_wait (struct target_ops *ops,
> ptid_t ptid, struct target_waitstatus *status, int options)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> static RETSIGTYPE (*prev_sigint) ();
> int sigrc = 0;
> enum sim_stop reason = sim_running;
> @@ -702,11 +838,11 @@ gdbsim_wait (struct target_ops *ops,
> #else
> prev_sigint = signal (SIGINT, gdbsim_cntrl_c);
> #endif
> - sim_resume (gdbsim_desc, resume_step, resume_siggnal);
> + sim_resume (sim_data->gdbsim_desc, resume_step, resume_siggnal);
> signal (SIGINT, prev_sigint);
> resume_step = 0;
>
> - sim_stop_reason (gdbsim_desc, &reason, &sigrc);
> + sim_stop_reason (sim_data->gdbsim_desc, &reason, &sigrc);
>
> switch (reason)
> {
> @@ -764,15 +900,26 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
> int write, struct mem_attrib *attrib,
> struct target_ops *target)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
> /* If no program is running yet, then ignore the simulator for
> memory. Pass the request down to the next target, hopefully
> an exec file. */
> if (!target_has_execution)
> return 0;
>
> - if (!program_loaded)
> + if (!sim_data->program_loaded)
> error (_("No program loaded."));
>
> + /* Note that we obtained the sim_data pointer above using
> + SIM_INSTANCE_NOT_NEEDED. We do this so that we don't needlessly
> + allocate a sim instance prior to loading a program. If we
> + get to this point in the code though, gdbsim_desc should be
> + non-NULL. (Note that a sim instance is needed in order to load
> + the program...) */
> + gdb_assert (sim_data->gdbsim_desc != NULL);
> +
> if (remote_debug)
> {
> /* FIXME: Send to something other than STDOUT? */
> @@ -786,11 +933,11 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
>
> if (write)
> {
> - len = sim_write (gdbsim_desc, memaddr, myaddr, len);
> + len = sim_write (sim_data->gdbsim_desc, memaddr, myaddr, len);
> }
> else
> {
> - len = sim_read (gdbsim_desc, memaddr, myaddr, len);
> + len = sim_read (sim_data->gdbsim_desc, memaddr, myaddr, len);
> if (remote_debug && len > 0)
> dump_mem (myaddr, len);
> }
> @@ -800,6 +947,8 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
> static void
> gdbsim_files_info (struct target_ops *target)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> const char *file = "nothing";
>
> if (exec_bfd)
> @@ -812,7 +961,7 @@ gdbsim_files_info (struct target_ops *ta
> {
> printf_filtered ("\tAttached to %s running program %s\n",
> target_shortname, file);
> - sim_info (gdbsim_desc, 0);
> + sim_info (sim_data->gdbsim_desc, 0);
> }
> }
>
> @@ -821,12 +970,15 @@ gdbsim_files_info (struct target_ops *ta
> static void
> gdbsim_mourn_inferior (struct target_ops *target)
> {
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
> if (remote_debug)
> printf_filtered ("gdbsim_mourn_inferior:\n");
>
> remove_breakpoints ();
> generic_mourn_inferior ();
> - delete_thread_silent (remote_sim_ptid);
> + delete_thread_silent (sim_data->remote_sim_ptid);
> }
>
> /* Pass the command argument through to the simulator verbatim. The
> @@ -835,7 +987,10 @@ gdbsim_mourn_inferior (struct target_ops
> void
> simulator_command (char *args, int from_tty)
> {
> - if (gdbsim_desc == NULL)
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
This command can be run at any random time, even before
"target sim". I don't think you should be attaching a
sim_inferior_data to the current inferior here
(get_sim_inferior_data attaches one if there isn't one already).
That is, if "inferior_data (inf, sim_inferior_data_key)"
is NULL, then the error below also applies.
> + if (sim_data->gdbsim_desc == NULL)
> {
>
> /* PREVIOUSLY: The user may give a command before the simulator
> @@ -851,7 +1006,7 @@ simulator_command (char *args, int from_
> error (_("Not connected to the simulator target"));
> }
>
> - sim_do_command (gdbsim_desc, args);
> + sim_do_command (sim_data->gdbsim_desc, args);
>
> /* Invalidate the register cache, in case the simulator command does
> something funny. */
> @@ -863,7 +1018,10 @@ simulator_command (char *args, int from_
> static int
> gdbsim_thread_alive (struct target_ops *ops, ptid_t ptid)
> {
> - if (ptid_equal (ptid, remote_sim_ptid))
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> +
Here you should be getting at the sim specified by PTID too,
not the current. otherwise "info threads" is finding
that all threads not of the current inferior are
always dead. There may be other places with the same
issue. Basically, if you have a PTID argument, you
should be using that instead of the current inferior.
( At this point, I'll point out that there'll be several
places that will want to map a pid to a
sim_inferior_data object, and it is no longer clear
whether using the inferior data mechanism is a win,
compared to a private list like linux-thread-db.c
maintains. Note that even current_inferior() does a
lookup by PID internally. The per-inferior mechanism
has the small disadvantage that all registered keys
allocate a slot space in the inferior instances' generic
data array, even if not used. )
> + if (ptid_equal (ptid, sim_data->remote_sim_ptid))
> /* The simulators' task is always alive. */
> return 1;
>
> @@ -877,8 +1035,10 @@ static char *
> gdbsim_pid_to_str (struct target_ops *ops, ptid_t ptid)
> {
> static char buf[64];
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
>
> - if (ptid_equal (remote_sim_ptid, ptid))
> + if (ptid_equal (sim_data->remote_sim_ptid, ptid))
Ditto.
> {
> xsnprintf (buf, sizeof buf, "Thread <main>");
> return buf;
> @@ -934,7 +1094,6 @@ _initialize_remote_sim (void)
> add_com ("sim", class_obscure, simulator_command,
> _("Send a command to the simulator."));
>
> - /* Yes, 42000 is arbitrary. The only sense out of it, is that it
> - isn't 0. */
> - remote_sim_ptid = ptid_build (42000, 0, 42000);
> + sim_inferior_data_key
> + = register_inferior_data_with_cleanup (sim_inferior_data_cleanup);
> }
--
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-08-03 12:05 ` Pedro Alves
@ 2010-08-06 23:21 ` Kevin Buettner
2010-08-08 16:43 ` Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Kevin Buettner @ 2010-08-06 23:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
On Tue, 3 Aug 2010 13:05:11 +0100
Pedro Alves <pedro@codesourcery.com> wrote:
> Pong. ;-)
Thanks for the patch review. I've appended a new patch at the end and
respond to each of your comments below.
> > +/* Value of the next pid to allocate for an inferior. */
> > +static int next_pid = 42000;
>
> Maybe this should be reset on, say, gdbsim_open.
Done. I sort of liked the idea of letting it continue to increment, but
if I do that I need to figure out how to make it wrap at some point.
In the end, I decided it was cleaner to do as you suggest.
> > +static int
> > +check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
> > +{
> > + struct sim_inferior_data *sd1, *sd2;
> > +
> > + sd1 = arg;
> > + sd2 = inferior_data (inf, sim_inferior_data_key);
> > +
> > + return (sd2 != NULL && sd1 != sd2 && sd2->gdbsim_desc != NULL
> > + && sd1->gdbsim_desc == sd2->gdbsim_desc);
> > +}
>
> Why doesn't this return true when sd1 == sd2? Or, is that
> something that should never happen?
We didn't want to return true when sd1 == sd2 because we're searching
for duplicate sim instances, not the very same instance.
It's moot now because I ended up rearranging get_sim_inferior_data()
to attempt the sim open, if needed, prior to doing the other
allocations. I ended up passing the resulting SIM_DESC in arg
instead. That ended up being nicer anyway because now the condition
is simpler and, hopefully, easier to read and understand.
[most of get_sim_inferior_data snipped]
> > + error (_("Inferior %d and inferior %d would have identical simulator state.\n"
> > + "(This simulator does not support the running of more than one inferior.)"),
> > + inf->num, idup->num); }
> > + }
>
> These throw an error while leaving the per-inferior data set in
> the inferior. The next_pid global is also always incremented,
> even on error. I think you should build the sim_data and open
> the simulator fully, before setting the per-inferior pointer,
> and incrementing that counter.
Good suggestion. Done.
Note though, for sim instances after the first, that it's still
possible for the per-inferior data to be allocated via some other
means prior to actually allocating the sim. I think that that's
as it should be though.
> > +static void
> > +sim_inferior_data_cleanup (struct inferior *inf, void *data)
> [...]
>
> This is only called when the "struct inferior" object is
> deleted. Remember that the inferior stays around in a
> "proto-inferior" state (debugging the executable), after
> kill/detach/program exit, for example, but there's no garantee that
> the next target you'll use it with will be the sim. The
> user may do "tar remote" at that point, say.
> Thus, you should close the sim descriptor and clear the sim
> related data from the inferior on other occasions
> as well. IIUC, it is desirable to keep the sim descriptor
> open (and the program loaded in the sim), even after
> kill/detach/program exit, so you'll want to cleanup
> all sim instances and per-inferior sim data when the
> gdbsim target is closed.
Done. (I'll have more to say about gdbsim_close later.)
> > @@ -507,7 +620,7 @@ gdbsim_open (char *args, int from_tty)
> > sim_close to be called if the simulator is already open, but push_target
> > is called after sim_open! We can't move the call to push_target before
> > the call to sim_open because sim_open may invoke `error'. */
> > - if (gdbsim_desc != NULL)
> > + if (sim_data->gdbsim_desc != NULL)
> > unpush_target (&gdbsim_ops);
>
> Although the current inferior may not have a gdbsim_desc,
> others might. The question "(gdbsim_desc != NULL)" was
> answering before was "is the gdbsim target open?". That's
> no longer true after that change.
Right you are. I ended up adding the static global `gdbsim_is_open'
which is set in gdbsim_open() and cleared in gdbsim_closed(). There
is at least one other static global which could have been subverted
for this purpose, but I thought it clearer to create a flag which
directly asks/answers the question "is the gdbsim target open?"
> > @@ -543,14 +656,17 @@ gdbsim_open (char *args, int from_tty)
[...]
> > + if (sim_data->gdbsim_desc == 0)
> > + {
> > + freeargv (sim_argv);
> > + sim_argv = NULL;
> > + error (_("unable to create simulator instance"));
> > + }
>
> This is leaving sim_data set on the inferior when a
> exception/error is thrown, and the gdbsim target ends
> up not opened. I think it'd be better to build the sim_data
> on the side, and one assign it in the inferior when the
> open was succesful.
Done.
> > gdbsim_close (int quitting)
[...]
> > + sim_close (sim_data->gdbsim_desc, quitting);
> > + sim_data->gdbsim_desc = NULL;
> > + }
>
> Following up on a comment I made above, when the target is closed,
> you no longer have access to any of the inferiors it was
> controlling. Thus, you should do the above for all the
> inferiors, not just the current.
Done.
> > +
> > + if (sim_argv != NULL)
> > + {
> > + freeargv (sim_argv);
> > + sim_argv = NULL;
> > }
>
> Hmm, not sure I understood the lifetime of this object.
> I thought it was just a temporary object to pass to
> sim_open. Why isn't it released right after calling
> sim_open? What happens to it when you run more than
> one inferior?
sim_open() is called from two places, gdbsim_open() and
get_sim_inferior_data(). The call in gdbsim_open allocates
the first sim instance. The call in get_sim_inferior_data()
allocates all subsequent sim instances (up until the target
is closed). It is for those subsequent sim instance allocations
that sim_argv has a lifetime that lasts from gdbsim_open()
through gdbsim_close().
I've added a comment regarding the lifetime of sim_argv just
prior to its declaration.
> > + delete_thread_silent (sim_data->remote_sim_ptid);
> > + delete_inferior_silent (ptid_get_pid (sim_data->remote_sim_ptid));
The above were in gdbsim_close(). While I was reworking
gdbsim_close(), I found that I needed to remove the calls to
delete_thread_silent() and delete_inferior_silent(). (I was getting
SIGSEGVs with them in place - actually, I think it was just the call
to delete_inferior_silent() that was the problem.)
Also, I moved the call to generic_mourn_inferior into the new function
gdbsim_close_inferior() (which is invoked via iterate_over_inferiors).
Thus, generic_mourn_inferior() is invoked for each inferior which
has sim inferior data.
Anyway, I'd appreciate it if you'd take a particularly close look at
gdbsim_close() and gdbsim_close_inferior().
> > gdbsim_resume (struct target_ops *ops,
> > ptid_t ptid, int step, enum target_signal siggnal)
> > {
> > - if (!ptid_equal (inferior_ptid, remote_sim_ptid))
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> > +
> > + if (!ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
> > error (_("The program is not being run."));
>
> You'll want to be able to resume all inferiors here. Try
> "set schedule-multiple on", and then "continue". You'll
> see that the PTID argument changes from the PID of the
> inferior to minus_one_ptid. That is, if you've set
> target_supports_multi_process... hmm, oh, you haven't.
Done. Though it's not particularly useful since we can't wait for
multiple inferiors yet.
> > gdbsim_stop (ptid_t ptid)
> > {
> > - if (!sim_stop (gdbsim_desc))
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NEEDED);
> > +
> > + if (!sim_stop (sim_data->gdbsim_desc))
>
> Pedantically, you should stop the inferior specified by
> the PTID argument, not the current. When async is supported,
> you may see the core calling this callback with minus_one_ptid,
> meaning stop-the-world.
> (this only applies when you add support for actually resuming
> more than one sim simultaneously)
Done. I've added cases form minus_one_ptid as well as single
inferiors.
> > {
> > quit ();
> > }
This call to quit() is now in gdbsim_stop_inferior(). I was surprised
to see that failure to stop the sim is considered to be a fatal error.
I should think that an ordinary error would be sufficient.
> > @@ -676,13 +807,18 @@ gdb_os_poll_quit (host_callback *p)
> > static void
> > gdbsim_cntrl_c (int signo)
> > {
> > - gdbsim_stop (remote_sim_ptid);
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> > +
> > + gdbsim_stop (sim_data->remote_sim_ptid);
> > }
>
> and here I think you should be telling gdbsim_stop that
> you want all sims stopped [gdbsim_stop (minus_one_ptid)].
> (this only applies when you add support for actually resuming
> more than one sim simultaneously)
Done.
> > simulator_command (char *args, int from_tty)
> > {
> > - if (gdbsim_desc == NULL)
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> > +
>
> This command can be run at any random time, even before
> "target sim". I don't think you should be attaching a
> sim_inferior_data to the current inferior here
> (get_sim_inferior_data attaches one if there isn't one already).
> That is, if "inferior_data (inf, sim_inferior_data_key)"
> is NULL, then the error below also applies.
[...]
> > error (_("Not connected to the simulator target"));
Agreed. I now call inferior_data() directly instead of using
get_sim_inferior_data(). I've also added a fair sized comment
of explanation.
> > static int
> > gdbsim_thread_alive (struct target_ops *ops, ptid_t ptid)
> > {
> > - if (ptid_equal (ptid, remote_sim_ptid))
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> > +
>
> Here you should be getting at the sim specified by PTID too,
> not the current. otherwise "info threads" is finding
> that all threads not of the current inferior are
> always dead. There may be other places with the same
> issue. Basically, if you have a PTID argument, you
> should be using that instead of the current inferior.
Done. I ended up creating a function named
get_sim_inferior_data_by_ptid() for use in those instances where a
ptid is provided.
> ( At this point, I'll point out that there'll be several
> places that will want to map a pid to a
> sim_inferior_data object, and it is no longer clear
> whether using the inferior data mechanism is a win,
> compared to a private list like linux-thread-db.c
> maintains. Note that even current_inferior() does a
> lookup by PID internally. The per-inferior mechanism
> has the small disadvantage that all registered keys
> allocate a slot space in the inferior instances' generic
> data array, even if not used. )
Noted.
> > gdbsim_pid_to_str (struct target_ops *ops, ptid_t ptid)
> > {
> > static char buf[64];
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (SIM_INSTANCE_NOT_NEEDED);
> >
> > - if (ptid_equal (remote_sim_ptid, ptid))
> > + if (ptid_equal (sim_data->remote_sim_ptid, ptid))
>
> Ditto.
I started off by revising this to fetch sim_data by ptid. But, after
thinking about it some more, it seemed to that...
> > xsnprintf (buf, sizeof buf, "Thread <main>");
...the string "Thread <main>" would be printed for all sim inferiors
and that didn't seem very useful as there's nothing in that string
allowing the user to distinguish between them.
So, in the end, I decided that it made the most sense to just use
normal_pid_to_str().
Here's the new patch:
* remote-sim.c (program_loaded, gdbsim_desc, remote_sim_ptid)
(resume_siggnal, resume_step): Move these static globals...
(struct sim_inferior_data): ...into this new struct.
(sim_inferior_data_key, next_pid, sim_argv, gdbsim_is_open):
New static globals.
(gdb_callback, callbacks_initialized): Move these globals to
a point earlier in the file.
(check_for_duplicate_sim_descriptor, get_sim_inferior_data)
(get_sim_inferior_data_by_ptid, sim_inferior_data_cleanup)
(gdbsim_close_inferior, gdbsim_resume_inferior)
(gdbsim_stop_inferior): New functions.
(SIM_INSTANCE_NOT_NEEDED, SIM_INSTANCE_NEEDED, INITIAL_PID):
New constants.
(gdbsim_fetch_register, gdbsim_store_register, gdbsim_load)
(gdbsim_create_inferior, gdbsim_open, gdbsim_close, gdbsim_resume)
(gdbsim_stop, gdbsim_cntrl_c, gdbsim_wait)
(gdbsim_xfer_inferior_memory, gdbsim_files_info)
(gdbsim_mourn_inferior, simulator_command, gdbsim_thread_alive,
(gdbsim_pid_to_str): Invoke `get_sim_inferior_data' to set
new local variable `sim_data' in each of these functions. Use
`sim_data' to reference former globals `program_loaded',
`gdbsim_desc', `remote_sim_ptid', `resume_siggnal', and
`resume_step'.
(gdbsim_open): Remove local variable `argv'. Put results of call
to `gdb_buildargv' in `sim_argv' rather than in `argv'. Don't
make a cleanup for it. Free it though when a sim instance cannot
be obtained.
(gdbsim_close): Free sim_argv and null it out as appropriate.
Close sim instances in all inferiors.
(gdbsim_cntrl_c): Stop all inferiors.
(gdbsim_wait):
(_initialize_remote_sim): Initialize `sim_inferior_data_key'.
Index: remote-sim.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-sim.c,v
retrieving revision 1.96
diff -u -p -r1.96 remote-sim.c
--- remote-sim.c 16 May 2010 21:11:14 -0000 1.96
+++ remote-sim.c 6 Aug 2010 23:08:31 -0000
@@ -101,19 +101,175 @@ void simulator_command (char *args, int
/* Forward data declarations */
extern struct target_ops gdbsim_ops;
-static int program_loaded = 0;
+static const struct inferior_data *sim_inferior_data_key;
-/* We must keep track of whether the simulator has been opened or not because
- GDB can call a target's close routine twice, but sim_close doesn't allow
- this. We also need to record the result of sim_open so we can pass it
- back to the other sim_foo routines. */
-static SIM_DESC gdbsim_desc = 0;
-
-/* This is the ptid we use while we're connected to the simulator.
- Its value is arbitrary, as the simulator target don't have a notion
- or processes or threads, but we need something non-null to place in
- inferior_ptid. */
-static ptid_t remote_sim_ptid;
+/* Simulator-specific, per-inferior state. */
+struct sim_inferior_data {
+ /* Flag which indicates whether or not the program has been loaded. */
+ int program_loaded;
+
+ /* 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 target_signal resume_siggnal;
+
+ /* Flag which indicates whether resume should step or not. */
+ int resume_step;
+};
+
+/* Flag indicating the "open" status of this module. It's set to 1
+ in gdbsim_open() and 0 in gdbsim_close(). */
+static int gdbsim_is_open = 0;
+
+/* 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
+
+/* Argument list to pass to sim_open(). It is allocated in gdbsim_open()
+ and deallocated in gdbsim_close(). The lifetime needs to extend beyond
+ the call to gdbsim_open() due to the fact that other sim instances other
+ than the first will be allocated after the gdbsim_open() call. */
+static char **sim_argv = NULL;
+
+/* OS-level callback functions for write, flush, etc. */
+static host_callback gdb_callback;
+static int callbacks_initialized = 0;
+
+/* Callback for iterate_over_inferiors. It checks to see if the sim
+ descriptor passed via ARG is the same as that for the inferior
+ designated by INF. Return true if so; false otherwise. */
+
+static int
+check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data;
+ SIM_DESC new_sim_desc = arg;
+
+ sim_data = inferior_data (inf, sim_inferior_data_key);
+
+ return (sim_data != NULL && sim_data->gdbsim_desc == new_sim_desc);
+}
+
+/* Flags indicating whether or not a sim instance is needed. One of these
+ flags should be passed to get_sim_inferior_data(). */
+
+enum {SIM_INSTANCE_NOT_NEEDED = 0, SIM_INSTANCE_NEEDED = 1};
+
+/* Obtain pointer to per-inferior simulator data, allocating it if necessary.
+ Attempt to open the sim if SIM_INSTANCE_NEEDED is true. */
+
+static struct sim_inferior_data *
+get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
+{
+ SIM_DESC sim_desc = NULL;
+ struct sim_inferior_data *sim_data
+ = inferior_data (inf, sim_inferior_data_key);
+
+ /* Try to allocate a new sim instance, if needed. We do this ahead of
+ a potential allocation of a sim_inferior_data struct in order to
+ avoid needlessly allocating that struct in the event that the sim
+ instance allocation fails. */
+ if (sim_instance_needed == SIM_INSTANCE_NEEDED
+ && (sim_data == NULL || sim_data->gdbsim_desc == NULL))
+ {
+ struct inferior *idup;
+ sim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
+ if (sim_desc == NULL)
+ error (_("Unable to create simulator instance for inferior %d."),
+ inf->num);
+
+ idup = iterate_over_inferiors (check_for_duplicate_sim_descriptor,
+ sim_desc);
+ if (idup != NULL)
+ {
+ /* We don't close the descriptor due to the fact that it's
+ shared with some other inferior. If we were to close it,
+ that might needlessly muck up the other inferior. Of
+ course, it's possible that the damage has already been
+ done... Note that it *will* ultimately be closed during
+ cleanup of the other inferior. */
+ sim_desc = NULL;
+ error (
+ _("Inferior %d and inferior %d would have identical simulator state.\n"
+ "(This simulator does not support the running of more than one inferior.)"),
+ inf->num, idup->num);
+ }
+ }
+
+ if (sim_data == NULL)
+ {
+ sim_data = XZALLOC(struct sim_inferior_data);
+ set_inferior_data (inf, sim_inferior_data_key, sim_data);
+
+ /* Allocate a ptid for this inferior. */
+ sim_data->remote_sim_ptid = ptid_build (next_pid, 0, next_pid);
+ next_pid++;
+
+ /* Initialize the other instance variables. */
+ sim_data->program_loaded = 0;
+ sim_data->gdbsim_desc = sim_desc;
+ sim_data->resume_siggnal = TARGET_SIGNAL_0;
+ sim_data->resume_step = 0;
+ }
+ else if (sim_desc)
+ {
+ /* This handles the case where sim_data was allocated prior to
+ needing a sim instance. */
+ sim_data->gdbsim_desc = sim_desc;
+ }
+
+
+ return sim_data;
+}
+
+/* Return pointer to per-inferior simulator data using PTID to find the
+ 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)
+{
+ struct inferior *inf;
+ int pid = ptid_get_pid (ptid);
+
+ if (pid <= 0)
+ return NULL;
+
+ inf = find_inferior_pid (pid);
+
+ if (inf)
+ return get_sim_inferior_data (inf, sim_instance_needed);
+ else
+ return NULL;
+}
+
+/* Free the per-inferior simulator data. */
+
+static void
+sim_inferior_data_cleanup (struct inferior *inf, void *data)
+{
+ struct sim_inferior_data *sim_data = data;
+
+ if (sim_data != NULL)
+ {
+ if (sim_data->gdbsim_desc)
+ {
+ sim_close (sim_data->gdbsim_desc, 0);
+ sim_data->gdbsim_desc = NULL;
+ }
+ xfree (sim_data);
+ }
+}
static void
dump_mem (char *buf, int len)
@@ -142,9 +298,6 @@ dump_mem (char *buf, int len)
}
}
-static host_callback gdb_callback;
-static int callbacks_initialized = 0;
-
/* Initialize gdb_callback. */
static void
@@ -278,6 +431,8 @@ gdbsim_fetch_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
if (regno == -1)
{
@@ -310,7 +465,7 @@ gdbsim_fetch_register (struct target_ops
gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
memset (buf, 0, MAX_REGISTER_SIZE);
- nr_bytes = sim_fetch_register (gdbsim_desc,
+ nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
buf,
@@ -350,6 +505,9 @@ gdbsim_store_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
+
if (regno == -1)
{
for (regno = 0; regno < gdbarch_num_regs (gdbarch); regno++)
@@ -362,7 +520,7 @@ gdbsim_store_register (struct target_ops
int nr_bytes;
regcache_cooked_read (regcache, regno, tmp);
- nr_bytes = sim_store_register (gdbsim_desc,
+ nr_bytes = sim_store_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
tmp, register_size (gdbarch, regno));
@@ -404,6 +562,8 @@ gdbsim_load (char *args, int fromtty)
{
char **argv;
char *prog;
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
if (args == NULL)
error_no_arg (_("program to load"));
@@ -422,13 +582,13 @@ gdbsim_load (char *args, int fromtty)
/* FIXME: We will print two messages on error.
Need error to either not print anything if passed NULL or need
another routine that doesn't take any arguments. */
- if (sim_load (gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
+ if (sim_load (sim_data->gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
error (_("unable to load program"));
/* FIXME: If a load command should reset the targets registers then
a call to sim_create_inferior() should go here. */
- program_loaded = 1;
+ sim_data->program_loaded = 1;
}
@@ -444,12 +604,14 @@ static void
gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args,
char **env, int from_tty)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
int len;
char *arg_buf, **argv;
if (exec_file == 0 || exec_bfd == 0)
warning (_("No executable file specified."));
- if (!program_loaded)
+ if (!sim_data->program_loaded)
warning (_("No program loaded."));
if (remote_debug)
@@ -457,7 +619,7 @@ gdbsim_create_inferior (struct target_op
(exec_file ? exec_file : "(NULL)"),
args);
- if (ptid_equal (inferior_ptid, remote_sim_ptid))
+ if (ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
gdbsim_kill (target);
remove_breakpoints ();
init_wait_for_inferior ();
@@ -475,9 +637,9 @@ gdbsim_create_inferior (struct target_op
}
else
argv = NULL;
- sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
+ sim_create_inferior (sim_data->gdbsim_desc, exec_bfd, argv, env);
- inferior_ptid = remote_sim_ptid;
+ inferior_ptid = sim_data->remote_sim_ptid;
inferior_appeared (current_inferior (), ptid_get_pid (inferior_ptid));
add_thread_silent (inferior_ptid);
@@ -496,18 +658,20 @@ gdbsim_open (char *args, int from_tty)
{
int len;
char *arg_buf;
- char **argv;
+ struct sim_inferior_data *sim_data;
+ SIM_DESC gdbsim_desc;
if (remote_debug)
printf_filtered ("gdbsim_open: args \"%s\"\n", args ? args : "(null)");
- /* Remove current simulator if one exists. Only do this if the simulator
- has been opened because sim_close requires it.
- This is important because the call to push_target below will cause
- sim_close to be called if the simulator is already open, but push_target
- is called after sim_open! We can't move the call to push_target before
- the call to sim_open because sim_open may invoke `error'. */
- if (gdbsim_desc != NULL)
+ /* Ensure that the sim target is not on the target stack. This is
+ necessary, because if it is on the target stack, the call to
+ push_target below will invoke sim_close(), thus freeing various
+ state (including a sim instance) that we allocate prior to
+ invoking push_target(). We want to delay the push_target()
+ operation until after we complete those operations which could
+ error out. */
+ if (gdbsim_is_open)
unpush_target (&gdbsim_ops);
len = (7 + 1 /* gdbsim */
@@ -543,14 +707,26 @@ gdbsim_open (char *args, int from_tty)
strcat (arg_buf, " "); /* 1 */
strcat (arg_buf, args);
}
- argv = gdb_buildargv (arg_buf);
- make_cleanup_freeargv (argv);
+ sim_argv = gdb_buildargv (arg_buf);
init_callbacks ();
- gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, argv);
+ gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
if (gdbsim_desc == 0)
- error (_("unable to create simulator instance"));
+ {
+ freeargv (sim_argv);
+ sim_argv = NULL;
+ error (_("unable to create simulator instance"));
+ }
+
+ /* Reset the pid numberings for this batch of sim instances. */
+ next_pid = INITIAL_PID;
+
+ /* Allocate the inferior data, but do not allocate a sim instance
+ since we've already just done that. */
+ sim_data = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
+ sim_data->gdbsim_desc = gdbsim_desc;
push_target (&gdbsim_ops);
printf_filtered ("Connected to the simulator.\n");
@@ -558,6 +734,30 @@ gdbsim_open (char *args, int from_tty)
/* There's nothing running after "target sim" or "load"; not until
"run". */
inferior_ptid = null_ptid;
+
+ gdbsim_is_open = 1;
+}
+
+/* Callback for iterate_over_inferiors. Called (indirectly) by
+ gdbsim_close(). */
+
+static int
+gdbsim_close_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data = inferior_data (inf,
+ sim_inferior_data_key);
+ if (sim_data != NULL)
+ {
+ ptid_t ptid = sim_data->remote_sim_ptid;
+
+ sim_inferior_data_cleanup (inf, sim_data);
+ set_inferior_data (inf, sim_inferior_data_key, NULL);
+
+ inferior_ptid = ptid;
+ generic_mourn_inferior ();
+ }
+
+ return 0;
}
/* Does whatever cleanup is required for a target that we are no longer
@@ -572,21 +772,23 @@ gdbsim_open (char *args, int from_tty)
static void
gdbsim_close (int quitting)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_close: quitting %d\n", quitting);
- program_loaded = 0;
+ iterate_over_inferiors (gdbsim_close_inferior, NULL);
- if (gdbsim_desc != NULL)
+ if (sim_argv != NULL)
{
- sim_close (gdbsim_desc, quitting);
- gdbsim_desc = NULL;
+ freeargv (sim_argv);
+ sim_argv = NULL;
}
end_callbacks ();
- generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
- delete_inferior_silent (ptid_get_pid (remote_sim_ptid));
+
+ gdbsim_is_open = 0;
}
/* Takes a program previously attached to and detaches it.
@@ -613,21 +815,59 @@ gdbsim_detach (struct target_ops *ops, c
or to run free; SIGGNAL is the signal value (e.g. SIGINT) to be given
to the target, or zero for no signal. */
-static enum target_signal resume_siggnal;
-static int resume_step;
+struct resume_data
+{
+ enum target_signal siggnal;
+ int step;
+};
+
+static int
+gdbsim_resume_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (inf, SIM_INSTANCE_NOT_NEEDED);
+ struct resume_data *rd = arg;
+
+ if (sim_data)
+ {
+ sim_data->resume_siggnal = rd->siggnal;
+ sim_data->resume_step = rd->step;
+
+ if (remote_debug)
+ printf_filtered (_("gdbsim_resume: pid %d, step %d, signal %d\n"),
+ inf->pid, rd->step, rd->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;
+}
static void
gdbsim_resume (struct target_ops *ops,
ptid_t ptid, int step, enum target_signal siggnal)
{
- if (!ptid_equal (inferior_ptid, remote_sim_ptid))
+ 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;
+
+ /* We don't access any sim_data members within this function.
+ What's of interest is whether or not the call to
+ get_sim_inferior_data_by_ptid(), above, is able to obtain a
+ non-NULL pointer. If it managed to obtain a non-NULL pointer, we
+ know we have a single inferior to consider. If it's NULL, we
+ either have multiple inferiors to resume or an error condition. */
+
+ if (sim_data)
+ gdbsim_resume_inferior (find_inferior_pid (ptid_get_pid (ptid)), &rd);
+ else if (ptid_equal (ptid, minus_one_ptid))
+ iterate_over_inferiors (gdbsim_resume_inferior, &rd);
+ else
error (_("The program is not being run."));
-
- if (remote_debug)
- printf_filtered ("gdbsim_resume: step %d, signal %d\n", step, siggnal);
-
- resume_siggnal = siggnal;
- resume_step = step;
}
/* Notify the simulator of an asynchronous request to stop.
@@ -639,12 +879,44 @@ gdbsim_resume (struct target_ops *ops,
For simulators that do not support this operation, just abort */
+static int
+gdbsim_stop_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED);
+
+ if (sim_data)
+ {
+ /* Try to stop the sim. If it can't stop, exit GDB altogether. */
+ if (!sim_stop (sim_data->gdbsim_desc))
+ {
+ quit ();
+ }
+ }
+
+ /* 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;
+}
+
static void
gdbsim_stop (ptid_t ptid)
{
- if (!sim_stop (gdbsim_desc))
+ struct sim_inferior_data *sim_data;
+
+ if (ptid_equal (ptid, minus_one_ptid))
+ {
+ iterate_over_inferiors (gdbsim_stop_inferior, NULL);
+ }
+ else
{
- quit ();
+ struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+
+ if (inf == NULL)
+ error (_("Can't stop pid %d. No inferior found."), ptid_get_pid (ptid));
+
+ gdbsim_stop_inferior (inf, NULL);
}
}
@@ -676,17 +948,32 @@ gdb_os_poll_quit (host_callback *p)
static void
gdbsim_cntrl_c (int signo)
{
- gdbsim_stop (remote_sim_ptid);
+ gdbsim_stop (minus_one_ptid);
}
static ptid_t
gdbsim_wait (struct target_ops *ops,
ptid_t ptid, struct target_waitstatus *status, int options)
{
+ struct sim_inferior_data *sim_data;
static RETSIGTYPE (*prev_sigint) ();
int sigrc = 0;
enum sim_stop reason = sim_running;
+ /* This target isn't able to (yet) resume more than one inferior at a time.
+ When ptid is minus_one_ptid, just use the current inferior. If we're
+ given an explicit pid, we'll try to find it and use that instead. */
+ if (ptid_equal (ptid, minus_one_ptid))
+ sim_data = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
+ else
+ {
+ sim_data = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NEEDED);
+ if (sim_data == NULL)
+ error (_("Unable to wait for pid %d. Inferior not found."),
+ ptid_get_pid (ptid));
+ inferior_ptid = ptid;
+ }
+
if (remote_debug)
printf_filtered ("gdbsim_wait\n");
@@ -702,11 +989,13 @@ gdbsim_wait (struct target_ops *ops,
#else
prev_sigint = signal (SIGINT, gdbsim_cntrl_c);
#endif
- sim_resume (gdbsim_desc, resume_step, resume_siggnal);
+ sim_resume (sim_data->gdbsim_desc, sim_data->resume_step,
+ sim_data->resume_siggnal);
+
signal (SIGINT, prev_sigint);
- resume_step = 0;
+ sim_data->resume_step = 0;
- sim_stop_reason (gdbsim_desc, &reason, &sigrc);
+ sim_stop_reason (sim_data->gdbsim_desc, &reason, &sigrc);
switch (reason)
{
@@ -764,15 +1053,26 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
int write, struct mem_attrib *attrib,
struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
/* If no program is running yet, then ignore the simulator for
memory. Pass the request down to the next target, hopefully
an exec file. */
if (!target_has_execution)
return 0;
- if (!program_loaded)
+ if (!sim_data->program_loaded)
error (_("No program loaded."));
+ /* Note that we obtained the sim_data pointer above using
+ SIM_INSTANCE_NOT_NEEDED. We do this so that we don't needlessly
+ allocate a sim instance prior to loading a program. If we
+ get to this point in the code though, gdbsim_desc should be
+ non-NULL. (Note that a sim instance is needed in order to load
+ the program...) */
+ gdb_assert (sim_data->gdbsim_desc != NULL);
+
if (remote_debug)
{
/* FIXME: Send to something other than STDOUT? */
@@ -786,11 +1086,11 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
if (write)
{
- len = sim_write (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_write (sim_data->gdbsim_desc, memaddr, myaddr, len);
}
else
{
- len = sim_read (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_read (sim_data->gdbsim_desc, memaddr, myaddr, len);
if (remote_debug && len > 0)
dump_mem (myaddr, len);
}
@@ -800,6 +1100,8 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
static void
gdbsim_files_info (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
const char *file = "nothing";
if (exec_bfd)
@@ -812,7 +1114,7 @@ gdbsim_files_info (struct target_ops *ta
{
printf_filtered ("\tAttached to %s running program %s\n",
target_shortname, file);
- sim_info (gdbsim_desc, 0);
+ sim_info (sim_data->gdbsim_desc, 0);
}
}
@@ -821,12 +1123,15 @@ gdbsim_files_info (struct target_ops *ta
static void
gdbsim_mourn_inferior (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_mourn_inferior:\n");
remove_breakpoints ();
generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
+ delete_thread_silent (sim_data->remote_sim_ptid);
}
/* Pass the command argument through to the simulator verbatim. The
@@ -835,7 +1140,20 @@ gdbsim_mourn_inferior (struct target_ops
void
simulator_command (char *args, int from_tty)
{
- if (gdbsim_desc == NULL)
+ struct sim_inferior_data *sim_data;
+
+ /* We use inferior_data() instead of get_sim_inferior_data() here in
+ order to avoid attaching a sim_inferior_data struct to an
+ inferior unnecessarily. The reason we take such care here is due
+ to the fact that this function, simulator_command(), may be called
+ even when the sim target is not active. If we were to use
+ get_sim_inferior_data() here, it is possible that this call would
+ be made either prior to gdbsim_open() or after gdbsim_close(),
+ thus allocating memory that would not be garbage collected until
+ the ultimate destruction of the associated inferior. */
+
+ sim_data = inferior_data (current_inferior (), sim_inferior_data_key);
+ if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
{
/* PREVIOUSLY: The user may give a command before the simulator
@@ -851,7 +1169,7 @@ simulator_command (char *args, int from_
error (_("Not connected to the simulator target"));
}
- sim_do_command (gdbsim_desc, args);
+ sim_do_command (sim_data->gdbsim_desc, args);
/* Invalidate the register cache, in case the simulator command does
something funny. */
@@ -863,7 +1181,13 @@ simulator_command (char *args, int from_
static int
gdbsim_thread_alive (struct target_ops *ops, ptid_t ptid)
{
- if (ptid_equal (ptid, remote_sim_ptid))
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED);
+
+ if (sim_data == NULL)
+ return 0;
+
+ if (ptid_equal (ptid, sim_data->remote_sim_ptid))
/* The simulators' task is always alive. */
return 1;
@@ -876,14 +1200,6 @@ gdbsim_thread_alive (struct target_ops *
static char *
gdbsim_pid_to_str (struct target_ops *ops, ptid_t ptid)
{
- static char buf[64];
-
- if (ptid_equal (remote_sim_ptid, ptid))
- {
- xsnprintf (buf, sizeof buf, "Thread <main>");
- return buf;
- }
-
return normal_pid_to_str (ptid);
}
@@ -934,7 +1250,6 @@ _initialize_remote_sim (void)
add_com ("sim", class_obscure, simulator_command,
_("Send a command to the simulator."));
- /* Yes, 42000 is arbitrary. The only sense out of it, is that it
- isn't 0. */
- remote_sim_ptid = ptid_build (42000, 0, 42000);
+ sim_inferior_data_key
+ = register_inferior_data_with_cleanup (sim_inferior_data_cleanup);
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-08-06 23:21 ` Kevin Buettner
@ 2010-08-08 16:43 ` Pedro Alves
2010-08-10 0:21 ` Kevin Buettner
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2010-08-08 16:43 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
On Saturday 07 August 2010 00:20:43, Kevin Buettner wrote:
> +static int
> +gdbsim_close_inferior (struct inferior *inf, void *arg)
> +{
> + struct sim_inferior_data *sim_data = inferior_data (inf,
> + sim_inferior_data_key);
> + if (sim_data != NULL)
> + {
> + ptid_t ptid = sim_data->remote_sim_ptid;
> +
> + sim_inferior_data_cleanup (inf, sim_data);
> + set_inferior_data (inf, sim_inferior_data_key, NULL);
> +
> + inferior_ptid = ptid;
Please do "switch_to_thread (ptid)" instead, so that the
program space and current inferior globals are kept
in sync with inferior_ptid while mourning.
> + generic_mourn_inferior ();
> + }
> +
> + return 0;
> }
>
> +static int
> +gdbsim_stop_inferior (struct inferior *inf, void *arg)
> +{
> + struct sim_inferior_data *sim_data
> + = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED);
> +
> + if (sim_data)
> + {
> + /* Try to stop the sim. If it can't stop, exit GDB altogether. */
> + if (!sim_stop (sim_data->gdbsim_desc))
> + {
> + quit ();
> + }
> + }
> +
Calling quit doesn't exit GDB; it throws the same exception ^C usually
ends up throwing, cancelling the whole current operation all the way back
to the top CLI prompt. (Well, usually. Some places catch all
exceptions instead of just RETURN_MASK_ERROR.
Other than that, it looked good to me. Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] remote-sim.c: Add support for multiple sim instances
2010-08-08 16:43 ` Pedro Alves
@ 2010-08-10 0:21 ` Kevin Buettner
0 siblings, 0 replies; 9+ messages in thread
From: Kevin Buettner @ 2010-08-10 0:21 UTC (permalink / raw)
To: gdb-patches
On Sun, 8 Aug 2010 17:42:57 +0100
Pedro Alves <pedro@codesourcery.com> wrote:
> On Saturday 07 August 2010 00:20:43, Kevin Buettner wrote:
>
> > +static int
> > +gdbsim_close_inferior (struct inferior *inf, void *arg)
> > +{
> > + struct sim_inferior_data *sim_data = inferior_data (inf,
> > + sim_inferior_data_key);
> > + if (sim_data != NULL)
> > + {
> > + ptid_t ptid = sim_data->remote_sim_ptid;
> > +
> > + sim_inferior_data_cleanup (inf, sim_data);
> > + set_inferior_data (inf, sim_inferior_data_key, NULL);
> > +
> > + inferior_ptid = ptid;
>
> Please do "switch_to_thread (ptid)" instead, so that the
> program space and current inferior globals are kept
> in sync with inferior_ptid while mourning.
I did this, but testing showed an assertion failure, inf != NULL,
when the target was opened and then closed without creating an
inferior.
I ended up checking to see whether the inferior had been created prior
to invoking switch_to_thread() and generic_mourn_inferior().
Here's what I ended up with:
static int
gdbsim_close_inferior (struct inferior *inf, void *arg)
{
struct sim_inferior_data *sim_data = inferior_data (inf,
sim_inferior_data_key);
if (sim_data != NULL)
{
ptid_t ptid = sim_data->remote_sim_ptid;
sim_inferior_data_cleanup (inf, sim_data);
set_inferior_data (inf, sim_inferior_data_key, NULL);
/* Having a ptid allocated and stored in remote_sim_ptid does
not mean that a corresponding inferior was ever created.
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_pid (ptid_get_pid (ptid)) != NULL)
{
switch_to_thread (ptid);
generic_mourn_inferior ();
}
}
return 0;
}
> > +static int
> > +gdbsim_stop_inferior (struct inferior *inf, void *arg)
> > +{
> > + struct sim_inferior_data *sim_data
> > + = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED);
> > +
> > + if (sim_data)
> > + {
> > + /* Try to stop the sim. If it can't stop, exit GDB altogether. */
> > + if (!sim_stop (sim_data->gdbsim_desc))
> > + {
> > + quit ();
> > + }
> > + }
> > +
>
> Calling quit doesn't exit GDB; it throws the same exception ^C usually
> ends up throwing, cancelling the whole current operation all the way back
> to the top CLI prompt. (Well, usually. Some places catch all
> exceptions instead of just RETURN_MASK_ERROR.
This really had me puzzled for a while because quit() - defined in
utils.c - calls fatal(). When I looked up fatal, I didn't look
closely at the name of the file it was defined in, but I did see that
it printed a message and then called exit(). Problem was, I was
looking at the fatal() defined in gdbserver/utils.c, rather than the
definition of fatal in utils.c. And, of course, the definition of
fatal() in utils.c does exactly what you say it does.
I ended up deleting that comment.
> Other than that, it looked good to me. Thanks!
Thank you for the patch reviews!
Below is the patch which I've just committed:
* remote-sim.c (program_loaded, gdbsim_desc, remote_sim_ptid)
(resume_siggnal, resume_step): Move these static globals...
(struct sim_inferior_data): ...into this new struct.
(sim_inferior_data_key, next_pid, sim_argv, gdbsim_is_open):
New static globals.
(gdb_callback, callbacks_initialized): Move these globals to
a point earlier in the file.
(check_for_duplicate_sim_descriptor, get_sim_inferior_data)
(get_sim_inferior_data_by_ptid, sim_inferior_data_cleanup)
(gdbsim_close_inferior, gdbsim_resume_inferior)
(gdbsim_stop_inferior): New functions.
(SIM_INSTANCE_NOT_NEEDED, SIM_INSTANCE_NEEDED, INITIAL_PID):
New constants.
(gdbsim_fetch_register, gdbsim_store_register, gdbsim_load)
(gdbsim_create_inferior, gdbsim_open, gdbsim_close, gdbsim_resume)
(gdbsim_stop, gdbsim_cntrl_c, gdbsim_wait)
(gdbsim_xfer_inferior_memory, gdbsim_files_info)
(gdbsim_mourn_inferior, simulator_command, gdbsim_thread_alive,
(gdbsim_pid_to_str): Invoke `get_sim_inferior_data' to set
new local variable `sim_data' in each of these functions. Use
`sim_data' to reference former globals `program_loaded',
`gdbsim_desc', `remote_sim_ptid', `resume_siggnal', and
`resume_step'.
(gdbsim_open): Remove local variable `argv'. Put results of call
to `gdb_buildargv' in `sim_argv' rather than in `argv'. Don't
make a cleanup for it. Free it though when a sim instance cannot
be obtained.
(gdbsim_close): Free sim_argv and null it out as appropriate.
Close sim instances in all inferiors.
(gdbsim_cntrl_c): Stop all inferiors.
(gdbsim_wait):
(_initialize_remote_sim): Initialize `sim_inferior_data_key'.
Index: remote-sim.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-sim.c,v
retrieving revision 1.96
diff -u -p -r1.96 remote-sim.c
--- remote-sim.c 16 May 2010 21:11:14 -0000 1.96
+++ remote-sim.c 10 Aug 2010 00:12:36 -0000
@@ -101,19 +101,175 @@ void simulator_command (char *args, int
/* Forward data declarations */
extern struct target_ops gdbsim_ops;
-static int program_loaded = 0;
+static const struct inferior_data *sim_inferior_data_key;
-/* We must keep track of whether the simulator has been opened or not because
- GDB can call a target's close routine twice, but sim_close doesn't allow
- this. We also need to record the result of sim_open so we can pass it
- back to the other sim_foo routines. */
-static SIM_DESC gdbsim_desc = 0;
-
-/* This is the ptid we use while we're connected to the simulator.
- Its value is arbitrary, as the simulator target don't have a notion
- or processes or threads, but we need something non-null to place in
- inferior_ptid. */
-static ptid_t remote_sim_ptid;
+/* Simulator-specific, per-inferior state. */
+struct sim_inferior_data {
+ /* Flag which indicates whether or not the program has been loaded. */
+ int program_loaded;
+
+ /* 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 target_signal resume_siggnal;
+
+ /* Flag which indicates whether resume should step or not. */
+ int resume_step;
+};
+
+/* Flag indicating the "open" status of this module. It's set to 1
+ in gdbsim_open() and 0 in gdbsim_close(). */
+static int gdbsim_is_open = 0;
+
+/* 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
+
+/* Argument list to pass to sim_open(). It is allocated in gdbsim_open()
+ and deallocated in gdbsim_close(). The lifetime needs to extend beyond
+ the call to gdbsim_open() due to the fact that other sim instances other
+ than the first will be allocated after the gdbsim_open() call. */
+static char **sim_argv = NULL;
+
+/* OS-level callback functions for write, flush, etc. */
+static host_callback gdb_callback;
+static int callbacks_initialized = 0;
+
+/* Callback for iterate_over_inferiors. It checks to see if the sim
+ descriptor passed via ARG is the same as that for the inferior
+ designated by INF. Return true if so; false otherwise. */
+
+static int
+check_for_duplicate_sim_descriptor (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data;
+ SIM_DESC new_sim_desc = arg;
+
+ sim_data = inferior_data (inf, sim_inferior_data_key);
+
+ return (sim_data != NULL && sim_data->gdbsim_desc == new_sim_desc);
+}
+
+/* Flags indicating whether or not a sim instance is needed. One of these
+ flags should be passed to get_sim_inferior_data(). */
+
+enum {SIM_INSTANCE_NOT_NEEDED = 0, SIM_INSTANCE_NEEDED = 1};
+
+/* Obtain pointer to per-inferior simulator data, allocating it if necessary.
+ Attempt to open the sim if SIM_INSTANCE_NEEDED is true. */
+
+static struct sim_inferior_data *
+get_sim_inferior_data (struct inferior *inf, int sim_instance_needed)
+{
+ SIM_DESC sim_desc = NULL;
+ struct sim_inferior_data *sim_data
+ = inferior_data (inf, sim_inferior_data_key);
+
+ /* Try to allocate a new sim instance, if needed. We do this ahead of
+ a potential allocation of a sim_inferior_data struct in order to
+ avoid needlessly allocating that struct in the event that the sim
+ instance allocation fails. */
+ if (sim_instance_needed == SIM_INSTANCE_NEEDED
+ && (sim_data == NULL || sim_data->gdbsim_desc == NULL))
+ {
+ struct inferior *idup;
+ sim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
+ if (sim_desc == NULL)
+ error (_("Unable to create simulator instance for inferior %d."),
+ inf->num);
+
+ idup = iterate_over_inferiors (check_for_duplicate_sim_descriptor,
+ sim_desc);
+ if (idup != NULL)
+ {
+ /* We don't close the descriptor due to the fact that it's
+ shared with some other inferior. If we were to close it,
+ that might needlessly muck up the other inferior. Of
+ course, it's possible that the damage has already been
+ done... Note that it *will* ultimately be closed during
+ cleanup of the other inferior. */
+ sim_desc = NULL;
+ error (
+ _("Inferior %d and inferior %d would have identical simulator state.\n"
+ "(This simulator does not support the running of more than one inferior.)"),
+ inf->num, idup->num);
+ }
+ }
+
+ if (sim_data == NULL)
+ {
+ sim_data = XZALLOC(struct sim_inferior_data);
+ set_inferior_data (inf, sim_inferior_data_key, sim_data);
+
+ /* Allocate a ptid for this inferior. */
+ sim_data->remote_sim_ptid = ptid_build (next_pid, 0, next_pid);
+ next_pid++;
+
+ /* Initialize the other instance variables. */
+ sim_data->program_loaded = 0;
+ sim_data->gdbsim_desc = sim_desc;
+ sim_data->resume_siggnal = TARGET_SIGNAL_0;
+ sim_data->resume_step = 0;
+ }
+ else if (sim_desc)
+ {
+ /* This handles the case where sim_data was allocated prior to
+ needing a sim instance. */
+ sim_data->gdbsim_desc = sim_desc;
+ }
+
+
+ return sim_data;
+}
+
+/* Return pointer to per-inferior simulator data using PTID to find the
+ 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)
+{
+ struct inferior *inf;
+ int pid = ptid_get_pid (ptid);
+
+ if (pid <= 0)
+ return NULL;
+
+ inf = find_inferior_pid (pid);
+
+ if (inf)
+ return get_sim_inferior_data (inf, sim_instance_needed);
+ else
+ return NULL;
+}
+
+/* Free the per-inferior simulator data. */
+
+static void
+sim_inferior_data_cleanup (struct inferior *inf, void *data)
+{
+ struct sim_inferior_data *sim_data = data;
+
+ if (sim_data != NULL)
+ {
+ if (sim_data->gdbsim_desc)
+ {
+ sim_close (sim_data->gdbsim_desc, 0);
+ sim_data->gdbsim_desc = NULL;
+ }
+ xfree (sim_data);
+ }
+}
static void
dump_mem (char *buf, int len)
@@ -142,9 +298,6 @@ dump_mem (char *buf, int len)
}
}
-static host_callback gdb_callback;
-static int callbacks_initialized = 0;
-
/* Initialize gdb_callback. */
static void
@@ -278,6 +431,8 @@ gdbsim_fetch_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
if (regno == -1)
{
@@ -310,7 +465,7 @@ gdbsim_fetch_register (struct target_ops
gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
memset (buf, 0, MAX_REGISTER_SIZE);
- nr_bytes = sim_fetch_register (gdbsim_desc,
+ nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
buf,
@@ -350,6 +505,9 @@ gdbsim_store_register (struct target_ops
struct regcache *regcache, int regno)
{
struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
+
if (regno == -1)
{
for (regno = 0; regno < gdbarch_num_regs (gdbarch); regno++)
@@ -362,7 +520,7 @@ gdbsim_store_register (struct target_ops
int nr_bytes;
regcache_cooked_read (regcache, regno, tmp);
- nr_bytes = sim_store_register (gdbsim_desc,
+ nr_bytes = sim_store_register (sim_data->gdbsim_desc,
gdbarch_register_sim_regno
(gdbarch, regno),
tmp, register_size (gdbarch, regno));
@@ -404,6 +562,8 @@ gdbsim_load (char *args, int fromtty)
{
char **argv;
char *prog;
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
if (args == NULL)
error_no_arg (_("program to load"));
@@ -422,13 +582,13 @@ gdbsim_load (char *args, int fromtty)
/* FIXME: We will print two messages on error.
Need error to either not print anything if passed NULL or need
another routine that doesn't take any arguments. */
- if (sim_load (gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
+ if (sim_load (sim_data->gdbsim_desc, prog, NULL, fromtty) == SIM_RC_FAIL)
error (_("unable to load program"));
/* FIXME: If a load command should reset the targets registers then
a call to sim_create_inferior() should go here. */
- program_loaded = 1;
+ sim_data->program_loaded = 1;
}
@@ -444,12 +604,14 @@ static void
gdbsim_create_inferior (struct target_ops *target, char *exec_file, char *args,
char **env, int from_tty)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
int len;
char *arg_buf, **argv;
if (exec_file == 0 || exec_bfd == 0)
warning (_("No executable file specified."));
- if (!program_loaded)
+ if (!sim_data->program_loaded)
warning (_("No program loaded."));
if (remote_debug)
@@ -457,7 +619,7 @@ gdbsim_create_inferior (struct target_op
(exec_file ? exec_file : "(NULL)"),
args);
- if (ptid_equal (inferior_ptid, remote_sim_ptid))
+ if (ptid_equal (inferior_ptid, sim_data->remote_sim_ptid))
gdbsim_kill (target);
remove_breakpoints ();
init_wait_for_inferior ();
@@ -475,9 +637,9 @@ gdbsim_create_inferior (struct target_op
}
else
argv = NULL;
- sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
+ sim_create_inferior (sim_data->gdbsim_desc, exec_bfd, argv, env);
- inferior_ptid = remote_sim_ptid;
+ inferior_ptid = sim_data->remote_sim_ptid;
inferior_appeared (current_inferior (), ptid_get_pid (inferior_ptid));
add_thread_silent (inferior_ptid);
@@ -496,18 +658,20 @@ gdbsim_open (char *args, int from_tty)
{
int len;
char *arg_buf;
- char **argv;
+ struct sim_inferior_data *sim_data;
+ SIM_DESC gdbsim_desc;
if (remote_debug)
printf_filtered ("gdbsim_open: args \"%s\"\n", args ? args : "(null)");
- /* Remove current simulator if one exists. Only do this if the simulator
- has been opened because sim_close requires it.
- This is important because the call to push_target below will cause
- sim_close to be called if the simulator is already open, but push_target
- is called after sim_open! We can't move the call to push_target before
- the call to sim_open because sim_open may invoke `error'. */
- if (gdbsim_desc != NULL)
+ /* Ensure that the sim target is not on the target stack. This is
+ necessary, because if it is on the target stack, the call to
+ push_target below will invoke sim_close(), thus freeing various
+ state (including a sim instance) that we allocate prior to
+ invoking push_target(). We want to delay the push_target()
+ operation until after we complete those operations which could
+ error out. */
+ if (gdbsim_is_open)
unpush_target (&gdbsim_ops);
len = (7 + 1 /* gdbsim */
@@ -543,14 +707,26 @@ gdbsim_open (char *args, int from_tty)
strcat (arg_buf, " "); /* 1 */
strcat (arg_buf, args);
}
- argv = gdb_buildargv (arg_buf);
- make_cleanup_freeargv (argv);
+ sim_argv = gdb_buildargv (arg_buf);
init_callbacks ();
- gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, argv);
+ gdbsim_desc = sim_open (SIM_OPEN_DEBUG, &gdb_callback, exec_bfd, sim_argv);
if (gdbsim_desc == 0)
- error (_("unable to create simulator instance"));
+ {
+ freeargv (sim_argv);
+ sim_argv = NULL;
+ error (_("unable to create simulator instance"));
+ }
+
+ /* Reset the pid numberings for this batch of sim instances. */
+ next_pid = INITIAL_PID;
+
+ /* Allocate the inferior data, but do not allocate a sim instance
+ since we've already just done that. */
+ sim_data = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
+ sim_data->gdbsim_desc = gdbsim_desc;
push_target (&gdbsim_ops);
printf_filtered ("Connected to the simulator.\n");
@@ -558,6 +734,38 @@ gdbsim_open (char *args, int from_tty)
/* There's nothing running after "target sim" or "load"; not until
"run". */
inferior_ptid = null_ptid;
+
+ gdbsim_is_open = 1;
+}
+
+/* Callback for iterate_over_inferiors. Called (indirectly) by
+ gdbsim_close(). */
+
+static int
+gdbsim_close_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data = inferior_data (inf,
+ sim_inferior_data_key);
+ if (sim_data != NULL)
+ {
+ ptid_t ptid = sim_data->remote_sim_ptid;
+
+ sim_inferior_data_cleanup (inf, sim_data);
+ set_inferior_data (inf, sim_inferior_data_key, NULL);
+
+ /* Having a ptid allocated and stored in remote_sim_ptid does
+ not mean that a corresponding inferior was ever created.
+ 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_pid (ptid_get_pid (ptid)) != NULL)
+ {
+ switch_to_thread (ptid);
+ generic_mourn_inferior ();
+ }
+ }
+
+ return 0;
}
/* Does whatever cleanup is required for a target that we are no longer
@@ -572,21 +780,23 @@ gdbsim_open (char *args, int from_tty)
static void
gdbsim_close (int quitting)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_close: quitting %d\n", quitting);
- program_loaded = 0;
+ iterate_over_inferiors (gdbsim_close_inferior, NULL);
- if (gdbsim_desc != NULL)
+ if (sim_argv != NULL)
{
- sim_close (gdbsim_desc, quitting);
- gdbsim_desc = NULL;
+ freeargv (sim_argv);
+ sim_argv = NULL;
}
end_callbacks ();
- generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
- delete_inferior_silent (ptid_get_pid (remote_sim_ptid));
+
+ gdbsim_is_open = 0;
}
/* Takes a program previously attached to and detaches it.
@@ -613,21 +823,59 @@ gdbsim_detach (struct target_ops *ops, c
or to run free; SIGGNAL is the signal value (e.g. SIGINT) to be given
to the target, or zero for no signal. */
-static enum target_signal resume_siggnal;
-static int resume_step;
+struct resume_data
+{
+ enum target_signal siggnal;
+ int step;
+};
+
+static int
+gdbsim_resume_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (inf, SIM_INSTANCE_NOT_NEEDED);
+ struct resume_data *rd = arg;
+
+ if (sim_data)
+ {
+ sim_data->resume_siggnal = rd->siggnal;
+ sim_data->resume_step = rd->step;
+
+ if (remote_debug)
+ printf_filtered (_("gdbsim_resume: pid %d, step %d, signal %d\n"),
+ inf->pid, rd->step, rd->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;
+}
static void
gdbsim_resume (struct target_ops *ops,
ptid_t ptid, int step, enum target_signal siggnal)
{
- if (!ptid_equal (inferior_ptid, remote_sim_ptid))
+ 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;
+
+ /* We don't access any sim_data members within this function.
+ What's of interest is whether or not the call to
+ get_sim_inferior_data_by_ptid(), above, is able to obtain a
+ non-NULL pointer. If it managed to obtain a non-NULL pointer, we
+ know we have a single inferior to consider. If it's NULL, we
+ either have multiple inferiors to resume or an error condition. */
+
+ if (sim_data)
+ gdbsim_resume_inferior (find_inferior_pid (ptid_get_pid (ptid)), &rd);
+ else if (ptid_equal (ptid, minus_one_ptid))
+ iterate_over_inferiors (gdbsim_resume_inferior, &rd);
+ else
error (_("The program is not being run."));
-
- if (remote_debug)
- printf_filtered ("gdbsim_resume: step %d, signal %d\n", step, siggnal);
-
- resume_siggnal = siggnal;
- resume_step = step;
}
/* Notify the simulator of an asynchronous request to stop.
@@ -637,14 +885,45 @@ gdbsim_resume (struct target_ops *ops,
simulator is not running then the stop request is processed when
the simulator is next resumed.
- For simulators that do not support this operation, just abort */
+ For simulators that do not support this operation, just abort. */
+
+static int
+gdbsim_stop_inferior (struct inferior *inf, void *arg)
+{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED);
+
+ if (sim_data)
+ {
+ if (!sim_stop (sim_data->gdbsim_desc))
+ {
+ quit ();
+ }
+ }
+
+ /* 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;
+}
static void
gdbsim_stop (ptid_t ptid)
{
- if (!sim_stop (gdbsim_desc))
+ struct sim_inferior_data *sim_data;
+
+ if (ptid_equal (ptid, minus_one_ptid))
{
- quit ();
+ iterate_over_inferiors (gdbsim_stop_inferior, NULL);
+ }
+ else
+ {
+ struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+
+ if (inf == NULL)
+ error (_("Can't stop pid %d. No inferior found."), ptid_get_pid (ptid));
+
+ gdbsim_stop_inferior (inf, NULL);
}
}
@@ -676,17 +955,32 @@ gdb_os_poll_quit (host_callback *p)
static void
gdbsim_cntrl_c (int signo)
{
- gdbsim_stop (remote_sim_ptid);
+ gdbsim_stop (minus_one_ptid);
}
static ptid_t
gdbsim_wait (struct target_ops *ops,
ptid_t ptid, struct target_waitstatus *status, int options)
{
+ struct sim_inferior_data *sim_data;
static RETSIGTYPE (*prev_sigint) ();
int sigrc = 0;
enum sim_stop reason = sim_running;
+ /* This target isn't able to (yet) resume more than one inferior at a time.
+ When ptid is minus_one_ptid, just use the current inferior. If we're
+ given an explicit pid, we'll try to find it and use that instead. */
+ if (ptid_equal (ptid, minus_one_ptid))
+ sim_data = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
+ else
+ {
+ sim_data = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NEEDED);
+ if (sim_data == NULL)
+ error (_("Unable to wait for pid %d. Inferior not found."),
+ ptid_get_pid (ptid));
+ inferior_ptid = ptid;
+ }
+
if (remote_debug)
printf_filtered ("gdbsim_wait\n");
@@ -702,11 +996,13 @@ gdbsim_wait (struct target_ops *ops,
#else
prev_sigint = signal (SIGINT, gdbsim_cntrl_c);
#endif
- sim_resume (gdbsim_desc, resume_step, resume_siggnal);
+ sim_resume (sim_data->gdbsim_desc, sim_data->resume_step,
+ sim_data->resume_siggnal);
+
signal (SIGINT, prev_sigint);
- resume_step = 0;
+ sim_data->resume_step = 0;
- sim_stop_reason (gdbsim_desc, &reason, &sigrc);
+ sim_stop_reason (sim_data->gdbsim_desc, &reason, &sigrc);
switch (reason)
{
@@ -764,15 +1060,26 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
int write, struct mem_attrib *attrib,
struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
/* If no program is running yet, then ignore the simulator for
memory. Pass the request down to the next target, hopefully
an exec file. */
if (!target_has_execution)
return 0;
- if (!program_loaded)
+ if (!sim_data->program_loaded)
error (_("No program loaded."));
+ /* Note that we obtained the sim_data pointer above using
+ SIM_INSTANCE_NOT_NEEDED. We do this so that we don't needlessly
+ allocate a sim instance prior to loading a program. If we
+ get to this point in the code though, gdbsim_desc should be
+ non-NULL. (Note that a sim instance is needed in order to load
+ the program...) */
+ gdb_assert (sim_data->gdbsim_desc != NULL);
+
if (remote_debug)
{
/* FIXME: Send to something other than STDOUT? */
@@ -786,11 +1093,11 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
if (write)
{
- len = sim_write (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_write (sim_data->gdbsim_desc, memaddr, myaddr, len);
}
else
{
- len = sim_read (gdbsim_desc, memaddr, myaddr, len);
+ len = sim_read (sim_data->gdbsim_desc, memaddr, myaddr, len);
if (remote_debug && len > 0)
dump_mem (myaddr, len);
}
@@ -800,6 +1107,8 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
static void
gdbsim_files_info (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NEEDED);
const char *file = "nothing";
if (exec_bfd)
@@ -812,7 +1121,7 @@ gdbsim_files_info (struct target_ops *ta
{
printf_filtered ("\tAttached to %s running program %s\n",
target_shortname, file);
- sim_info (gdbsim_desc, 0);
+ sim_info (sim_data->gdbsim_desc, 0);
}
}
@@ -821,12 +1130,15 @@ gdbsim_files_info (struct target_ops *ta
static void
gdbsim_mourn_inferior (struct target_ops *target)
{
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
+
if (remote_debug)
printf_filtered ("gdbsim_mourn_inferior:\n");
remove_breakpoints ();
generic_mourn_inferior ();
- delete_thread_silent (remote_sim_ptid);
+ delete_thread_silent (sim_data->remote_sim_ptid);
}
/* Pass the command argument through to the simulator verbatim. The
@@ -835,7 +1147,20 @@ gdbsim_mourn_inferior (struct target_ops
void
simulator_command (char *args, int from_tty)
{
- if (gdbsim_desc == NULL)
+ struct sim_inferior_data *sim_data;
+
+ /* We use inferior_data() instead of get_sim_inferior_data() here in
+ order to avoid attaching a sim_inferior_data struct to an
+ inferior unnecessarily. The reason we take such care here is due
+ to the fact that this function, simulator_command(), may be called
+ even when the sim target is not active. If we were to use
+ get_sim_inferior_data() here, it is possible that this call would
+ be made either prior to gdbsim_open() or after gdbsim_close(),
+ thus allocating memory that would not be garbage collected until
+ the ultimate destruction of the associated inferior. */
+
+ sim_data = inferior_data (current_inferior (), sim_inferior_data_key);
+ if (sim_data == NULL || sim_data->gdbsim_desc == NULL)
{
/* PREVIOUSLY: The user may give a command before the simulator
@@ -851,7 +1176,7 @@ simulator_command (char *args, int from_
error (_("Not connected to the simulator target"));
}
- sim_do_command (gdbsim_desc, args);
+ sim_do_command (sim_data->gdbsim_desc, args);
/* Invalidate the register cache, in case the simulator command does
something funny. */
@@ -863,7 +1188,13 @@ simulator_command (char *args, int from_
static int
gdbsim_thread_alive (struct target_ops *ops, ptid_t ptid)
{
- if (ptid_equal (ptid, remote_sim_ptid))
+ struct sim_inferior_data *sim_data
+ = get_sim_inferior_data_by_ptid (ptid, SIM_INSTANCE_NOT_NEEDED);
+
+ if (sim_data == NULL)
+ return 0;
+
+ if (ptid_equal (ptid, sim_data->remote_sim_ptid))
/* The simulators' task is always alive. */
return 1;
@@ -876,14 +1207,6 @@ gdbsim_thread_alive (struct target_ops *
static char *
gdbsim_pid_to_str (struct target_ops *ops, ptid_t ptid)
{
- static char buf[64];
-
- if (ptid_equal (remote_sim_ptid, ptid))
- {
- xsnprintf (buf, sizeof buf, "Thread <main>");
- return buf;
- }
-
return normal_pid_to_str (ptid);
}
@@ -934,7 +1257,6 @@ _initialize_remote_sim (void)
add_com ("sim", class_obscure, simulator_command,
_("Send a command to the simulator."));
- /* Yes, 42000 is arbitrary. The only sense out of it, is that it
- isn't 0. */
- remote_sim_ptid = ptid_build (42000, 0, 42000);
+ sim_inferior_data_key
+ = register_inferior_data_with_cleanup (sim_inferior_data_cleanup);
}
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-08-10 0:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-02 22:30 [RFA] remote-sim.c: Add support for multiple sim instances Kevin Buettner
2010-07-08 20:08 ` Tom Tromey
2010-07-27 23:07 ` Kevin Buettner
2010-07-27 23:14 ` Pedro Alves
2010-07-27 23:31 ` Kevin Buettner
2010-08-03 12:05 ` Pedro Alves
2010-08-06 23:21 ` Kevin Buettner
2010-08-08 16:43 ` Pedro Alves
2010-08-10 0:21 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).