[MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Vladimir Prus]

There are several strategies to accepting commands when inferior_ptid is
running. One approach is to plain disallow all commands when inferior_ptid
is running.  This seems too strict.  Clearly, setting ignore count of 
a breakpoint does not require any access to the target at all.  Another 
approach is to document which commands may be allowed when the target is 
running. The problem is that each individual command may work or not work 
depending on the properties of the target. 

So, it's better to allow all commands up-front, and emit an error if we 
try an operation that the current target does not allow. This way, we'll 
never mistakenly prevent an operation that the target actually can perform.
In case of error, the frontend may show the error to the user, and user 
change either change his mind, or explicitly stop a thread, or ask the 
frontend to implicitly interrupt the target, or ask gdb to do same.

OK?

- Volodya

	* gdb/top.c (execute_command_1): Don't check if the inferiour
	is running.
---
 gdb/top.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index e09ccaa..a891a43 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -444,14 +444,6 @@ execute_command_1 (char *p, int from_tty, int internal)
  	  && !get_cmd_no_selected_thread_ok (c))
 	error (_("\
 Cannot execute this command without a selected thread.  See `help thread'"));
-      /* If the target is running, we allow only a limited set of
-         commands.  */
-      else if (target_can_async_p ()
-	       && target_has_execution
-	       && ((!non_stop && any_running ())
-		   || (non_stop && is_running (inferior_ptid)))
-	       && !get_cmd_async_ok (c))
-	error (_("Cannot execute this command while the target is running."));
 
       /* Pass null arg rather than an empty one.  */
       arg = *p ? p : 0;
-- 
1.5.3.5

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Pedro Alves]

A Saturday 28 June 2008 17:55:45, Vladimir Prus escreveu:
> There are several strategies to accepting commands when inferior_ptid is
> running. One approach is to plain disallow all commands when inferior_ptid
> is running.  This seems too strict.  Clearly, setting ignore count of
> a breakpoint does not require any access to the target at all.  Another
> approach is to document which commands may be allowed when the target is
> running. The problem is that each individual command may work or not work
> depending on the properties of the target.
>
> So, it's better to allow all commands up-front, and emit an error if we
> try an operation that the current target does not allow. This way, we'll
> never mistakenly prevent an operation that the target actually can perform.
> In case of error, the frontend may show the error to the user, and user
> change either change his mind, or explicitly stop a thread, or ask the
> frontend to implicitly interrupt the target, or ask gdb to do same.
>
> OK?
>

I think this is the right direction.

In many cases, the error that will come out of the target is just
plain non-sense, so we should catch the offensive command earlier
if possible.  E.g., reading registers from a running thread in linux
will error out with the same error as if the thread does not exist
at all, a very confusing error.
I even added the infrun.c:ensure_not_running function and its calls
to that effect.

Since only non-stop is affected by this, I think it's a sane direction.
Then we get to protect commands or make the targets throw more reasonable
errors as we find them.  Better than releasing GDB to users with non-stop,
then having them not being able to execute some command simply because
we forgot to that it as "safe".

The other perspective is that there will be surelly code paths that
may bork as not expecting an exception, hence the cleanups may not
be setup correctly.  But, those are bugs we should fix anyway.

