[patch 8/8] Types GC [varobj]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

rewrite varobj memory management to fix its stale references, it depends on
many of the previous patches and could not easily separate it.

obsoletes:
	Re: [patch] [4/5] Types reference counting [varobj-validation]
	http://sourceware.org/ml/gdb-patches/2009-04/msg00610.html

	[patch] Fix gdb.mi hang on floating VAROBJs
	http://sourceware.org/ml/gdb-patches/2009-05/msg00433.html


Thanks,
Jan

gdb/
2009-05-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile.c: Remove including varobj.h.
	(clear_symtab_users): Remove the varobj_invalidate call.
	* varobj.c: Include observer.h and parser-defs.h.
	(rootcount, varobj_list, varobj_invalidate): Remove.
	(install_variable, uninstall_variable): Remove rootcount handling.
	(all_varobjs, varobj_types_mark_used_iter, varobj_types_mark_used)
	(varobj_invalidate_iter, varobj_invalidate, varobj_revalidate_iter)
	(varobj_revalidate, varobj_revalidate_for_objfile): New.
	(_initialize_varobj): Register varobj_invalidate, varobj_revalidate,
	varobj_revalidate_for_objfile and varobj_types_mark_used observers.
	* varobj.h (varobj_invalidate): Remove prototype.

gdb/testsuite/
2009-05-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.mi/mi2-var-cmd.exp (floating varobj invalidation): New test.

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 06e8c6e..fe0552f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -51,7 +51,6 @@
 #include "observer.h"
 #include "exec.h"
 #include "parser-defs.h"
-#include "varobj.h"
 #include "elf-bfd.h"
 #include "solib.h"
 #include "remote.h"
@@ -2813,10 +2812,6 @@ clear_symtab_users (void)
      between expressions and which ought to be reset each time.  */
   expression_context_block = NULL;
   innermost_block = NULL;
-
-  /* Varobj may refer to old symbols, perform a cleanup.  */
-  varobj_invalidate ();
-
 }
 
 static void
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 718c690..c3b3564 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -34,6 +34,8 @@
 #include "vec.h"
 #include "gdbthread.h"
 #include "inferior.h"
+#include "observer.h"
+#include "parser-defs.h"
 
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
@@ -397,7 +399,6 @@ static int format_code[] = { 0, 't', 'd', 'x', 'o' };
 
 /* Header of the list of root variable objects */
 static struct varobj_root *rootlist;
-static int rootcount = 0;	/* number of root varobjs in the list */
 
 /* Prime number indicating the number of buckets in the hash table */
 /* A prime large enough to avoid too many colisions */
@@ -945,37 +946,6 @@ varobj_set_value (struct varobj *var, char *expression)
   return 1;
 }
 
-/* Returns a malloc'ed list with all root variable objects */
-static int
-varobj_list (struct varobj ***varlist)
-{
-  struct varobj **cv;
-  struct varobj_root *croot;
-  int mycount = rootcount;
-
-  /* Alloc (rootcount + 1) entries for the result */
-  *varlist = xmalloc ((rootcount + 1) * sizeof (struct varobj *));
-
-  cv = *varlist;
-  croot = rootlist;
-  while ((croot != NULL) && (mycount > 0))
-    {
-      *cv = croot->rootvar;
-      mycount--;
-      cv++;
-      croot = croot->next;
-    }
-  /* Mark the end of the list */
-  *cv = NULL;
-
-  if (mycount || (croot != NULL))
-    warning
-      ("varobj_list: assertion failed - wrong tally of root vars (%d:%d)",
-       rootcount, mycount);
-
-  return rootcount;
-}
-
 /* Assign a new value to a variable object.  If INITIAL is non-zero,
    this is the first assignement after the variable object was just
    created, or changed type.  In that case, just assign the value 
@@ -1353,7 +1323,6 @@ install_variable (struct varobj *var)
       else
 	var->root->next = rootlist;
       rootlist = var->root;
-      rootcount++;
     }
 
   return 1;			/* OK */
@@ -1430,7 +1399,6 @@ uninstall_variable (struct varobj *var)
 	  else
 	    prer->next = cr->next;
 	}
-      rootcount--;
     }
 
 }
@@ -2752,6 +2720,166 @@ all_root_varobjs (void (*func) (struct varobj *var, void *data), void *data)
       (*func) (var_root->rootvar, data);
     }
 }
