MI: frozen variable objects

MI: frozen variable objects

[Vladimir Prus]

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


This patch introduces so called "frozen" variable objects -- variable objects 
that are not implicitly updated by the "-var-update *" command or by 
"-var-update" on parent variable objects.

Those are needed in two cases:

    - GUI might want to capture a value of some variable in the program
    and don't update it automatically. For example, so that at a later point
    you can look at the current values, and compare them with previous values.
    Now, MI does not provide any mechanism to do it.

    - Some values might be "read-sensitive" -- it might be some memory-mapped
    hardware register and reading from it will extact data from FIFO, or
    acknowledge an interrupt, so this is not something that can be done
    implicitly.

Frozen variable object is a mechanism to handle those use cases. The most 
important design aspects are:

   - A variable object can be made frozen either by explicit request from the
    user, or automatically by gdb, if gdb detect that the value is
    read-sensitive. The present patch does not implement such auto-detection
    but I'm prototyped such functionality too, and it will be coming later.

   - If a variable object is frozen, then the value of that object, or any
     children of it, will be fetched from the memory (via value_fetch_lazy)
     only by explicit -var-update for that variable object or a child. No
     other operations, importantly -var-update * and -var-update of parents
     of frozen variable object, and -var-list-children, will fetch the value.

   - It is possible that a frozen variable has no value fetched. In that 
    case -var-evaluate-expression will return empty string -- it won't
    implicitly fetch the value.

The patch is not fully finished -- it does not include testcase changes, and 
it does not include docs. Both will take quite some time, so I'd like to pass 
the code patch itself for review first.

Thanks,
Volodya

	* varobj.h (varobj_set_frozen): New
	(varobj_get_frozen): New.
	(varobj_update): New parameter explicit.
	* varobj.c (struct varobj): New fields frozen
	and not_fetched.
	(varobj_set_frozen, varobj_get_frozen): New.
	(install_new_value): Don't fetch values for 
	frozen variable object, or children thereof.  Allow
	a frozen variable object to have non-fetched value.
	(varobj_update): Allow updating child variables.
	Don't traverse frozen children.
	(new_variable): Initialize the frozen field.
	(c_value_of_variable): Return NULL for frozen
	variable without any value yet.
	* mi/mi-cmd-var.c (varobj_update_one): New parameter
	'explicit'.
	(mi_cmd_var_create): Output the 'frozen' field,
	as soon as testsuite is adjusted to expect that field.
	(mi_cmd_var_set_frozen): New.
	(mi_cmd_var_list_children): : Output the 'frozen' field,
	as soon as testsuite is adjusted to expect that field.
	(mi_cmd_var_update): Pass the 'explicit' parameter to
	varobj_update_one.
	* mi/mi-cmds.c (mi_cmds): Register '-var-set-frozen'.
	* mi/mi-cmds.h (mi_cmd_var_set_frozen): Declare.

[-- Attachment #2: frozen.diff --]
[-- Type: text/x-diff, Size: 15114 bytes --]

=== gdb/mi/mi-cmds.h
==================================================================
--- gdb/mi/mi-cmds.h	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmds.h	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -112,6 +112,7 @@
 extern mi_cmd_argv_ftype mi_cmd_var_info_type;
 extern mi_cmd_argv_ftype mi_cmd_var_list_children;
 extern mi_cmd_argv_ftype mi_cmd_var_set_format;
+extern mi_cmd_argv_ftype mi_cmd_var_set_frozen;
 extern mi_cmd_argv_ftype mi_cmd_var_show_attributes;
 extern mi_cmd_argv_ftype mi_cmd_var_show_format;
 extern mi_cmd_argv_ftype mi_cmd_var_update;
=== gdb/mi/mi-cmds.c
==================================================================
--- gdb/mi/mi-cmds.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmds.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -160,6 +160,7 @@
   { "var-info-type", { NULL, 0 }, 0, mi_cmd_var_info_type},
   { "var-list-children", { NULL, 0 }, 0, mi_cmd_var_list_children},
   { "var-set-format", { NULL, 0 }, 0, mi_cmd_var_set_format},
+  { "var-set-frozen", { NULL, 0 }, 0, mi_cmd_var_set_frozen},
   { "var-show-attributes", { NULL, 0 }, 0, mi_cmd_var_show_attributes},
   { "var-show-format", { NULL, 0 }, 0, mi_cmd_var_show_format},
   { "var-update", { NULL, 0 }, 0, mi_cmd_var_update},
=== gdb/mi/mi-cmd-var.c
==================================================================
--- gdb/mi/mi-cmd-var.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/mi/mi-cmd-var.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -37,7 +37,8 @@
 extern int varobjdebug;		/* defined in varobj.c */
 
 static int varobj_update_one (struct varobj *var,
-			      enum print_values print_values);
+			      enum print_values print_values,
+			      int explicit);
 
 /* VAROBJ operations */
 
@@ -108,6 +109,7 @@
       ui_out_field_string (uiout, "type", type);
       xfree (type);
     }
+  ui_out_field_int (uiout, "frozen", varobj_get_frozen (var)); 
 
   do_cleanups (old_cleanups);
   return MI_CMD_DONE;
@@ -215,6 +217,35 @@
 }
 
 enum mi_cmd_result
+mi_cmd_var_set_frozen (char *command, char **argv, int argc)
+{
+  struct varobj *var;
+  int frozen;
+
+  if (argc != 2)
+    error (_("mi_cmd_var_set_format: Usage: NAME FROZEN_FLAG."));
+
+  var = varobj_get_handle (argv[0]);
+  if (var == NULL)
+    error (_("Variable object not found"));
+
+  if (strcmp (argv[1], "0") == 0)
+    frozen = 0;
+  else if (strcmp (argv[1], "1") == 0)
+    frozen = 1;
+  else
+    error (_("Invalid flag value"));
+
+  varobj_set_frozen (var, frozen);
+
+  /* We don't automatically return the new value, or what varobjs got new
+     values during unfreezing.  If this information is required, client
+     should call -var-update explicitly.  */
+  return MI_CMD_DONE;
+}
+
+
+enum mi_cmd_result
 mi_cmd_var_show_format (char *command, char **argv, int argc)
 {
   enum varobj_display_formats format;
@@ -345,6 +376,7 @@
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
 	ui_out_field_string (uiout, "type", type);
+      ui_out_field_int (uiout, "frozen", varobj_get_frozen (*cc));
       do_cleanups (cleanup_child);
       cc++;
     }
@@ -503,7 +535,7 @@
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr, print_values);
+	  varobj_update_one (*cr, print_values, 0 /* implicit */);
 	  cr++;
 	}
       xfree (rootlist);
@@ -520,7 +552,7 @@
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var, print_values);
+      varobj_update_one (var, print_values, 1 /* explicit */);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -531,14 +563,15 @@
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var, enum print_values print_values)
+varobj_update_one (struct varobj *var, enum print_values print_values,
+		   int explicit)
 {
   struct varobj **changelist;
   struct varobj **cc;
   struct cleanup *cleanup = NULL;
   int nc;
 
-  nc = varobj_update (&var, &changelist);
+  nc = varobj_update (&var, &changelist, explicit);
 
   /* nc == 0 means that nothing has changed.
      nc == -1 means that an error occured in updating the variable.
=== gdb/varobj.c
==================================================================
--- gdb/varobj.c	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/varobj.c	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -103,7 +103,7 @@
 
   /* The value of this expression or subexpression.  This may be NULL. 
      Invariant: if type_changeable (this) is non-zero, the value is either
-     NULL, or not lazy.  */
+     NULL, or not lazy, or not_fetched is true.  */
   struct value *value;
 
   /* Did an error occur evaluating the expression or getting its value? */
@@ -126,6 +126,16 @@
 
   /* Was this variable updated via a varobj_set_value operation */
   int updated;
+
+  /* Is this variable frozen. Frozen variables are never implicitly
+     updated by -var-update * or -var-update <direct-or-indirect-parent>.
+  */
+  int frozen;
+
+  /* Is the value of this variable intentionally not fetched?  It is
+     not fetched if either the variable is frozen, or any parents is
+     frozen.  */
+  int not_fetched;
 };
 
 /* Every variable keeps a linked list of its children, described
@@ -682,7 +692,27 @@
   return var->format;
 }
 
+void
+varobj_set_frozen (struct varobj *var, int frozen)
+{
+  /* When a variable is unfrozen, we don't fetch its value.
+     The 'not_fetched' flag remains set, so next -var-update
+     won't complain.
+
+     We don't fetch the value, because for structures the client
+     should do -var-update anyway.  It would be bad to have different
+     client-size logic for structure and other types.  */
+  var->frozen = frozen;
+}
+
 int
