[rfc] Stop unlikely "run"'s earlier

[rfc] Stop unlikely "run"'s earlier

[Daniel Jacobowitz]

When you connect to a remote target using "target remote", it's
already running.  A common mistake at this point is to say "run"
instead of "continue".  If you do, GDB first asks you to kill the
currently running program, and then depending on its configuration
does one of two things.  A cross debugger will say "Don't know how to
run".  A native debugger will run the program... natively, not on
the remote target you were previously connected to!

I don't think this behavior is useful.  "run" should select a native
target when the current target is an executable or core file, but
not when it's connected to some other target.  This patch changes
the behavior to:

(gdb) run
The "remote" target can not run programs.  Try "help target" or "continue".

Any comments?  Shall I commit this?

-- 
Daniel Jacobowitz
CodeSourcery

2007-11-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* infcmd.c (kill_if_already_running): Make static.  Use
	target_require_runnable.
	* target.c (target_require_runnable): New.
	* target.h (target_require_runnable): Declare.

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.162
diff -u -p -r1.162 infcmd.c
--- infcmd.c	25 Oct 2007 11:30:55 -0000	1.162
+++ infcmd.c	7 Nov 2007 21:09:45 -0000
@@ -444,11 +444,15 @@ post_create_inferior (struct target_ops 
    from the beginning.  Ask the user to confirm that he wants to restart
    the program being debugged when FROM_TTY is non-null.  */
 
-void
+static void
 kill_if_already_running (int from_tty)
 {
   if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution)
     {
+      /* Bail out before killing the program if we will not be able to
+	 restart it.  */
+      target_require_runnable ();
+
       if (from_tty
 	  && !query ("The program being debugged has been started already.\n\
 Start it from the beginning? "))
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.149
diff -u -p -r1.149 target.c
--- target.c	22 Oct 2007 14:03:37 -0000	1.149
+++ target.c	7 Nov 2007 21:09:46 -0000
@@ -1730,6 +1730,41 @@ target_read_description (struct target_o
   return NULL;
 }
 
+/* Look through the currently pushed targets.  If none of them will
+   be able to restart the currently running process, issue an error
+   message.  */
+
+void
+target_require_runnable (void)
+{
+  struct target_ops *t;
+
+  for (t = target_stack; t != NULL; t = t->beneath)
+    {
+      /* If this target knows how to create a new program, then
+	 assume we will still be able to after killing the current
+	 one.  Either killing and mourning will not pop T, or else
+	 find_default_run_target will find it again.  */
+      if (t->to_create_inferior != NULL)
+	return;
+
+      /* Do not worry about thread_stratum targets that can not
+	 create inferiors.  Assume they will be pushed again if
+	 necessary, and continue to the process_stratum.  */
+      if (t->to_stratum == thread_stratum)
+	continue;
+
+      error (_("\
+The \"%s\" target can not run programs.  Try \"help target\" or \"continue\"."),
+	     t->to_shortname);
+    }
+
+  /* This function is only called if the target is running.  In that
+     case there should have been a process_stratum target and it
+     should either know how to create inferiors, or not... */
+  internal_error (__FILE__, __LINE__, "No targets found");
+}
+
 /* Look through the list of possible targets for a target that can
    execute a run or attach command without any other data.  This is
    used to locate the default process stratum.
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.104
diff -u -p -r1.104 target.h
--- target.h	23 Aug 2007 18:08:46 -0000	1.104
+++ target.h	7 Nov 2007 21:09:46 -0000
@@ -1203,6 +1203,8 @@ extern void initialize_targets (void);
 
 extern void noprocess (void);
 
+extern void target_require_runnable (void);
+
 extern void find_default_attach (char *, int);
 
 extern void find_default_create_inferior (char *, char *, char **, int);

Re: [rfc] Stop unlikely "run"'s earlier

[Eli Zaretskii]

> Date: Wed, 7 Nov 2007 16:18:33 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> I don't think this behavior is useful.  "run" should select a native
> target when the current target is an executable or core file, but
> not when it's connected to some other target.  This patch changes
> the behavior to:
> 
> (gdb) run
> The "remote" target can not run programs.  Try "help target" or "continue".
> 
> Any comments?  Shall I commit this?

I'm okay with the change, but how do you feel about updating the docs
as well?  Should we?

Re: [rfc] Stop unlikely "run"'s earlier

[Daniel Jacobowitz]

On Thu, Nov 08, 2007 at 12:06:16AM +0200, Eli Zaretskii wrote:
> I'm okay with the change, but how do you feel about updating the docs
> as well?  Should we?

I just looked at the description of "run" in the manual.  If anything,
this patch brings GDB better in line with its manual, but if you can
think of a way to describe the difference I'm all ears.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Stop unlikely "run"'s earlier

[Joel Brobecker]

> I don't think this behavior is useful.  "run" should select a native
> target when the current target is an executable or core file, but
> not when it's connected to some other target.  This patch changes
> the behavior to:
> 
> (gdb) run
> The "remote" target can not run programs.  Try "help target" or "continue".
> 
> Any comments?  Shall I commit this?

I agree this is a more useful behavior.

> 2007-11-07  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* infcmd.c (kill_if_already_running): Make static.  Use
> 	target_require_runnable.
> 	* target.c (target_require_runnable): New.
> 	* target.h (target_require_runnable): Declare.

Looks good to me.

-- 
Joel

Re: [rfc] Stop unlikely "run"'s earlier

[Eli Zaretskii]

> Date: Wed, 7 Nov 2007 17:17:30 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Thu, Nov 08, 2007 at 12:06:16AM +0200, Eli Zaretskii wrote:
> > I'm okay with the change, but how do you feel about updating the docs
> > as well?  Should we?
> 
> I just looked at the description of "run" in the manual.  If anything,
> this patch brings GDB better in line with its manual, but if you can
> think of a way to describe the difference I'm all ears.

Well, under "target remote" (in the node "Connecting") we say:

    Once the connection has been established, you can use all the usual
    commands to examine and change data and to step and continue the
    remote program.

Given you description, I'd say this should be augmented by explaining
that the program is already running at this stage, and telling the
user to use "continue", as opposed to "run" in local debugging.

Also, under "run" (in node "Starting"), it'd be a good thing to cite
the error message you want top introduce:

  The "remote" target can not run programs.  Try "help target" or "continue".

and tell the reader that the most probable cause here is that "run"
was used rather than "continue".  Without such an explanation, I'd be
quite lost if I saw this message (what do you mean ``can't run''? how
can I debug the darn thing it it cannot be run?)

WDYT?

Re: [rfc] Stop unlikely "run"'s earlier

[Daniel Jacobowitz]

On Sat, Nov 17, 2007 at 01:32:16PM +0200, Eli Zaretskii wrote:
> Well, under "target remote" (in the node "Connecting") we say:
> 
>     Once the connection has been established, you can use all the usual
>     commands to examine and change data and to step and continue the
>     remote program.
> 
> Given you description, I'd say this should be augmented by explaining
> that the program is already running at this stage, and telling the
> user to use "continue", as opposed to "run" in local debugging.
> 
> Also, under "run" (in node "Starting"), it'd be a good thing to cite
> the error message you want top introduce:
> 
>   The "remote" target can not run programs.  Try "help target" or "continue".
> 
> and tell the reader that the most probable cause here is that "run"
> was used rather than "continue".  Without such an explanation, I'd be
> quite lost if I saw this message (what do you mean ``can't run''? how
> can I debug the darn thing it it cannot be run?)
> 
> WDYT?

Hi Eli, sorry for the delay.  Is this documentation OK?  I also
changed the error message, to one I hope is slightly less confusing:

The "remote" target does not support "run".  Try "help target" or "continue".

-- 
Daniel Jacobowitz
CodeSourcery

2008-02-27  Daniel Jacobowitz  <dan@codesourcery.com>

	* infcmd.c (kill_if_already_running): Make static.  Use
	target_require_runnable.
	* target.c (target_require_runnable): New.
	* target.h (target_require_runnable): Declare.

2008-02-27  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.texinfo (Starting): Mention always-running targets.
	(Target Commands): Add an anchor for load.
	(Connecting): Explain continue instead of run.

Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.169
diff -u -p -r1.169 infcmd.c
--- infcmd.c	31 Jan 2008 13:37:21 -0000	1.169
+++ infcmd.c	27 Feb 2008 20:23:57 -0000
@@ -444,11 +444,15 @@ post_create_inferior (struct target_ops 
    from the beginning.  Ask the user to confirm that he wants to restart
    the program being debugged when FROM_TTY is non-null.  */
 
-void
+static void
 kill_if_already_running (int from_tty)
 {
   if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution)
     {
+      /* Bail out before killing the program if we will not be able to
+	 restart it.  */
+      target_require_runnable ();
+
       if (from_tty
 	  && !query ("The program being debugged has been started already.\n\
 Start it from the beginning? "))
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.154
diff -u -p -r1.154 target.c
--- target.c	25 Jan 2008 00:09:49 -0000	1.154
+++ target.c	27 Feb 2008 20:23:57 -0000
@@ -1723,6 +1723,41 @@ target_read_description (struct target_o
   return NULL;
 }
 
+/* Look through the currently pushed targets.  If none of them will
+   be able to restart the currently running process, issue an error
+   message.  */
+
+void
+target_require_runnable (void)
+{
+  struct target_ops *t;
+
+  for (t = target_stack; t != NULL; t = t->beneath)
+    {
+      /* If this target knows how to create a new program, then
+	 assume we will still be able to after killing the current
+	 one.  Either killing and mourning will not pop T, or else
+	 find_default_run_target will find it again.  */
+      if (t->to_create_inferior != NULL)
+	return;
+
+      /* Do not worry about thread_stratum targets that can not
+	 create inferiors.  Assume they will be pushed again if
+	 necessary, and continue to the process_stratum.  */
+      if (t->to_stratum == thread_stratum)
+	continue;
+
+      error (_("\
+The \"%s\" target does not support \"run\".  Try \"help target\" or \"continue\"."),
+	     t->to_shortname);
+    }
+
+  /* This function is only called if the target is running.  In that
+     case there should have been a process_stratum target and it
+     should either know how to create inferiors, or not... */
+  internal_error (__FILE__, __LINE__, "No targets found");
+}
+
 /* Look through the list of possible targets for a target that can
    execute a run or attach command without any other data.  This is
    used to locate the default process stratum.
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.110
diff -u -p -r1.110 target.h
--- target.h	20 Feb 2008 14:31:40 -0000	1.110
+++ target.h	27 Feb 2008 20:23:58 -0000
@@ -1188,6 +1188,8 @@ extern void initialize_targets (void);
 
 extern void noprocess (void);
 
+extern void target_require_runnable (void);
+
 extern void find_default_attach (char *, int);
 
 extern void find_default_create_inferior (char *, char *, char **, int);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.469
diff -u -p -r1.469 gdb.texinfo
--- doc/gdb.texinfo	25 Feb 2008 20:34:40 -0000	1.469
+++ doc/gdb.texinfo	27 Feb 2008 20:24:00 -0000
@@ -1818,8 +1818,19 @@ argument to @value{GDBN} (@pxref{Invocat
 
 If you are running your program in an execution environment that
 supports processes, @code{run} creates an inferior process and makes
-that process run your program.  (In environments without processes,
-@code{run} jumps to the start of your program.)
+that process run your program.  In some environments without processes,
+@code{run} jumps to the start of your program.  Other targets,
+like @samp{remote}, are always running.  If you get an error
+message like this one:
+
+@smallexample
+The "remote" target does not support "run".
+Try "help target" or "continue".
+@end smallexample
+
+@noindent
+then use @code{continue} to run your program.  You may need @code{load}
+first (@pxref{load}).
 
 The execution of a program is affected by certain information it
 receives from its superior.  @value{GDBN} provides ways to specify this
@@ -12669,6 +12680,7 @@ Show the current status of displaying co
 
 @kindex load @var{filename}
 @item load @var{filename}
+@anchor{load}
 Depending on what remote debugging facilities are configured into
 @value{GDBN}, the @code{load} command may be available.  Where it exists, it
 is meant to make @var{filename} (an executable) available for debugging
@@ -12847,8 +12859,9 @@ program has already exited, this will ha
 @end table
 
 Once the connection has been established, you can use all the usual
-commands to examine and change data and to step and continue the
-remote program.
+commands to examine and change data.  The remote program is already
+running; you can use @kbd{step} and @kbd{continue}, and you do not
+need to use @kbd{run}.
 
 @cindex interrupting remote programs
 @cindex remote programs, interrupting

Re: [rfc] Stop unlikely "run"'s earlier

[Eli Zaretskii]

> Date: Wed, 27 Feb 2008 15:28:04 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sourceware.org
> 
> Hi Eli, sorry for the delay.  Is this documentation OK?

Yes, thanks.

> I also
> changed the error message, to one I hope is slightly less confusing:
> 
> The "remote" target does not support "run".  Try "help target" or "continue".

Yes, it's fine now.

Re: [rfc] Stop unlikely "run"'s earlier

[Daniel Jacobowitz]

On Wed, Feb 27, 2008 at 11:25:31PM +0200, Eli Zaretskii wrote:
> > Date: Wed, 27 Feb 2008 15:28:04 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: gdb-patches@sourceware.org
> > 
> > Hi Eli, sorry for the delay.  Is this documentation OK?
> 
> Yes, thanks.

Thanks.  Checked in.

-- 
Daniel Jacobowitz
CodeSourcery