+
+/* Iterate all the existing VAROBJs and call the FUNC callback for them with an
+   arbitrary caller supplied DATA pointer.  */
+
+static void
+all_varobjs (void (*func) (struct varobj *var, void *data), void *data)
+{
+  struct vlist **vlp, *vl;
+
+  for (vlp = varobj_table; vlp < varobj_table + VAROBJ_TABLE_SIZE; vlp++)
+    for (vl = *vlp; vl != NULL; vl = vl->next)
+      (*func) (vl->var, data);
+}
+
+/* Helper for varobj_types_mark_used.  Call type_mark_used for any TYPEs
+   referenced from this VAR.  */
+
+static void
+varobj_types_mark_used_iter (struct varobj *var, void *unused)
+{
+  /* Even FLOATING or IS_INVALID VARs with non-NULL TYPE references will
+     free them in free_variable.  Still EXP may also reference TYPEs
+     but these belong to SYMBOLs which should be always associated with
+     an OBJFILE (and therefore not useful to be type_mark_used).  */
+
+  type_mark_used (var->type);
+  if (var->value)
+    type_mark_used (value_type (var->value));
+
+  /* Check VAROBJROOTs only once during the varobj_types_mark_used pass.  */
+
+  if (var->root->rootvar == var)
+    {
+      if (var->root->exp)
+	exp_types_mark_used (var->root->exp);
+    }
+}
+
+/* Call type_mark_used for any TYPEs referenced from this GDB source file.  */
+
+static void
+varobj_types_mark_used (void)
+{
+  /* Check all the VAROBJs, even non-root ones.  Child VAROBJs can reference
+     types from other OBJFILEs through TYPE_IS_OPAQUE resolutions by
+     check_typedef.  Such types references will not be interconnected into the
+     same TYPE_GROUP.  */
+
+  all_varobjs (varobj_types_mark_used_iter, NULL);
+}
+
+/* Invalidate VAR if it is tied to the specified OBJFILE.  Call this function
+   before you start removing OBJFILE.
+
+   Call varobj_revalidate_iter after the OBJFILE update get finished.
+
+   Invalidated varobjs will be always printed in_scope="invalid".  */
+
+static void
+varobj_invalidate_iter (struct varobj *var, void *objfile_voidp)
+{
+  struct objfile *objfile = objfile_voidp;
+
+  /* Check VAROBJROOTs only once during the varobj_invalidate pass.  */
+
+  if (var->root->rootvar == var)
+    {
+      /* Check even FLOATING VAROBJROOTs as their data will be still checked
+	 during varobj_update by varobj_get_type.  */
+
+      if (var->root->is_valid
+	  && block_objfile (var->root->valid_block) == objfile)
+	var->root->is_valid = 0;
+
+      if (var->root->exp && exp_uses_objfile (var->root->exp, objfile))
+	{
+	  var->root->is_valid = 0;
+
+	  /* No one touches EXP for !IS_VALID varobj.  */
+	  xfree (var->root->exp);
+	  var->root->exp = NULL;
+	}
+    }
+
+  if (var->type && TYPE_OBJFILE (var->type) == objfile)
+    {
+      var->root->is_valid = 0;
+
+      var->type = NULL;
+    }
+
+  if (var->value && TYPE_OBJFILE (value_type (var->value)) == objfile)
+    {
+      var->root->is_valid = 0;
+
+      value_free (var->value);
+      var->value = NULL;
+    }
+}
+
+/* Call varobj_invalidate_iter for all the VAROBJs.  */
+
+static void 
+varobj_invalidate (struct objfile *objfile)
+{
+  /* Check all the VAROBJs, even non-root ones.  Child VAROBJs can reference
+     types from other OBJFILEs through TYPE_IS_OPAQUE resolutions by
+     check_typedef.  */
+
+  all_varobjs (varobj_invalidate_iter, objfile);
+}
+
+/* Recreate any global varobjs possibly previously invalidated.  If the
+   expressions are no longer evaluatable set/keep the VAR invalid.  */
+
+static void
+varobj_revalidate_iter (struct varobj *var, void *unused)
+{
+  /* Global VAR must be re-evaluated.  */
+
+  if (var->root->valid_block == NULL)
+    {
+      struct varobj *tmp_var;
+
+      /* Try to create a varobj with same expression.  If we succeed
+	 replace the old varobj, otherwise invalidate it.  */
+      tmp_var = varobj_create (NULL, var->name, 0, USE_CURRENT_FRAME);
+      if (tmp_var != NULL)
+	{
+	  tmp_var->obj_name = xstrdup (var->obj_name);
+	  varobj_delete (var, NULL, 0);
+	  install_variable (tmp_var);
+	}
+      else
+	var->root->is_valid = 0;
+    }
+}
+
+/* Call varobj_revalidate_iter for all the root VAROBJs.  */
+
+static void 
+varobj_revalidate (void)
+{
+  /* Check only root VAROBJs.  Any successful revalidation will replace the
+     whole VAROBJs tree starting with root VAROBJs and its children get created
+     later on-demand.  So there is no point trying to revalidate the child
+     VAROBJs.  */
+
+  all_root_varobjs (varobj_revalidate_iter, NULL);
+}
+
+/* Call varobj_revalidate just providing a different function prototype.
+   Currently existing VAROBJs may become valid or change with new symbols
+   loaded.  */
+
+static void
+varobj_revalidate_for_objfile (struct objfile *objfile)
+{
+  varobj_revalidate ();
+}
 
 extern void _initialize_varobj (void);
 void
