[MI non-stop 10/11] Skip varobj in running threads.

[MI non-stop 10/11] Skip varobj in running threads.

[Vladimir Prus]

If a variable object is bound to a specific thread, and we're doing

  -var-update *

and varobj's thread is running, we cannot update varobj -- so we skip it.
Will commit when core non-stop is in.

- Volodya

	* mi/mi-cmd-var.c (mi_cmd_var_update): If varobj's
	thread is running, skip the varobj.
	* Makefile.in: Update dependencies.
---
 gdb/Makefile.in     |    3 ++-
 gdb/mi/mi-cmd-var.c |   16 ++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 779b99a..ceff555 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -3256,7 +3256,8 @@ mi-cmd-target.o: $(srcdir)/mi/mi-cmd-target.c $(defs_h) $(mi_cmds_h) \
 	$(mi_getopt_h) $(remote_h)
 	$(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-target.c
 mi-cmd-var.o: $(srcdir)/mi/mi-cmd-var.c $(defs_h) $(mi_cmds_h) $(ui_out_h) \
-	$(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h) $(mi_getopt_h)
+	$(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h) $(mi_getopt_h) \
+	$(gdbthread_h)
 	$(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-var.c
 mi-console.o: $(srcdir)/mi/mi-console.c $(defs_h) $(mi_console_h) \
 	$(gdb_string_h)
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 0adcf1f..d348cb5 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -29,6 +29,7 @@
 #include <ctype.h>
 #include "gdb_string.h"
 #include "mi-getopt.h"
+#include "gdbthread.h"
 
 const char mi_no_values[] = "--no-values";
 const char mi_simple_values[] = "--simple-values";
@@ -610,8 +611,19 @@ mi_cmd_var_update (char *command, char **argv, int argc)
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  if (*name == '*' || varobj_floating_p (*cr))
-	    varobj_update_one (*cr, print_values, 0 /* implicit */);
+	  int thread_id = varobj_get_thread_id (*cr);
+	  int thread_running = 0;
+	  if (thread_id == 0 && is_executing (inferior_ptid))
+	    thread_running = 1;
+	  else if (thread_id > 0)
+	    {
+	      struct thread_info *tp = find_thread_id (thread_id);
+	      if (tp)
+		thread_running = is_running (tp->ptid);
+	    }
+	  if (!thread_running)
+	    if (*name == '*' || varobj_floating_p (*cr))
+	      varobj_update_one (*cr, print_values, 0 /* implicit */);
 	  cr++;
 	}
       do_cleanups (cleanup);
-- 
1.5.3.5

Re: [MI non-stop 10/11] Skip varobj in running threads.

[Daniel Jacobowitz]

On Sat, Jun 28, 2008 at 09:00:06PM +0400, Vladimir Prus wrote:
> 
> If a variable object is bound to a specific thread, and we're doing
> 
>   -var-update *
> 
> and varobj's thread is running, we cannot update varobj -- so we skip it.
> Will commit when core non-stop is in.

I think this needs to go in the manual.  Would it be helpful to
indicate to the front end that a variable could not be updated due to
a running thread?

If a varobj is bound to a particular thread that usually means it's
bound to a particular frame, right?  In which case reading it while
the target is running doesn't make (much) sense, so there's nothing
target specific about this.
> +	  if (thread_id == 0 && is_executing (inferior_ptid))
> +	    thread_running = 1;
> +	  else if (thread_id > 0)
> +	    {
> +	      struct thread_info *tp = find_thread_id (thread_id);
> +	      if (tp)
> +		thread_running = is_running (tp->ptid);
> +	    }

Why is_running in one place and is_executing in the other?

Should we try reading from something besides inferior_ptid for unbound
varobjs?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 10/11] Skip varobj in running threads.

[Vladimir Prus]

On Friday 11 July 2008 17:53:35 Daniel Jacobowitz wrote:
> On Sat, Jun 28, 2008 at 09:00:06PM +0400, Vladimir Prus wrote:
> > 
> > If a variable object is bound to a specific thread, and we're doing
> > 
> >   -var-update *
> > 
> > and varobj's thread is running, we cannot update varobj -- so we skip it.
> > Will commit when core non-stop is in.
> 
> I think this needs to go in the manual.  Would it be helpful to
> indicate to the front end that a variable could not be updated due to
> a running thread?

Possibly -- the frontend may want to decorate varobj from running threads
in specific ways. OTOH, until non-stop frontends actually appear, we don't
know exact requirements, so I'd prefer to introduce bare minimum -- which
is not falling over trying evaluate expressions in running threads.

> If a varobj is bound to a particular thread that usually means it's
> bound to a particular frame, right?  

Yes. Assuming non-always-a-thread targets do not exist, varobj is bound to a 
particular thread if and only if it's bound to some frame.

> In which case reading it while 
> the target is running doesn't make (much) sense, so there's nothing
> target specific about this.

Right. Even if target can technically access memory of a running thread,
there's no way to evaluate such a varobj since we don't have no stack.

> > +	  if (thread_id == 0 && is_executing (inferior_ptid))
> > +	    thread_running = 1;
> > +	  else if (thread_id > 0)
> > +	    {
> > +	      struct thread_info *tp = find_thread_id (thread_id);
> > +	      if (tp)
> > +		thread_running = is_running (tp->ptid);
> > +	    }
> 
> Why is_running in one place and is_executing in the other?

It's a mistake, should be is_running.

> Should we try reading from something besides inferior_ptid for unbound
> varobjs?

The current MI/non-stop design assumes that commands are executed in a
specific thread. If it's running, and command needs to access target, we get
an error. This is more predictable than picking random thread. Some targets,
like break, can implicitly use the current frame, and switching to random
thread will make them behave erratically.