+varobj_get_frozen (struct varobj *var)
+{
+  return var->frozen;
+}
+
+
+int
 varobj_get_num_children (struct varobj *var)
 {
   if (var->num_children == -1)
@@ -896,6 +926,7 @@
 { 
   int changeable;
   int changed = 0;
+  int intentionally_not_fetched = 0;
 
   var->error = 0;
   /* We need to know the varobj's type to decide if the value should
@@ -907,11 +938,24 @@
   /* The new value might be lazy.  If the type is changeable,
      that is we'll be comparing values of this type, fetch the
      value now.  Otherwise, on the next update the old value
-     will be lazy, which means we've lost that old value.  */
+     will be lazy, which means we've lost that old value.  */     
   if (changeable && value && value_lazy (value))
     {
-      if (!gdb_value_fetch_lazy (value))
+      struct varobj *parent = var->parent;
+      int frozen = var->frozen;
+      for (; !frozen && parent; parent = parent->parent)
+	frozen |= parent->frozen;
+
+      if (frozen && initial)
 	{
+	  /* For variables that are frozen, or are children of frozen
+	     variables, we don't do fetch on initial assignment.
+	     For non-initial assignemnt we do the fetch, since it means we're
+	     explicitly asked to compare the new value with the old one.  */
+	  intentionally_not_fetched = 1;
+	}
+      else if (!gdb_value_fetch_lazy (value))
+	{
 	  var->error = 1;
 	  /* Set the value to NULL, so that for the next -var-update,
 	     we don't try to compare the new value with this value,
@@ -937,28 +981,43 @@
 	{
 	  /* Try to compare the values.  That requires that both
 	     values are non-lazy.  */
-	  
-	  /* Quick comparison of NULL values.  */
-	  if (var->value == NULL && value == NULL)
-	    /* Equal. */
-	    ;
-	  else if (var->value == NULL || value == NULL)
-	    changed = 1;
+	  if (var->not_fetched && value_lazy (var->value))
+	    {
+	      /* This is a frozen varobj and the value was never read.
+		 Presumably, UI shows some "never read" indicator.
+		 Now that we've fetched the real value, we need to report
+		 this varobj as changed so that UI can show the real
+		 value.  */
+	      changed = 1;
+	    }
 	  else
-	    {
-	      gdb_assert (!value_lazy (var->value));
-	      gdb_assert (!value_lazy (value));
-	      
-	      if (!value_contents_equal (var->value, value))
+	    {	  
+	      /* Quick comparison of NULL values.  */
+	      if (var->value == NULL && value == NULL)
+		/* Equal. */
+		;
+	      else if (var->value == NULL || value == NULL)
 		changed = 1;
+	      else
+		{
+		  gdb_assert (!value_lazy (var->value));
+		  gdb_assert (!value_lazy (value));
+		  
+		  if (!value_contents_equal (var->value, value))
+		    changed = 1;
+		}
 	    }
 	}
     }
     
   /* We must always keep the new value, since children depend on it. */
-  if (var->value != NULL)
+  if (var->value != NULL && var->value != value)
     value_free (var->value);
   var->value = value;
+  if (value && value_lazy (value) && intentionally_not_fetched)
+    var->not_fetched = 1;
+  else
+    var->not_fetched = 0;
   var->updated = 0;
 
   return changed;
@@ -975,18 +1034,22 @@
     -2 if the type changed
     Otherwise it is the number of children + parent changed
 
-   Only root variables can be updated... 
+   The EXPLICIT parameter specifies if this call is result
+   of MI request to update this specific variable, or 
+   result of implicit -var-update *. For implicit request, we don't
+   update frozen variables.
 
    NOTE: This function may delete the caller's varobj. If it
    returns -2, then it has done this and VARP will be modified
    to point to the new varobj. */
 
 int
-varobj_update (struct varobj **varp, struct varobj ***changelist)
+varobj_update (struct varobj **varp, struct varobj ***changelist,
+	       int explicit)
 {
   int changed = 0;
   int error = 0;
-  int type_changed;
+  int type_changed = 0;
   int i;
   int vleft;
   struct varobj *v;
@@ -1002,57 +1065,70 @@
   if (changelist == NULL)
     return -1;
 
-  /*  Only root variables can be updated... */
-  if ((*varp)->root->rootvar != *varp)
-    /* Not a root var */
-    return -1;
 
+  /* Frozen means frozen -- we don't check for any change in
+     this varobj, including its going out of scope, or
+     changing type.  One use case for frozen varobjs is
+     retaining previously evaluated expressions, and we don't
+     want them to be reevaluated at all.  */
+  if (!explicit && (*varp)->frozen)
+    return 0;
+
   /* Save the selected stack frame, since we will need to change it
      in order to evaluate expressions. */
   old_fid = get_frame_id (deprecated_selected_frame);
 
-  /* Update the root variable. value_of_root can return NULL
-     if the variable is no longer around, i.e. we stepped out of
-     the frame in which a local existed. We are letting the 
-     value_of_root variable dispose of the varobj if the type
-     has changed. */
-  type_changed = 1;
-  new = value_of_root (varp, &type_changed);
-  if (new == NULL)
-    {
-      (*varp)->error = 1;
-      return -1;
-    }
-
-  /* Initialize a stack for temporary results */
+  /* Initialize a stack for temporary results.  */
   vpush (&result, NULL);
 
-  /* If this is a "use_selected_frame" varobj, and its type has changed,
-     them note that it's changed. */
-  if (type_changed)
-    {
-      vpush (&result, *varp);
-      changed++;
-    }
+  /* Initialize a stack for traversing children.  */
+  vpush (&stack, NULL);
 
-  if (install_new_value ((*varp), new, type_changed))
+  if ((*varp)->root->rootvar == *varp)
     {
-      /* If type_changed is 1, install_new_value will never return
-	 non-zero, so we'll never report the same variable twice.  */
-      gdb_assert (!type_changed);
-      vpush (&result, (*varp));
-      changed++;
+      /* Update the root variable. value_of_root can return NULL
+	 if the variable is no longer around, i.e. we stepped out of
+	 the frame in which a local existed. We are letting the 
+	 value_of_root variable dispose of the varobj if the type
+	 has changed. */
+      type_changed = 1;
+      new = value_of_root (varp, &type_changed);
+      if (new == NULL)
+	{
+	  (*varp)->error = 1;
+	  return -1;
+	}
+      
+      /* If this is a "use_selected_frame" varobj, and its type has changed,
+	 them note that it's changed. */
+      if (type_changed)
+	{
+	  vpush (&result, *varp);
+	  changed++;
+	}
+      
+      if (install_new_value ((*varp), new, type_changed))
+	{
+	  /* If type_changed is 1, install_new_value will never return
+	     non-zero, so we'll never report the same variable twice.  */
+	  gdb_assert (!type_changed);
+	  vpush (&result, (*varp));
+	  changed++;
+	}
+      
+      /* Push the root's children */
+      if ((*varp)->children != NULL)
+	{
+	  struct varobj_child *c;
+	  for (c = (*varp)->children; c != NULL; c = c->next)
+	    if (!c->child->frozen)
+	      vpush (&stack, c->child);
+	}
     }
-
-  /* Initialize a stack */
-  vpush (&stack, NULL);
-
-  /* Push the root's children */
-  if ((*varp)->children != NULL)
+  else
     {
-      struct varobj_child *c;
-      for (c = (*varp)->children; c != NULL; c = c->next)
-	vpush (&stack, c->child);
+      /* This is not a root variable.  Push it to stack itself.  */
+      vpush (&stack, *varp);
     }
 
   /* Walk through the children, reconstructing them all. */
@@ -1064,9 +1140,10 @@
 	{
 	  struct varobj_child *c;
 	  for (c = v->children; c != NULL; c = c->next)
-	    vpush (&stack, c->child);
+	    if (!c->child->frozen)
+	      vpush (&stack, c->child);
 	}
-
+      
       /* Update this variable */
       new = value_of_child (v->parent, v->index);
       if (install_new_value (v, new, 0 /* type not changed */))
@@ -1435,6 +1512,8 @@
   var->format = 0;
   var->root = NULL;
   var->updated = 0;
+  var->frozen = 0;
+  var->not_fetched = 0;
 
   return var;
 }
@@ -2112,6 +2191,12 @@
 	    struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
 	    char *thevalue;
 
+	    if (var->not_fetched && value_lazy (var->value))
+	      /* Frozen variable and no value yet.  We don't
+		 implicitly fetch the value.  MI response will
+		 use empty string for the value, which is OK.  */
+	      return NULL;
+
 	    gdb_assert (type_changeable (var));
 	    gdb_assert (!value_lazy (var->value));
 	    common_val_print (var->value, stb,
=== gdb/varobj.h
==================================================================
--- gdb/varobj.h	(/patches/gdb/laziness/gdb_mainline)	(revision 2118)
+++ gdb/varobj.h	(/patches/gdb/frozen/gdb_mainline)	(revision 2118)
@@ -78,6 +78,10 @@
 extern enum varobj_display_formats varobj_get_display_format (
 							struct varobj *var);
 
+extern void varobj_set_frozen (struct varobj *var, int frozen);
+
+extern int varobj_get_frozen (struct varobj *var);
+
 extern int varobj_get_num_children (struct varobj *var);
 
 extern int varobj_list_children (struct varobj *var,
@@ -97,6 +101,7 @@
 
 extern int varobj_list (struct varobj ***rootlist);
 
-extern int varobj_update (struct varobj **varp, struct varobj ***changelist);
+extern int varobj_update (struct varobj **varp, struct varobj ***changelist,
+			  int explicit);
 
 #endif /* VAROBJ_H */

Re: MI: frozen variable objects

[Greg Watson]

I don't really understand the motivation for putting this kind of  
functionality into gdb. Any GUI that is more than just a front-end  
for gdb will most likely have an internal data structure representing  
the value of the variable, and can retain or manage access to the  
value if required. It seems to me that adding functionality like this  
to gdb just adds to bloat when it is really a GUI function anyway.

Greg

On Nov 16, 2006, at 5:47 AM, Vladimir Prus wrote:

>
> This patch introduces so called "frozen" variable objects --  
> variable objects
> that are not implicitly updated by the "-var-update *" command or by
> "-var-update" on parent variable objects.
>
> Those are needed in two cases:
>
>     - GUI might want to capture a value of some variable in the  
> program
>     and don't update it automatically. For example, so that at a  
> later point
>     you can look at the current values, and compare them with  
> previous values.
>     Now, MI does not provide any mechanism to do it.
>
>     - Some values might be "read-sensitive" -- it might be some  
> memory-mapped
>     hardware register and reading from it will extact data from  
> FIFO, or
>     acknowledge an interrupt, so this is not something that can be  
> done
>     implicitly.
>
> Frozen variable object is a mechanism to handle those use cases.  
> The most
> important design aspects are:
>
>    - A variable object can be made frozen either by explicit  
> request from the
>     user, or automatically by gdb, if gdb detect that the value is
>     read-sensitive. The present patch does not implement such auto- 
> detection
>     but I'm prototyped such functionality too, and it will be  
> coming later.
>
>    - If a variable object is frozen, then the value of that object,  
> or any
>      children of it, will be fetched from the memory (via  
> value_fetch_lazy)
>      only by explicit -var-update for that variable object or a  
> child. No
>      other operations, importantly -var-update * and -var-update of  
> parents
>      of frozen variable object, and -var-list-children, will fetch  
> the value.
>
>    - It is possible that a frozen variable has no value fetched. In  
> that
>     case -var-evaluate-expression will return empty string -- it won't
>     implicitly fetch the value.
>
> The patch is not fully finished -- it does not include testcase  
> changes, and
> it does not include docs. Both will take quite some time, so I'd  
> like to pass
> the code patch itself for review first.
>
> Thanks,
> Volodya
>
> 	* varobj.h (varobj_set_frozen): New
> 	(varobj_get_frozen): New.
> 	(varobj_update): New parameter explicit.
> 	* varobj.c (struct varobj): New fields frozen
> 	and not_fetched.
> 	(varobj_set_frozen, varobj_get_frozen): New.
> 	(install_new_value): Don't fetch values for
> 	frozen variable object, or children thereof.  Allow
> 	a frozen variable object to have non-fetched value.
> 	(varobj_update): Allow updating child variables.
> 	Don't traverse frozen children.
> 	(new_variable): Initialize the frozen field.
> 	(c_value_of_variable): Return NULL for frozen
> 	variable without any value yet.
> 	* mi/mi-cmd-var.c (varobj_update_one): New parameter
> 	'explicit'.
> 	(mi_cmd_var_create): Output the 'frozen' field,
> 	as soon as testsuite is adjusted to expect that field.
> 	(mi_cmd_var_set_frozen): New.
> 	(mi_cmd_var_list_children): : Output the 'frozen' field,
> 	as soon as testsuite is adjusted to expect that field.
> 	(mi_cmd_var_update): Pass the 'explicit' parameter to
> 	varobj_update_one.
> 	* mi/mi-cmds.c (mi_cmds): Register '-var-set-frozen'.
> 	* mi/mi-cmds.h (mi_cmd_var_set_frozen): Declare.
> <frozen.diff>

Re: MI: frozen variable objects

[Frederic RISS]

On Thu, 2006-11-16 at 06:58 -0700, Greg Watson wrote:
> I don't really understand the motivation for putting this kind of  
> functionality into gdb. Any GUI that is more than just a front-end  
> for gdb will most likely have an internal data structure representing  
> the value of the variable, and can retain or manage access to the  
> value if required. It seems to me that adding functionality like this  
> to gdb just adds to bloat when it is really a GUI function anyway.

The interesting part seems to be the one that hasn't been submited yet
about GDB auto-freezing some values due to archtectural requirements.
The debugger has architectural knowledge and it shouldn't be necessary
to duplicate it in the GUI.
I agree with your general point though. Maybe the functionality
shouldn't be in the debugger, but the information should be available to
the GUI... through varobj properties maybe? Of course this means that
the GUIs have to known about and respect this information.

Fred.

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Thu, Nov 16, 2006 at 04:24:58PM +0100, Frederic RISS wrote:
> On Thu, 2006-11-16 at 06:58 -0700, Greg Watson wrote:
> > I don't really understand the motivation for putting this kind of  
> > functionality into gdb. Any GUI that is more than just a front-end  
> > for gdb will most likely have an internal data structure representing  
> > the value of the variable, and can retain or manage access to the  
> > value if required. It seems to me that adding functionality like this  
> > to gdb just adds to bloat when it is really a GUI function anyway.

Disclaimer: I've already talked with Vladimir about this at length and
if I remember right he's the one who persuaded me that GDB was the
right place to do it.  After this I'll let him speak for himself.

Why should the GUI have a separate data structure for the value in a
manipulatable form?  I suspect that some GUIs only keep it around as a
string, for display.  Then if the user wants a radix switch, the
easiest way is to tell GDB (-var-set-format).

The overall utility of -var-update * is somewhat suspect, I think,
since it can take so long to run; to make it practical you'd have to
delete varobjs for things not currently on-screen.  But as long as that
functionality exists, we figured it would be nice to not break it.

The read-sensitive values that Vladimir alluded to are our ultimate
goal here and you'll be seeing rather more of them in the next
few weeks.  Whenever we've suggested to a customer that our Eclipse
ought to pop up a window that shows all of their target's peripheral
I/O registers, the response has been jumping-up-and-down enthusiastic.
But without some solution for this problem we just can't do it.  Having
that window pop open, acknowledge all your pending interrupts, and
steal one byte from each serial port FIFO, would be a real downer.

> The interesting part seems to be the one that hasn't been submited yet
> about GDB auto-freezing some values due to archtectural requirements.
> The debugger has architectural knowledge and it shouldn't be necessary
> to duplicate it in the GUI.

Exactly.  My original sketch for this stuff had an XML file that we
passed from the target directly to the GUI, but that duplicates a
certain amount of work between GDB and the GUI, and between GUIs.

> I agree with your general point though. Maybe the functionality
> shouldn't be in the debugger, but the information should be available to
> the GUI... through varobj properties maybe? Of course this means that
> the GUIs have to known about and respect this information.

And what about people who actually use the command line GDB?  That was
the argument that tipped the scales for me in favor of this approach.
It allows the varobj mechanism to be consistent with what we want the
GDB CLI to offer to the user.

That's not immediately clear what I mean.  Here's a sample.  This is
mostly pipe dream, I don't think we're going to implement it right now. 
But suppose you have a type:

struct regs
{
  int InterruptStatus;
  int Config;
  int ClockVal;
} *regs;

Maybe Config is a static register, ClockVal is a continually updating
read-only timer, and InterruptStatus is a read-sensitive value where
reading acknowledges any reported interrupt (this is not uncommon). If
the user does "print/x *regs", and GDB can determine that yes, regs is
really pointing to some hardware registers that GDB knows about, it
would be awesome for GDB to say:

$1 = { InterruptStatus = <read-sensitive>, Config = 0x8000,
       ClockVal = 0x1212 }

I haven't thought too much about the UI here, so don't take it too
seriously.  The key is to warn the user before they mess with the
state of their board doing something that seems like it ought to
be harmless.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Frederic RISS]

On Thu, 2006-11-16 at 10:55 -0500, Daniel Jacobowitz wrote:
> Why should the GUI have a separate data structure for the value in a
> manipulatable form?  I suspect that some GUIs only keep it around as a
> string, for display.  Then if the user wants a radix switch, the
> easiest way is to tell GDB (-var-set-format).

[Not directly related to the varobj-freezing topic: I think that GUIs
had a better time doing exactly what you say rather than doing it in a
"I know better than the debugger"-way. I've already been quite irritated
with Eclipse doing endianness swapping by itself in its memory view.]

> > The interesting part seems to be the one that hasn't been submited yet
> > about GDB auto-freezing some values due to archtectural requirements.
> > The debugger has architectural knowledge and it shouldn't be necessary
> > to duplicate it in the GUI.
> 
> Exactly.  My original sketch for this stuff had an XML file that we
> passed from the target directly to the GUI, but that duplicates a
> certain amount of work between GDB and the GUI, and between GUIs.
> 
> > I agree with your general point though. Maybe the functionality
> > shouldn't be in the debugger, but the information should be available to
> > the GUI... through varobj properties maybe? Of course this means that
> > the GUIs have to known about and respect this information.
> 
> And what about people who actually use the command line GDB?  That was
> the argument that tipped the scales for me in favor of this approach.
> It allows the varobj mechanism to be consistent with what we want the
> GDB CLI to offer to the user.

Of course things should be consistent between frontends... but I think
that could be argument _against_ the varobj-freezing. If we take a look
at your example:

> struct regs
> {
>   int InterruptStatus;
>   int Config;
>   int ClockVal;
> } *regs;
> 
> Maybe Config is a static register, ClockVal is a continually updating
> read-only timer, and InterruptStatus is a read-sensitive value where
> reading acknowledges any reported interrupt (this is not uncommon). If
> the user does "print/x *regs", and GDB can determine that yes, regs is
> really pointing to some hardware registers that GDB knows about, it
> would be awesome for GDB to say:
> 
> $1 = { InterruptStatus = <read-sensitive>, Config = 0x8000,
>        ClockVal = 0x1212 }

The CLI doesn't use varobjs to display values. Vladimir's change
wouldn't directly help here. Maybe the framework for doing exactly that
should land first and then the UI. I suspect that if CLI is modified to
generate an output like you describe, MI would get something similar as
a side-effect, wouldn't it? 

Fred.

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Thu, Nov 16, 2006 at 05:25:19PM +0100, Frederic RISS wrote:
> The CLI doesn't use varobjs to display values. Vladimir's change
> wouldn't directly help here.

I have hopes that some day, MI will be sufficient to implement most of
the GDB CLI on top of it.  This would be an amazing improvement in
modularization of GDB.  Having MI change as a side-effect of CLI UI
changes is awful for maintaining compatibility.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Greg Watson]

On Nov 16, 2006, at 8:55 AM, Daniel Jacobowitz wrote:

> On Thu, Nov 16, 2006 at 04:24:58PM +0100, Frederic RISS wrote:
>> On Thu, 2006-11-16 at 06:58 -0700, Greg Watson wrote:
>>> I don't really understand the motivation for putting this kind of
>>> functionality into gdb. Any GUI that is more than just a front-end
>>> for gdb will most likely have an internal data structure  
>>> representing
>>> the value of the variable, and can retain or manage access to the
>>> value if required. It seems to me that adding functionality like  
>>> this
>>> to gdb just adds to bloat when it is really a GUI function anyway.
>
> Disclaimer: I've already talked with Vladimir about this at length and
> if I remember right he's the one who persuaded me that GDB was the
> right place to do it.  After this I'll let him speak for himself.
>
> Why should the GUI have a separate data structure for the value in a
> manipulatable form?  I suspect that some GUIs only keep it around as a
> string, for display.  Then if the user wants a radix switch, the
> easiest way is to tell GDB (-var-set-format).

I guess this is ok for GUI's that are just a front-end for gdb. The  
trend seems to be to provide the more sophisticated debugger  
functionality in the GUI and use gdb for lower-level debug  
operations, accessing symbols, etc. This is the case for two parallel  
debuggers I know about, and I suspect we'll see it used more for  
multicore architectures. Ultimately I think there will be  
functionality that can only be provided by the GUI. For example,  
comparing values from programs executing on different cores. I guess  
the trade-off here is the work required in the GUI vs. gdb bloat.

>
> The overall utility of -var-update * is somewhat suspect, I think,
> since it can take so long to run; to make it practical you'd have to
> delete varobjs for things not currently on-screen.  But as long as  
> that
> functionality exists, we figured it would be nice to not break it.

And buggy :-). But I agree. We're moving away from '-var-update *' to  
checking only the variables that are of interest.

>
> The read-sensitive values that Vladimir alluded to are our ultimate
> goal here and you'll be seeing rather more of them in the next
> few weeks.  Whenever we've suggested to a customer that our Eclipse
> ought to pop up a window that shows all of their target's peripheral
> I/O registers, the response has been jumping-up-and-down enthusiastic.
> But without some solution for this problem we just can't do it.   
> Having
> that window pop open, acknowledge all your pending interrupts, and
> steal one byte from each serial port FIFO, would be a real downer.

Adding a varobj attribute to provide this information would  
definitely be a cool addition.

Regards,

Greg

Re: MI: frozen variable objects

[Vladimir Prus]

Frederic RISS wrote:

> On Thu, 2006-11-16 at 06:58 -0700, Greg Watson wrote:
>> I don't really understand the motivation for putting this kind of
>> functionality into gdb. Any GUI that is more than just a front-end
>> for gdb will most likely have an internal data structure representing
>> the value of the variable, and can retain or manage access to the
>> value if required. It seems to me that adding functionality like this
>> to gdb just adds to bloat when it is really a GUI function anyway.
> 
> The interesting part seems to be the one that hasn't been submited yet
> about GDB auto-freezing some values due to archtectural requirements.
> The debugger has architectural knowledge and it shouldn't be necessary
> to duplicate it in the GUI.
> I agree with your general point though. Maybe the functionality
> shouldn't be in the debugger, but the information should be available to
> the GUI... through varobj properties maybe? Of course this means that
> the GUIs have to known about and respect this information.

Since GUI typically rely to -var-update to read and report which values have
changed, it's best to change -var-update not to implicitly read
read-sensitive values. Doing this in GUI requires that GUI never
issues -var-update for a variable object that contains read-sensitive
children. That would be quite a drastic change in GUI architecture.

With frozen variable object as implemented in the patch, GUI need only
localized straight-forward changes.

- Volodya

Re: MI: frozen variable objects

[Frédéric Riss]

Le jeudi 16 novembre 2006 à 21:50 +0300, Vladimir Prus a écrit :
> Frederic RISS wrote:
> > The interesting part seems to be the one that hasn't been submited yet
> > about GDB auto-freezing some values due to archtectural requirements.
> > The debugger has architectural knowledge and it shouldn't be necessary
> > to duplicate it in the GUI.
> [...]
> Since GUI typically rely to -var-update to read and report which values have
> changed, it's best to change -var-update not to implicitly read
> read-sensitive values. Doing this in GUI requires that GUI never
> issues -var-update for a variable object that contains read-sensitive
> children. That would be quite a drastic change in GUI architecture.
> 
> With frozen variable object as implemented in the patch, GUI need only
> localized straight-forward changes.

First, note that I don't argue against adding support to the varobjs for
``frozen values'', even if that's what my mail sounded like :-). 

I gathered from your first mail that the ultimate goal is to add support
in GDB for 'read-sensitive' data. That support shouldn't be limited to
MI varobjs as Daniel pointed out. To get this support for the CLI,
you'll certainly need low-level changes in GDB's value handling.

If the above is right, then the low-level changes will prevent the
varobj layer from fetching the data, and you kinda get the result you
wanted. You'd just need to make sure that the data gets fetched when
requested explicitly and to report the not-fetched-because-sensitive
varobj state. 

Of course that would only work for read-sensitive data and wouldn't
allow the GUI to freeze others values. If you believe this additional
functionality would be useful to frontends, I think your opinion as a
KDevelop developer is worth listening to :-)

Fred.

Re: MI: frozen variable objects

[Vladimir Prus]

Frédéric Riss wrote:

> Le jeudi 16 novembre 2006 à 21:50 +0300, Vladimir Prus a écrit :
>> Frederic RISS wrote:
>> > The interesting part seems to be the one that hasn't been submited yet
>> > about GDB auto-freezing some values due to archtectural requirements.
>> > The debugger has architectural knowledge and it shouldn't be necessary
>> > to duplicate it in the GUI.
>> [...]
>> Since GUI typically rely to -var-update to read and report which values
>> have changed, it's best to change -var-update not to implicitly read
>> read-sensitive values. Doing this in GUI requires that GUI never
>> issues -var-update for a variable object that contains read-sensitive
>> children. That would be quite a drastic change in GUI architecture.
>> 
>> With frozen variable object as implemented in the patch, GUI need only
>> localized straight-forward changes.
> 
> First, note that I don't argue against adding support to the varobjs for
> ``frozen values'', even if that's what my mail sounded like :-).
> 
> I gathered from your first mail that the ultimate goal is to add support
> in GDB for 'read-sensitive' data. That support shouldn't be limited to
> MI varobjs as Daniel pointed out. To get this support for the CLI,
> you'll certainly need low-level changes in GDB's value handling.
> 
> If the above is right, then the low-level changes will prevent the
> varobj layer from fetching the data, and you kinda get the result you
> wanted. You'd just need to make sure that the data gets fetched when
> requested explicitly and to report the not-fetched-because-sensitive
> varobj state.

That not exactly the design we planned. The value/type level will need two
changes:

   - Some flag on type or value to indicate it's read sensitive
   - Some mechanism to allow presenting various target register, 
     scattered over address space, as a single structure

At the same time, value level is not the right level to handle presentation
of read-sensitive values or make fetch/not fetch decisions. There's just
value_fetch_lazy function -- that fetches the raw data from the target. We
can either:

   (1) make value_fetch_lazy do nothing for read-sensitive values,
   and add new value_fetch_really function that will read the value anyway,
   or
   (2) teach the clients of value_fetch_lazy to call it only when needed

Those alternatives are roughly at the same scale of complexity. Since the
printing code should look at the value to decide if it should print
"<read sensitive>" instead not-yet fetched value, it might as well avoid
calling value_fetch_lazy. And doing (1) would mean that value_fetch_lazy
no longer guarantees that the data is fully fetched, which will be
confusing.

- Volodya

Re: MI: frozen variable objects

[Frederic RISS]

On Fri, 2006-11-17 at 09:16 +0300, Vladimir Prus wrote:
> > If the above is right, then the low-level changes will prevent the
> > varobj layer from fetching the data, and you kinda get the result you
> > wanted. You'd just need to make sure that the data gets fetched when
> > requested explicitly and to report the not-fetched-because-sensitive
> > varobj state.
> 
> That not exactly the design we planned. The value/type level will need two
> changes:
> 
>    - Some flag on type or value to indicate it's read sensitive
>    - Some mechanism to allow presenting various target register, 
>      scattered over address space, as a single structure
> 
> At the same time, value level is not the right level to handle presentation
> of read-sensitive values or make fetch/not fetch decisions. There's just
> value_fetch_lazy function -- that fetches the raw data from the target. We
> can either:
> 
>    (1) make value_fetch_lazy do nothing for read-sensitive values,
>    and add new value_fetch_really function that will read the value anyway,
>    or
>    (2) teach the clients of value_fetch_lazy to call it only when needed

Well that's roughly what I meant. Once you get this groundwork in GDB,
all you'll have to do to get the behavior you described is to propagate
the explicitly-requested flag down to the varobj call to
value_fetch_lazy. Maybe your additional changes are necessary to handle
it gracefully. You've obviously studied this code more deeply and
recently than I did.

Fred.

Re: MI: frozen variable objects

[Vladimir Prus]

Greg Watson wrote:

> I don't really understand the motivation for putting this kind of
> functionality into gdb. Any GUI that is more than just a front-end
> for gdb will most likely have an internal data structure representing
> the value of the variable, and can retain or manage access to the
> value if required. It seems to me that adding functionality like this
> to gdb just adds to bloat when it is really a GUI function anyway.

I think it's not feasible to implement frozen variable objects in GUI
without sacrificing performance.

Suppose that you have a huge structure containing nothing less than all
memory mapped registers of a target. That's how we plan to present target
registers in UI.

There's a variable object corresponding to the entire structure, that has
children corresponding to register or register groups. The latter have more
children. On each step, -var-update is used to find all elements of this
structure that has changed. The comparison of all values is done by gdb.
That's a good side of MI -- otherwise, GUI would have to read value of every
single structure element and compare it itself. It would be slow.

With read-sensitive values, we don't want gdb to read such values on each
step. This is done by marking such values as frozen. As result, gdb will
only read those values on explicit request. The changes in gdb are:

   (1) Reporting that a variable object is frozen.
   (2) Not updating frozen variable object automatically
   (3) Ability to update non-root variable object

(1) and (2) are certainly needed -- I see no way around. As for (3),
I can imagine that GUI might notice that a child variable is frozen,
create new root variable object for that child, and update it as needed.
But still, the child corresponding to read-sensitive field should be be
updated. And I don't see why creating new root variable is better
than -var-update for a child.

Non-updating of frozen variable objects is the most complex part of this
patch, and as I say above, it's absolutely needed.

Another argument in favour of doing this in GDB is that I've prototyped GUI
side of things in KDevelop and now working on it in Eclipse, and it both
cases GUI changes are straight-forward. You just

   - Make UI show some indicator for frozen variables.
   - Add "Fetch this value" command that issues -var-update

Any other approach to handle this from GUI side would me much harder to
implement.

- Volodya

Re: MI: frozen variable objects

[Greg Watson]

On Nov 16, 2006, at 11:45 AM, Vladimir Prus wrote:

> Greg Watson wrote:
>
>> I don't really understand the motivation for putting this kind of
>> functionality into gdb. Any GUI that is more than just a front-end
>> for gdb will most likely have an internal data structure representing
>> the value of the variable, and can retain or manage access to the
>> value if required. It seems to me that adding functionality like this
>> to gdb just adds to bloat when it is really a GUI function anyway.
>
> I think it's not feasible to implement frozen variable objects in GUI
> without sacrificing performance.
>
> Suppose that you have a huge structure containing nothing less than  
> all
> memory mapped registers of a target. That's how we plan to present  
> target
> registers in UI.
>
> There's a variable object corresponding to the entire structure,  
> that has
> children corresponding to register or register groups. The latter  
> have more
> children. On each step, -var-update is used to find all elements of  
> this
> structure that has changed. The comparison of all values is done by  
> gdb.
> That's a good side of MI -- otherwise, GUI would have to read value  
> of every
> single structure element and compare it itself. It would be slow.
>
> With read-sensitive values, we don't want gdb to read such values  
> on each
> step. This is done by marking such values as frozen. As result, gdb  
> will
> only read those values on explicit request. The changes in gdb are:
>
>    (1) Reporting that a variable object is frozen.
>    (2) Not updating frozen variable object automatically
>    (3) Ability to update non-root variable object
>
> (1) and (2) are certainly needed -- I see no way around. As for (3),
> I can imagine that GUI might notice that a child variable is frozen,
> create new root variable object for that child, and update it as  
> needed.
> But still, the child corresponding to read-sensitive field should  
> be be
> updated. And I don't see why creating new root variable is better
> than -var-update for a child.
>
> Non-updating of frozen variable objects is the most complex part of  
> this
> patch, and as I say above, it's absolutely needed.
>
> Another argument in favour of doing this in GDB is that I've  
> prototyped GUI
> side of things in KDevelop and now working on it in Eclipse, and it  
> both
> cases GUI changes are straight-forward. You just
>
>    - Make UI show some indicator for frozen variables.
>    - Add "Fetch this value" command that issues -var-update
>
> Any other approach to handle this from GUI side would me much  
> harder to
> implement.
>


I agree that gdb should be where the actual check for value change is  
done. Maybe I'm missing something here, but I still don't understand  
the reason for requiring frozen values to be implemented in gdb. Is  
it just to allow your GUI to issue a single '-var-update *' each time  
the debugger suspends? In other words, you're implementing additional  
functionality in gdb to support this operation for the GUI. In our  
GUI (Eclipse-based, but not CDT), we have a class representing each  
variable, and a variable manager that is responsible for deciding  
which variables to check for updates. If a particular variable is not  
visible in the UI, or does not have some other condition on it, then  
we simple do not issue a -var-update command for that variable at  
all. It should be trivial to provide a 'read-sensitive' flag in the  
variable attributes that is read by the GUI when the variable is  
created, and it would never issue a -var-update for that variable.  
Incidentally, we moved away from the '-var-update *' approach because  
it causes gdb 6.5 to crash in certain situations under Linux.

Regards,

Greg

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 08:09:12AM -0700, Greg Watson wrote:
> I agree that gdb should be where the actual check for value change is  
> done. Maybe I'm missing something here, but I still don't understand  
> the reason for requiring frozen values to be implemented in gdb. Is  
> it just to allow your GUI to issue a single '-var-update *' each time  
> the debugger suspends? In other words, you're implementing additional  
> functionality in gdb to support this operation for the GUI.

That's one reason.  The other is that -var-list-children --all-values
shouldn't read it either.  Yes, a GUI could avoid that operation too;
but offering them when they're dangerous to use seems very unwise.

Isn't all of varobj an additional functionality to support GUI
operations?

> In our  
> GUI (Eclipse-based, but not CDT), we have a class representing each  
> variable, and a variable manager that is responsible for deciding  
> which variables to check for updates. If a particular variable is not  
> visible in the UI, or does not have some other condition on it, then  
> we simple do not issue a -var-update command for that variable at  
> all. It should be trivial to provide a 'read-sensitive' flag in the  
> variable attributes that is read by the GUI when the variable is  
> created, and it would never issue a -var-update for that variable.  

True.  If your GUI doesn't use any of the problematic commands, then
there's no bug - seems like a trivial reduction to me.

> Incidentally, we moved away from the '-var-update *' approach because  
> it causes gdb 6.5 to crash in certain situations under Linux.

I hope you'll forgive me for saying that that's infuriating behavior.
The CDT developers seem to do it too.  I don't believe you've reported
this bug; therefore it will never be fixed, and folklore will
propogate that -var-update * is unusable.

Please report bugs!

I realize our bug tracker doesn't have the timeliest response.  With
only volunteers to work with, we do what we can, but I try to make sure
that the important ones get fixed.  And the very best thing you can do
with a bug report is to include details and a testcase.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Greg Watson]

