This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: Don't kill the program after "file"
Sorry for the delay getting back to this. It was breaking our
nightly simulator testing, too.
On Tue, Aug 01, 2006 at 06:11:05PM -0400, Daniel Jacobowitz wrote:
> The simulator is being a bit schizophrenic. It is not running, yet it
> is. This happens because inferior_ptid is set to null_ptid after kill
> or before execution, and the sim target is left on the stack, but
> to_has_execution is always set for "target sim".
>
> As it happens I have patches for a similar issue with target
> extended-remote. The remote patches are substantial, but I can
> probably break out the simulator related bits. I'll try to do that.
>
> They change target_has_execution from "is capable of running" to
> "is running right now".
Here's what I meant. This is a bit complicated, so I added a section
about it to the internals manual (and a little framework for
documenting the rest of struct target_ops, as people get inspired).
I'll just quote the new documentation:
A target vector can be completely inactive (not pushed on the target
stack), active but not running (pushed, but not connected to a fully
manifested inferior), or completely active (pushed, with an accessible
inferior). Most targets are only completely inactive or completely
active, but some support persistant connections to a target even
when the target has exited or not yet started.
For example, connecting to the simulator using @code{target sim} does
not create a running program. Neither registers nor memory are
accessible until @code{run}. Similarly, after @code{kill}, the
program can not continue executing. But in both cases @value{GDBN}
remains connected to the simulator, and target-specific commands
are directed to the simulator.
A target which only supports complete activation should push itself
onto the stack in its @code{to_open} routine (by calling
@code{push_target}), and unpush itself from the stack in its
@code{to_mourn_inferior} routine (by calling @code{unpush_target}).
A target which supports both partial and complete activation should
still call @code{push_target} in @code{to_open}, but not call
@code{unpush_target} in @code{to_mourn_inferior}. Instead, it should
call one of @code{target_mark_running} and @code{target_mark_exited}
in its @code{to_open}, depending on whether the target is fully active
after connection. It should also call @code{target_mark_running} any
time the inferior becomes fully active (e.g.@: in
@code{to_create_inferior} and @code{to_attach}), and
@code{target_mark_exited} when the inferior becomes inactive (in
@code{to_mourn_inferior}). The target should also make sure to call
@code{target_mourn_inferior} from its @code{to_kill}, to return the
target to inactive state.
This is a bit different than we had before, but I think it's fairly
clear and more useful. I tested this on arm-sim, where it removes a
bunch of ERRORs, by fixing "target sim; kill; file" - Fred reported
that "file" in that case warned that the target was already running,
even though we'd just issued a "kill".
I'll be using this approach for the extended-remote target, or a
similar variant, soon.
Any comments on this patch? Shall I commit it? Eli, do the
gdbint.texinfo changes look good to you?
--
Daniel Jacobowitz
CodeSourcery
2006-08-16 Daniel Jacobowitz <dan@codesourcery.com>
* gdbint.texinfo (Target Vector Definition): Move most
content into Existing Targets. Add a menu.
(Existing Targets): New section, moved from Target Vector
Definition. Use @subsection.
(Managing Execution State): New section.
2006-08-16 Daniel Jacobowitz <dan@codesourcery.com>
* remote-sim.c (gdbsim_kill): Call target_mourn_inferior.
(gdbsim_load): Don't bother to adjust inferior_ptid here.
(gdbsim_create_inferior): Mark the simulator as running.
(gdbsim_open): Don't bother fetching registers. Mark
the target as not running.
(gdbsim_xfer): When the program is not running, pass memory
requests down.
(gdbsim_mourn_inferior): Mark the target as not running.
* target.c (target_mark_running, target_mark_exited): New.
* target.h (target_has_execution): Update the comment.
(target_mark_running, target_mark_exited): New prototypes.
Index: gdb-mainline/gdb/doc/gdbint.texinfo
===================================================================
--- gdb-mainline.orig/gdb/doc/gdbint.texinfo 2006-05-15 00:20:01.000000000 -0700
+++ gdb-mainline/gdb/doc/gdbint.texinfo 2006-08-16 19:02:19.000000000 -0700
@@ -4405,11 +4405,56 @@ actually exercises control over a proces
@value{GDBN} includes some 30-40 different target vectors; however,
each configuration of @value{GDBN} includes only a few of them.
-@section File Targets
+@menu
+* Managing Execution State::
+* Existing Targets::
+@end menu
+
+@node Managing Execution State
+@section Managing Execution State
+@cindex execution state
+
+A target vector can be completely inactive (not pushed on the target
+stack), active but not running (pushed, but not connected to a fully
+manifested inferior), or completely active (pushed, with an accessible
+inferior). Most targets are only completely inactive or completely
+active, but some support persistant connections to a target even
+when the target has exited or not yet started.
+
+For example, connecting to the simulator using @code{target sim} does
+not create a running program. Neither registers nor memory are
+accessible until @code{run}. Similarly, after @code{kill}, the
+program can not continue executing. But in both cases @value{GDBN}
+remains connected to the simulator, and target-specific commands
+are directed to the simulator.
+
+A target which only supports complete activation should push itself
+onto the stack in its @code{to_open} routine (by calling
+@code{push_target}), and unpush itself from the stack in its
+@code{to_mourn_inferior} routine (by calling @code{unpush_target}).
+
+A target which supports both partial and complete activation should
+still call @code{push_target} in @code{to_open}, but not call
+@code{unpush_target} in @code{to_mourn_inferior}. Instead, it should
+call one of @code{target_mark_running} and @code{target_mark_exited}
+in its @code{to_open}, depending on whether the target is fully active
+after connection. It should also call @code{target_mark_running} any
+time the inferior becomes fully active (e.g.@: in
+@code{to_create_inferior} and @code{to_attach}), and
+@code{target_mark_exited} when the inferior becomes inactive (in
+@code{to_mourn_inferior}). The target should also make sure to call
+@code{target_mourn_inferior} from its @code{to_kill}, to return the
+target to inactive state.
+
+@node Existing Targets
+@section Existing Targets
+@cindex targets
+
+@subsection File Targets
Both executables and core files have target vectors.
-@section Standard Protocol and Remote Stubs
+@subsection Standard Protocol and Remote Stubs
@value{GDBN}'s file @file{remote.c} talks a serial protocol to code
that runs in the target system. @value{GDBN} provides several sample
@@ -4454,13 +4499,13 @@ of the debugger/stub.
From reading the stub, it's probably not obvious how breakpoints work.
They are simply done by deposit/examine operations from @value{GDBN}.
-@section ROM Monitor Interface
+@subsection ROM Monitor Interface
-@section Custom Protocols
+@subsection Custom Protocols
-@section Transport Layer
+@subsection Transport Layer
-@section Builtin Simulator
+@subsection Builtin Simulator
@node Native Debugging
Index: gdb-mainline/gdb/remote-sim.c
===================================================================
--- gdb-mainline.orig/gdb/remote-sim.c 2006-04-19 00:18:09.000000000 -0700
+++ gdb-mainline/gdb/remote-sim.c 2006-08-16 18:42:28.000000000 -0700
@@ -383,8 +383,8 @@ gdbsim_kill (void)
printf_filtered ("gdbsim_kill\n");
/* There is no need to `kill' running simulator - the simulator is
- not running */
- inferior_ptid = null_ptid;
+ not running. Mourning it is enough. */
+ target_mourn_inferior ();
}
/* Load an executable file into the target process. This is expected to
@@ -410,8 +410,6 @@ gdbsim_load (char *args, int fromtty)
if (sr_get_debug ())
printf_filtered ("gdbsim_load: prog \"%s\"\n", prog);
- inferior_ptid = null_ptid;
-
/* 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. */
@@ -469,6 +467,7 @@ gdbsim_create_inferior (char *exec_file,
sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
inferior_ptid = pid_to_ptid (42);
+ target_mark_running (&gdbsim_ops);
insert_breakpoints (); /* Needed to get correct instruction in cache */
clear_proceed_status ();
@@ -543,8 +542,12 @@ gdbsim_open (char *args, int from_tty)
error (_("unable to create simulator instance"));
push_target (&gdbsim_ops);
- target_fetch_registers (-1);
printf_filtered ("Connected to the simulator.\n");
+
+ /* There's nothing running after "target sim" or "load"; not until
+ "run". */
+ inferior_ptid = null_ptid;
+ target_mark_exited (&gdbsim_ops);
}
/* Does whatever cleanup is required for a target that we are no longer
@@ -747,6 +750,12 @@ gdbsim_xfer_inferior_memory (CORE_ADDR m
int write, struct mem_attrib *attrib,
struct target_ops *target)
{
+ /* 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)
error (_("No program loaded."));
@@ -802,6 +811,7 @@ gdbsim_mourn_inferior (void)
printf_filtered ("gdbsim_mourn_inferior:\n");
remove_breakpoints ();
+ target_mark_exited (&gdbsim_ops);
generic_mourn_inferior ();
}
Index: gdb-mainline/gdb/target.c
===================================================================
--- gdb-mainline.orig/gdb/target.c 2006-08-16 00:22:21.000000000 -0700
+++ gdb-mainline/gdb/target.c 2006-08-16 17:58:58.000000000 -0700
@@ -645,6 +645,56 @@ update_current_target (void)
current_target.beneath = target_stack;
}
+/* Mark OPS as a running target. This reverses the effect
+ of target_mark_exited. */
+
+void
+target_mark_running (struct target_ops *ops)
+{
+ struct target_ops *t;
+
+ for (t = target_stack; t != NULL; t = t->beneath)
+ if (t == ops)
+ break;
+ if (t == NULL)
+ internal_error (__FILE__, __LINE__,
+ "Attempted to mark unpushed target \"%s\" as running",
+ ops->to_shortname);
+
+ ops->to_has_execution = 1;
+ ops->to_has_all_memory = 1;
+ ops->to_has_memory = 1;
+ ops->to_has_stack = 1;
+ ops->to_has_registers = 1;
+
+ update_current_target ();
+}
+
+/* Mark OPS as a non-running target. This reverses the effect
+ of target_mark_running. */
+
+void
+target_mark_exited (struct target_ops *ops)
+{
+ struct target_ops *t;
+
+ for (t = target_stack; t != NULL; t = t->beneath)
+ if (t == ops)
+ break;
+ if (t == NULL)
+ internal_error (__FILE__, __LINE__,
+ "Attempted to mark unpushed target \"%s\" as running",
+ ops->to_shortname);
+
+ ops->to_has_execution = 0;
+ ops->to_has_all_memory = 0;
+ ops->to_has_memory = 0;
+ ops->to_has_stack = 0;
+ ops->to_has_registers = 0;
+
+ update_current_target ();
+}
+
/* Push a new target type into the stack of the existing target accessors,
possibly superseding some of the existing accessors.
Index: gdb-mainline/gdb/target.h
===================================================================
--- gdb-mainline.orig/gdb/target.h 2006-08-16 00:22:21.000000000 -0700
+++ gdb-mainline/gdb/target.h 2006-08-16 18:26:51.000000000 -0700
@@ -873,11 +873,12 @@ int target_follow_fork (int follow_child
(current_target.to_has_registers)
/* Does the target have execution? Can we make it jump (through
- hoops), or pop its stack a few times? FIXME: If this is to work that
- way, it needs to check whether an inferior actually exists.
- remote-udi.c and probably other targets can be the current target
- when the inferior doesn't actually exist at the moment. Right now
- this just tells us whether this target is *capable* of execution. */
+ hoops), or pop its stack a few times? This means that the current
+ target is currently executing; for some targets, that's the same as
+ whether or not the target is capable of execution, but there are
+ also targets which can be current while not executing. In that
+ case this will become true after target_create_inferior or
+ target_attach. */
#define target_has_execution \
(current_target.to_has_execution)
@@ -1131,6 +1132,13 @@ extern void target_preopen (int);
extern void pop_target (void);
+/* Mark a pushed target as running or exited, for targets which do not
+ automatically pop when not active. */
+
+void target_mark_running (struct target_ops *);
+
+void target_mark_exited (struct target_ops *);
+
/* Struct section_table maps address ranges to file sections. It is
mostly used with BFD files, but can be used without (e.g. for handling
raw disks, or files not in formats handled by BFD). */