- Volodya

Re: [MI non-stop 10/11] Skip varobj in running threads.

[Pedro Alves]

On Saturday 28 June 2008 18:00:06, Vladimir Prus wrote:

>
>  const char mi_no_values[] = "--no-values";
>  const char mi_simple_values[] = "--simple-values";
> @@ -610,8 +611,19 @@ mi_cmd_var_update (char *command, char **argv, int
> argc) cr = rootlist;
>        while (*cr != NULL)
>  	{
> -	  if (*name == '*' || varobj_floating_p (*cr))
> -	    varobj_update_one (*cr, print_values, 0 /* implicit */);
> +	  int thread_id = varobj_get_thread_id (*cr);
> +	  int thread_running = 0;
> +	  if (thread_id == 0 && is_executing (inferior_ptid))
> +	    thread_running = 1;
> +	  else if (thread_id > 0)
> +	    {
> +	      struct thread_info *tp = find_thread_id (thread_id);
> +	      if (tp)
> +		thread_running = is_running (tp->ptid);
> +	    }
> +	  if (!thread_running)
> +	    if (*name == '*' || varobj_floating_p (*cr))
> +	      varobj_update_one (*cr, print_values, 0 /* implicit */);

You should reverse the logic, and do the update if the thread is_stopped
instead of !is_running.  That is because there is now exited state
to care for as well, and is_running also returns false on those.  Essentially,
the is_STATE functions should not be tested with negation, if you're looking
for a particular state.

-- 
Pedro Alves

Re: [MI non-stop 10/11] Skip varobj in running threads.

[Daniel Jacobowitz]

On Fri, Jul 11, 2008 at 02:56:39PM +0100, Pedro Alves wrote:
> You should reverse the logic, and do the update if the thread is_stopped
> instead of !is_running.  That is because there is now exited state
> to care for as well, and is_running also returns false on those.  Essentially,
> the is_STATE functions should not be tested with negation, if you're looking
> for a particular state.

Sounds like we need a big comment for that :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [MI non-stop 10/11] Skip varobj in running threads.

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

Vladimir Prus wrote:

> 
> If a variable object is bound to a specific thread, and we're doing
> 
>   -var-update *
> 
> and varobj's thread is running, we cannot update varobj -- so we skip it.
> Will commit when core non-stop is in.

I've checked in the following revision.

- Volodya



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: commit.diff --]
[-- Type: text/x-diff; name="commit.diff", Size: 2832 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.9558
diff -u -p -r1.9558 ChangeLog
--- gdb/ChangeLog	13 Jul 2008 04:13:49 -0000	1.9558
+++ gdb/ChangeLog	13 Jul 2008 05:29:56 -0000
@@ -1,5 +1,12 @@
 2008-07-13  Vladimir Prus  <vladimir@codesourcery.com>
 
+	Skip varobj in running threads.
+        * mi/mi-cmd-var.c (mi_cmd_var_update): If varobj's
+        thread is not stopped, skip the varobj.
+        * Makefile.in: Update dependencies.
+
+2008-07-13  Vladimir Prus  <vladimir@codesourcery.com>
+
 	Enable all commands while inferiour is running
         * mi/mi-main.c (mi_cmd_execute): Don't check if
         inferiour is executing.
Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1036
diff -u -p -r1.1036 Makefile.in
--- gdb/Makefile.in	12 Jul 2008 22:16:34 -0000	1.1036
+++ gdb/Makefile.in	13 Jul 2008 05:29:58 -0000
@@ -3256,7 +3256,8 @@ mi-cmd-target.o: $(srcdir)/mi/mi-cmd-tar
 	$(mi_getopt_h) $(remote_h)
 	$(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-target.c
 mi-cmd-var.o: $(srcdir)/mi/mi-cmd-var.c $(defs_h) $(mi_cmds_h) $(ui_out_h) \
-	$(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h) $(mi_getopt_h)
+	$(mi_out_h) $(varobj_h) $(value_h) $(gdb_string_h) $(mi_getopt_h) \
+	$(gdbthread_h)
 	$(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/mi/mi-cmd-var.c
 mi-console.o: $(srcdir)/mi/mi-console.c $(defs_h) $(mi_console_h) \
 	$(gdb_string_h)
Index: gdb/mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.52
diff -u -p -r1.52 mi-cmd-var.c
--- gdb/mi/mi-cmd-var.c	25 Jun 2008 15:15:42 -0000	1.52
+++ gdb/mi/mi-cmd-var.c	13 Jul 2008 05:30:17 -0000
@@ -29,6 +29,7 @@
 #include <ctype.h>
 #include "gdb_string.h"
 #include "mi-getopt.h"
+#include "gdbthread.h"
 
 const char mi_no_values[] = "--no-values";
 const char mi_simple_values[] = "--simple-values";
@@ -610,8 +611,21 @@ mi_cmd_var_update (char *command, char *
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  if (*name == '*' || varobj_floating_p (*cr))
-	    varobj_update_one (*cr, print_values, 0 /* implicit */);
+	  int thread_id = varobj_get_thread_id (*cr);
+	  int thread_stopped = 0;
+	  if (thread_id == -1 && is_stopped (inferior_ptid))
+	    thread_stopped = 1;
+	  else
+	    {	      
+	      struct thread_info *tp = find_thread_id (thread_id);
+	      if (tp)
+		thread_stopped = is_stopped (tp->ptid);
+	      else
+		thread_stopped = 1;
+	    }
+	  if (thread_stopped)
+	    if (*name == '*' || varobj_floating_p (*cr))
+	      varobj_update_one (*cr, print_values, 0 /* implicit */);
 	  cr++;
 	}
       do_cleanups (cleanup);