On Nov 17, 2006, at 8:15 AM, Daniel Jacobowitz wrote:
>
>
>> Incidentally, we moved away from the '-var-update *' approach because
>> it causes gdb 6.5 to crash in certain situations under Linux.
>
> I hope you'll forgive me for saying that that's infuriating behavior.
> The CDT developers seem to do it too.  I don't believe you've reported
> this bug; therefore it will never be fixed, and folklore will
> propogate that -var-update * is unusable.

Bug #'s 2188 and 2190.

Greg

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 08:26:01AM -0700, Greg Watson wrote:
> 
> On Nov 17, 2006, at 8:15 AM, Daniel Jacobowitz wrote:
> >
> >
> >>Incidentally, we moved away from the '-var-update *' approach because
> >>it causes gdb 6.5 to crash in certain situations under Linux.
> >
> >I hope you'll forgive me for saying that that's infuriating behavior.
> >The CDT developers seem to do it too.  I don't believe you've reported
> >this bug; therefore it will never be fixed, and folklore will
> >propogate that -var-update * is unusable.
> 
> Bug #'s 2188 and 2190.

Ah, I figured those weren't associated.  My mistake and apologies.  A
testcase was sent to me for 2188 off-list.  I can't reproduce it,
however.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Greg Watson]

On Nov 17, 2006, at 8:33 AM, Daniel Jacobowitz wrote:

> On Fri, Nov 17, 2006 at 08:26:01AM -0700, Greg Watson wrote:
>>
>> On Nov 17, 2006, at 8:15 AM, Daniel Jacobowitz wrote:
>>>
>>>
>>>> Incidentally, we moved away from the '-var-update *' approach  
>>>> because
>>>> it causes gdb 6.5 to crash in certain situations under Linux.
>>>
>>> I hope you'll forgive me for saying that that's infuriating  
>>> behavior.
>>> The CDT developers seem to do it too.  I don't believe you've  
>>> reported
>>> this bug; therefore it will never be fixed, and folklore will
>>> propogate that -var-update * is unusable.
>>
>> Bug #'s 2188 and 2190.
>
> Ah, I figured those weren't associated.  My mistake and apologies.  A
> testcase was sent to me for 2188 off-list.  I can't reproduce it,
> however.

We'll try and get some better information on reproducing the problem.

Greg

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 08:41:05AM -0700, Greg Watson wrote:
> We'll try and get some better information on reproducing the problem.

Thanks!  I really appreciate it; see my comments in 2188.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 08:41:05AM -0700, Greg Watson wrote:
> We'll try and get some better information on reproducing the problem.

FYI: I was working on another old patch today that I've been meaning to
submit, and I suspect it plus Vladimir's laziness changes will get you
a more sensible failure mode rather than an internal error.