@@ -2770,53 +2898,9 @@ When non-zero, varobj debugging is enabled."),
 			    NULL,
 			    show_varobjdebug,
 			    &setlist, &showlist);
-}
-
-/* Invalidate the varobjs that are tied to locals and re-create the ones that
-   are defined on globals.
-   Invalidated varobjs will be always printed in_scope="invalid".  */
 
-void 
-varobj_invalidate (void)
-{
-  struct varobj **all_rootvarobj;
-  struct varobj **varp;
-
-  if (varobj_list (&all_rootvarobj) > 0)
-    {
-      varp = all_rootvarobj;
-      while (*varp != NULL)
-	{
-	  /* Floating varobjs are reparsed on each stop, so we don't care if
-	     the presently parsed expression refers to something that's gone.
-	     */
-	  if ((*varp)->root->floating)
-	    continue;
-
-	  /* global var must be re-evaluated.  */     
-	  if ((*varp)->root->valid_block == NULL)
-	    {
-	      struct varobj *tmp_var;
-
-	      /* Try to create a varobj with same expression.  If we succeed
-		 replace the old varobj, otherwise invalidate it.  */
-	      tmp_var = varobj_create (NULL, (*varp)->name, (CORE_ADDR) 0,
-				       USE_CURRENT_FRAME);
-	      if (tmp_var != NULL) 
-		{ 
-		  tmp_var->obj_name = xstrdup ((*varp)->obj_name);
-		  varobj_delete (*varp, NULL, 0);
-		  install_variable (tmp_var);
-		}
-	      else
-		(*varp)->root->is_valid = 0;
-	    }
-	  else /* locals must be invalidated.  */
-	    (*varp)->root->is_valid = 0;
-
-	  varp++;
-	}
-    }
-  xfree (all_rootvarobj);
-  return;
+  observer_attach_objfile_unloading (varobj_invalidate);
+  observer_attach_objfile_unloaded (varobj_revalidate);
+  observer_attach_new_objfile (varobj_revalidate_for_objfile);
+  observer_attach_mark_used (varobj_types_mark_used);
 }
diff --git a/gdb/varobj.h b/gdb/varobj.h
index c1471d4..08843d9 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -136,8 +136,6 @@ extern void all_root_varobjs (void (*func) (struct varobj *var, void *data),
 extern VEC(varobj_update_result) *varobj_update (struct varobj **varp, 
 						 int explicit);
 
-extern void varobj_invalidate (void);
-
 extern int varobj_editable_p (struct varobj *var);
 
 extern int varobj_floating_p (struct varobj *var);
diff --git a/gdb/testsuite/gdb.mi/mi2-var-cmd.exp b/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
index a4279a8..a1689a0 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-cmd.exp
@@ -523,5 +523,9 @@ mi_gdb_test "-var-update selected_a" \
 	"\\^done,changelist=\\\[\{name=\"selected_a\",in_scope=\"true\",type_changed=\"true\",new_type=\"int\",new_num_children=\"0\"\}\\\]" \
 	"update selected_a in do_special_tests"
 
+mi_gdb_test "-file-exec-and-symbols ${binfile}" "\\^done" \
+	    "floating varobj invalidation"
+
+
 mi_gdb_exit
 return 0