> - Volodya
>
> 	* gdb/top.c (execute_command_1): Don't check if the inferiour
> 	is running.
> ---
>  gdb/top.c |    8 --------
>  1 files changed, 0 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/top.c b/gdb/top.c
> index e09ccaa..a891a43 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -444,14 +444,6 @@ execute_command_1 (char *p, int from_tty, int
> internal) && !get_cmd_no_selected_thread_ok (c))
>  	error (_("\
>  Cannot execute this command without a selected thread.  See `help
> thread'")); -      /* If the target is running, we allow only a limited set
> of -         commands.  */
> -      else if (target_can_async_p ()
> -	       && target_has_execution
> -	       && ((!non_stop && any_running ())
> -		   || (non_stop && is_running (inferior_ptid)))
> -	       && !get_cmd_async_ok (c))
> -	error (_("Cannot execute this command while the target is running."));
>
>        /* Pass null arg rather than an empty one.  */
>        arg = *p ? p : 0;



-- 
Pedro Alves

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Vladimir Prus]

On Friday 11 July 2008 17:46:36 Pedro Alves wrote:
> A Saturday 28 June 2008 17:55:45, Vladimir Prus escreveu:
> > There are several strategies to accepting commands when inferior_ptid is
> > running. One approach is to plain disallow all commands when inferior_ptid
> > is running.  This seems too strict.  Clearly, setting ignore count of
> > a breakpoint does not require any access to the target at all.  Another
> > approach is to document which commands may be allowed when the target is
> > running. The problem is that each individual command may work or not work
> > depending on the properties of the target.
> >
> > So, it's better to allow all commands up-front, and emit an error if we
> > try an operation that the current target does not allow. This way, we'll
> > never mistakenly prevent an operation that the target actually can perform.
> > In case of error, the frontend may show the error to the user, and user
> > change either change his mind, or explicitly stop a thread, or ask the
> > frontend to implicitly interrupt the target, or ask gdb to do same.
> >
> > OK?
> >
> 
> I think this is the right direction.
> 
> In many cases, the error that will come out of the target is just
> plain non-sense, so we should catch the offensive command earlier
> if possible.  E.g., reading registers from a running thread in linux
> will error out with the same error as if the thread does not exist
> at all, a very confusing error.
> I even added the infrun.c:ensure_not_running function and its calls
> to that effect.
> 
> Since only non-stop is affected by this, I think it's a sane direction.
> Then we get to protect commands or make the targets throw more reasonable
> errors as we find them.  Better than releasing GDB to users with non-stop,
> then having them not being able to execute some command simply because
> we forgot to that it as "safe".
> 
> The other perspective is that there will be surelly code paths that
> may bork as not expecting an exception, hence the cleanups may not
> be setup correctly.  But, those are bugs we should fix anyway.

I've of the same opinion. It seems to me that examining each and every command
is not quite practical. So, we either get to enable all commands and then disable,
or clarify error messages, or disable all commands, and enable those that actually
have to work. Enabling all commands seems safer, since the worst problem is inaccurate
error message.

- Volodya

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 10:58:24PM +0400, Vladimir Prus wrote:
> I've of the same opinion. It seems to me that examining each and every command
> is not quite practical. So, we either get to enable all commands and then disable,
> or clarify error messages, or disable all commands, and enable those that actually
> have to work. Enabling all commands seems safer, since the worst problem is inaccurate
> error message.

Well, or inconsistent data structures or assertion failures if
something changes underneath... anyway, good enough for me; it's OK.
Does this make the command flag obsolete?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Pedro Alves]

A Friday 11 July 2008 20:04:21, Daniel Jacobowitz wrote:
> On Fri, Jul 11, 2008 at 10:58:24PM +0400, Vladimir Prus wrote:
> > I've of the same opinion. It seems to me that examining each and every
> > command is not quite practical. So, we either get to enable all commands
> > and then disable, or clarify error messages, or disable all commands, and
> > enable those that actually have to work. Enabling all commands seems
> > safer, since the worst problem is inaccurate error message.

> Well, or inconsistent data structures or assertion failures if
> something changes underneath... anyway, good enough for me; it's OK.
> Does this make the command flag obsolete?

That has an easy workaround.  Don't call the command.  Disabling a
command that would otherwise execute fine does not have a workaround
(at least currently it doesn't).

-- 
Pedro Alves

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 03:04:21PM -0400, Daniel Jacobowitz wrote:
> Does this make the command flag obsolete?

This question remains.  Nothing calls get_cmd_async_ok now.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Vladimir Prus]

On Sunday 13 July 2008 08:26:08 Daniel Jacobowitz wrote:
> On Fri, Jul 11, 2008 at 03:04:21PM -0400, Daniel Jacobowitz wrote:
> > Does this make the command flag obsolete?
> 
> This question remains.  Nothing calls get_cmd_async_ok now.

Yes. A patch is forthcoming.

- Volodya

Re: [MI non-stop 07/11, RFA] Allow all CLI command even if target is executing.

[Daniel Jacobowitz]

On Sat, Jun 28, 2008 at 08:55:45PM +0400, Vladimir Prus wrote:
> There are several strategies to accepting commands when inferior_ptid is
> running. One approach is to plain disallow all commands when inferior_ptid
> is running.  This seems too strict.  Clearly, setting ignore count of 
> a breakpoint does not require any access to the target at all.  Another 
> approach is to document which commands may be allowed when the target is 
> running. The problem is that each individual command may work or not work 
> depending on the properties of the target. 
> 
> So, it's better to allow all commands up-front, and emit an error if we 
> try an operation that the current target does not allow. This way, we'll 
> never mistakenly prevent an operation that the target actually can perform.
> In case of error, the frontend may show the error to the user, and user 
> change either change his mind, or explicitly stop a thread, or ask the 
> frontend to implicitly interrupt the target, or ask gdb to do same.
> 
> OK?
> 
> - Volodya
> 
> 	* gdb/top.c (execute_command_1): Don't check if the inferiour
> 	is running.

What does this do to the quality of error messages?  Particularly for
targets other than Linux, or targets with async but not non-stop.

It's true that we'll have to get rid of this eventually, but can we do
it without having to think about each command individually?

"ignore" is an interesting example.  If a breakpoint has been ignored
twice and you lower the ignore count to one while the target is
running do we need to stop?

-- 
Daniel Jacobowitz
CodeSourcery