It's not enough on its own to fix the problem and it conflicts with
Vladimir's pending patch.  So at the moment I'm going to have to sit on
it a few more days.  When Vladimir's patch goes in I'll update it.
Let me know if you want a copy meanwhile.

Despite what I realize I just wrote in my last message, I may suggest
Vladimir's patch (the laziness changes - NOT frozen varobjs) for the
release branch if it helps to fix this problem.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Greg Watson]

On Nov 17, 2006, at 8:15 AM, Daniel Jacobowitz wrote:

> On Fri, Nov 17, 2006 at 08:09:12AM -0700, Greg Watson wrote:
>> I agree that gdb should be where the actual check for value change is
>> done. Maybe I'm missing something here, but I still don't understand
>> the reason for requiring frozen values to be implemented in gdb. Is
>> it just to allow your GUI to issue a single '-var-update *' each time
>> the debugger suspends? In other words, you're implementing additional
>> functionality in gdb to support this operation for the GUI.
>
> That's one reason.  The other is that -var-list-children --all-values
> shouldn't read it either.  Yes, a GUI could avoid that operation too;
> but offering them when they're dangerous to use seems very unwise.
>
> Isn't all of varobj an additional functionality to support GUI
> operations?

Yes, definitely.  But the operative word is 'support', not 'provide'.  
However, you are in the best place to know what your GUI needs gdb to  
do. I just wanted to better understand that, not hinder what you're  
proposing.

Cheers,

Greg

Re: MI: frozen variable objects

[Vladimir Prus]

Greg Watson wrote:

> I agree that gdb should be where the actual check for value change is
> done. Maybe I'm missing something here, but I still don't understand
> the reason for requiring frozen values to be implemented in gdb. Is
> it just to allow your GUI to issue a single '-var-update *' each time
> the debugger suspends? In other words, you're implementing additional
> functionality in gdb to support this operation for the GUI. In our
> GUI (Eclipse-based, but not CDT), we have a class representing each
> variable, and a variable manager that is responsible for deciding
> which variables to check for updates. If a particular variable is not
> visible in the UI, or does not have some other condition on it, then
> we simple do not issue a -var-update command for that variable at
> all. It should be trivial to provide a 'read-sensitive' flag in the
> variable attributes that is read by the GUI when the variable is
> created, and it would never issue a -var-update for that variable.

Suppose you have a variable "all_registers" that is actually a big structure
and a few fields on the bottom are read-sensitive.

Current gdb only allows you to issue -var-update on top-level variable
object, so you either issue it, or don't. If you issue it, gdb will happily
fetch new values for the entire structure, including read-sensitive field.
If you don't issue it, you won't get new values for ordinary fields.

Current gdb is not more fine-grained that this, so presenting all registers
as a structure is just not possible.

> Incidentally, we moved away from the '-var-update *' approach because
> it causes gdb 6.5 to crash in certain situations under Linux.

Can you tell exactly what were those situations?

- Volodya

Re: MI: frozen variable objects

[Nick Roberts]

> This patch introduces so called "frozen" variable objects -- variable objects
> that are not implicitly updated by the "-var-update *" command or by
> "-var-update" on parent variable objects.

This explains your previous patch (Variable objects laziness):

+     We do this for frozen varobjs as well, since this
+     function is only called when it's decided that we need
+     to fetch the new value of a frozen variable.  */

I've not experienced a need for such functionality probably because my Emacs
mode is still unreleased and therefore has a very small user base.  Since you
and Daniel J see that it is needed (through Eclipse?) it seems sensible to
install a patch like this (and the earlier one, which I do understand now -
perhaps the ChangeLog could say "Use install_new_value instead of
gdb_value_fetch_lazy").  However, I would just suggest that these changes are
made shortly after the release of GDB 6.6.  That way there is time test them
with frontends that might be using GDB/MI while GDB is still in CVS.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 10:48:59AM +1300, Nick Roberts wrote:
> gdb_value_fetch_lazy").  However, I would just suggest that these changes are
> made shortly after the release of GDB 6.6.  That way there is time test them
> with frontends that might be using GDB/MI while GDB is still in CVS.

Do you think this would do any good?  As far as I know, you and Vlad
are the only people who ever test GDB/MI from CVS with frontends.

I'm not particularly inclined to wait, since none of this will show up
unless your target / IDE specifically try to use it, but I'm sure I
could be persuaded.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Nick Roberts]

> > gdb_value_fetch_lazy").  However, I would just suggest that these changes
> > are made shortly after the release of GDB 6.6.  That way there is time test
> > them with frontends that might be using GDB/MI while GDB is still in CVS.

> Do you think this would do any good? 

AFAICS the first change (Variable objects laziness) is optimisation of existing
behaviour.  It looks sound to me overall but, with the many conditionals, could
easily introduce new bugs.  I see no urgency and as I don't have a deep
understanding of all the details the only way I can really check it is to
use it within Emacs.  Maybe the second change is safer as it is a new feature,
although it could still clearly impact on existing behaviour.

>                                          As far as I know, you and Vlad
> are the only people who ever test GDB/MI from CVS with frontends.

Alan Magloire has also said to me that he will check changes to MI with
Eclipse but I don't know how that relates to your interest in Eclipse.

> I'm not particularly inclined to wait, since none of this will show up
> unless your target / IDE specifically try to use it, but I'm sure I
> could be persuaded.

If existing behaviour is broken it _should_ show up.  A six month release cycle
doesn't seem long to wait especially coming from Emacs whose last mainline
release was Oct 2001 (GDB 5.3 was current when I started!).  Maybe there is
another agenda with Emacs, I don't know, but presumably it will be released one
day and I would like it to work with current GDB when it is.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 12:03:30PM +1300, Nick Roberts wrote:
> > I'm not particularly inclined to wait, since none of this will show up
> > unless your target / IDE specifically try to use it, but I'm sure I
> > could be persuaded.
> 
> If existing behaviour is broken it _should_ show up.  A six month release cycle
> doesn't seem long to wait especially coming from Emacs whose last mainline
> release was Oct 2001 (GDB 5.3 was current when I started!).  Maybe there is
> another agenda with Emacs, I don't know, but presumably it will be released one
> day and I would like it to work with current GDB when it is.

I think Vladimir's said everything I would have said in response to
this.  Is your concern breaking your MI frontend in Emacs?  If so,
then you need to test - either routinely on HEAD, or if you have
more limited time, then on release branches.  That's why we keep
release branches around for a few weeks and announce prereleases.

Whether the patch lands on GDB 6.6 or 6.7 doesn't make much difference
to the risk.  If there's no testing, it might be released broken; if
there is testing, it won't be.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Nick Roberts]

 > I think Vladimir's said everything I would have said in response to
 > this.

He said (I had to read the archives as I'm not subscribed to gdb-patches):

                                     ...it seems to me that frontend authors
   are much better testing branch before it's realeased than random CVS state,

If the CVS state is indeed random then that must reflect poorly on the review
process.

 > Is your concern breaking your MI frontend in Emacs?  If so,
 > then you need to test - either routinely on HEAD, or if you have
 > more limited time, then on release branches.  That's why we keep
 > release branches around for a few weeks and announce prereleases.

If the changes go in after the release I generally have six months to spot a
bug, if they go in now I'll have roughly two weeks.  But I find something else
anomalous about this.  Vladimir (on behalf of Codesourcery?)  submits a patch
for MI which has 26 hunks which you're proposing to approve in three days,
just as a release is coming up.  I have submitted many patches over the last
year which still not been reviewed/approved.  Notably one patch for
-var-set-format which is just a one line change:

   http://sourceware.org/ml/gdb-patches/2006-05/msg00348.html

resubmitted in August:

   http://sourceware.org/ml/gdb/2006-08/msg00179.html

and one to fix an _existing_ bug (PR mi/2077):

   http://sourceware.org/ml/gdb-patches/2006-10/msg00239.html

This does not seem impartial use of resources to review MI changes.

To balance that I must add that GDB development benefits immensely from
Codesourcery and I appreciate that you do a huge amount of work for no
personal gain.

 > Whether the patch lands on GDB 6.6 or 6.7 doesn't make much difference
 > to the risk.  If there's no testing, it might be released broken; if
 > there is testing, it won't be.

I have no formal testing process.  I just use it and see if it breaks.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Sat, Nov 18, 2006 at 09:48:24AM +1300, Nick Roberts wrote:
>  > Is your concern breaking your MI frontend in Emacs?  If so,
>  > then you need to test - either routinely on HEAD, or if you have
>  > more limited time, then on release branches.  That's why we keep
>  > release branches around for a few weeks and announce prereleases.
> 
> If the changes go in after the release I generally have six months to spot a
> bug, if they go in now I'll have roughly two weeks.

I don't get the fuss.  It's not an immensely destabilizing change or a
huge new subsystem.  Why should it be treated separately from any other
patch posted in the last few months, in the later half of a release
gap?

GCC needs to enforce a three-stage system, but we don't.  We keep GDB
working from trunk pretty much all of the time.  I think we do, in that
regard, a great job.

> But I find something else anomalous about this.  Vladimir (on behalf
> of Codesourcery?) submits a patch for MI which has 26 hunks which
> you're proposing to approve in three days, just as a release is
> coming up.

Let me be perfectly clear about this.  I can spend a certain amount of
my work time reviewing community patches, because my employer is very
understanding about the FSF development process.  I'm lucky in that
respect and hopefully so is GDB.

I can spend a great deal more of my work time reviewing patches that
are directly to my employer's benefit and I do precisely that. 
Similarly I can spend much more time writing patches that are useful
to my employer (e.g. flash support) than I can on things I just think
would be good (e.g. several thousand lines of pointer to member
improvements that I still haven't gotten committed).  Don't
misunderstand me, I think the things I'm doing at work for GDB
are cool and good to have even in the FSF tree - otherwise we'd just
keep an internal fork.  But they tend to be of more use to embedded
developers than non-embedded because that's where we presently
have more customers.

I still spend both work and personal time reviewing GDB patches.  I
spend far more time than I want to doing this.  I'd rather be writing
my own patches.  Even so, the load of unreviewed patches far exceeds
what I can do on my own.  I have no chance whatsoever of keeping up.  I
have all your unreviewed patches flagged in my inbox, and I'll probably
get to them someday, but there are no extra hours in my day.

I am rapidly approaching burnout on GDB patch review.  I may stop doing
it entirely just to keep my sanity and have a little bit of my free
time back.

I appreciate that you fix things, especially those MI PRs.  I will
somehow get to them.  But, good lord, I need more help from other
maintainers!

And more maintainers.  All: Should Nick be an MI maintainer now?

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Nick Roberts]

 > > If the changes go in after the release I generally have six months to spot
 > > a bug, if they go in now I'll have roughly two weeks.
 > 
 > I don't get the fuss.  It's not an immensely destabilizing change or a
 > huge new subsystem.  Why should it be treated separately from any other
 > patch posted in the last few months, in the later half of a release
 > gap?

Since you are confident it DTRT, I guess there is no problem.  More importantly
you have the authority, so just do it.  Richard Stallman has no such qualms
(he calls it "exercising leadership").

Now Vladimir fetches values in one place, I have a small patch on top of that
which updates references properly, I think.

 >...
 > I appreciate that you fix things, especially those MI PRs.  I will
 > somehow get to them.  But, good lord, I need more help from other
 > maintainers!
 > 
 > And more maintainers.  All: Should Nick be an MI maintainer now?

I would just commit small patches (like -var-set-format) and try to provide
support for the larger ones (like Variable objects laziness).

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI: frozen variable objects

[Eli Zaretskii]

> Date: Fri, 17 Nov 2006 16:05:01 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Vladimir Prus <vladimir@codesourcery.com>, 	gdb-patches@sources.redhat.com
> > 
> > If the changes go in after the release I generally have six months to spot a
> > bug, if they go in now I'll have roughly two weeks.
> 
> I don't get the fuss.  It's not an immensely destabilizing change or a
> huge new subsystem.  Why should it be treated separately from any other
> patch posted in the last few months, in the later half of a release
> gap?

Two weeks does seem a tad too short, much shorter than ``the last few
months''.  While you are right that the change is not immensely
destabilizing, I'm sure you can understand Nick's concern for the
quality of the released GDB, even if only in relatively minor
features.

> Let me be perfectly clear about this.  I can spend a certain amount of
> my work time reviewing community patches, because my employer is very
> understanding about the FSF development process.  I'm lucky in that
> respect and hopefully so is GDB.

Yes, we are extremely lucky that we have you, Daniel, and that you can
do such a wonderful job for so many hours a day.  I'm sure Nick didn't
mean any disrespect.

> And more maintainers.

Absolutely.  Any suggestions welcome.

> All: Should Nick be an MI maintainer now?

I vote in favor.

Re: MI: frozen variable objects

[Vladimir Prus]

Nick Roberts wrote:

> 
>> This patch introduces so called "frozen" variable objects -- variable
>> objects that are not implicitly updated by the "-var-update *" command or
>> by "-var-update" on parent variable objects.
> 
> This explains your previous patch (Variable objects laziness):
> 
> +     We do this for frozen varobjs as well, since this
> +     function is only called when it's decided that we need
> +     to fetch the new value of a frozen variable.  */
> 
> I've not experienced a need for such functionality probably because my
> Emacs
> mode is still unreleased and therefore has a very small user base.  Since
> you and Daniel J see that it is needed (through Eclipse?) it seems
> sensible to install a patch like this (and the earlier one, which I do
> understand now - perhaps the ChangeLog could say "Use install_new_value
> instead of
> gdb_value_fetch_lazy").  However, I would just suggest that these changes
> are
> made shortly after the release of GDB 6.6.  That way there is time test
> them with frontends that might be using GDB/MI while GDB is still in CVS.

I don't have a strong opinion here, but it seems to me that frontend authors
are much better testing branch before it's realeased than random CVS state,
since frontends will typically work with released GDB.

And then, it's not much different if this change will be tested when it's on
6.6 branch than when it's on 6.7 branch.

- Volodya

Re: MI: frozen variable objects

[Daniel Jacobowitz]

On Fri, Nov 17, 2006 at 10:48:59AM +1300, Nick Roberts wrote:
> gdb_value_fetch_lazy").  However, I would just suggest that these changes are
> made shortly after the release of GDB 6.6.  That way there is time test them
> with frontends that might be using GDB/MI while GDB is still in CVS.

While I disagree with this as a general position (see my later
messages), it's now irrelevant: trunk is past the branchpoint :-)
I don't think this would be appropriate to add to a release branch.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI: frozen variable objects

[Nick Roberts]

 > > >  Incidentally, we moved away from the '-var-update *' approach because
 > > >  it causes gdb 6.5 to crash in certain situations under Linux.


 > >  I hope you'll forgive me for saying that that's infuriating behavior.
 > >  The CDT developers seem to do it too.  I don't believe you've reported
 > >  this bug; therefore it will never be fixed, and folklore will
 > >  propogate that -var-update * is unusable.


 > Bug #'s 2188 and 2190.

I hadn't seen these either.  For future reference MI has it's own category in
the database.

-- 
Nick                                           http://www.inet.net.nz/~nickrob