This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] varobj deletion after the binary has changed


Nick Roberts wrote:
Looks good. Maybe varobj_update could use an enum:

enum varobj_update_values
{
SCOPE_FALSE = -1,
TYPE_CHANGED,
SCOPE_INVALID
}
Yes, it's better and more readable.
As you'll see in my patch, the varobj_update function could return more than these case of error, it's the caller that decides how to deal with these errors.

(It looks like we could remove the return value of varobj_update_one as it
doesn't seem to be used.)
You're right, I removed it as well.

Attached is the new implementation plus the new exp file.

Denis
2007-01-31  Denis Pilat  <denis.pilat@st.com>

	* varobj.c (struct varobj_root): Add is_valid member.
	(varobj_get_type): Check for invalid varobj.
	(varobj_get_attributes): Likewise.
	(variable_editable):Likewise.
	(varobj_update): Likewise plus use an enum for returned error values.
	(new_root_variable): Set root varobj as valid by default.
	(varobj_invalidate): New function.
	* varobj.h (enum varobj_update_error): New enum.
	(varobj_invalidate): New function.
	* symfile.c (clear_symtab_users): Use varobj_invalidate.
	* mi/mi-cmd-var.c (varobj_update_one): Change returned type to void and
	use of new enum varobj_update_error to deal with errors.


Index: varobj.c
===================================================================
--- varobj.c	(revision 553)
+++ varobj.c	(working copy)
@@ -71,6 +71,10 @@ struct varobj_root
      using the currently selected frame. */
   int use_selected_frame;
 
+  /* Flag that indicates validity: set to 0 when this varobj_root refers 
+     to symbols that do not exist anymore.  */
+  int is_valid;
+
   /* Language info for this variable and its children */
   struct language_specific *lang;
 
@@ -742,8 +746,9 @@ varobj_get_type (struct varobj *var)
   long length;
 
   /* For the "fake" variables, do not return a type. (It's type is
-     NULL, too.) */
-  if (CPLUS_FAKE_CHILD (var))
+     NULL, too.)
+     Do not return a type for invalid variables as well.  */
+  if (CPLUS_FAKE_CHILD (var) || !var->root->is_valid)
     return NULL;
 
   stb = mem_fileopen ();
@@ -778,7 +783,7 @@ varobj_get_attributes (struct varobj *va
 {
   int attributes = 0;
 
-  if (variable_editable (var))
+  if (var->root->is_valid && variable_editable (var))
     /* FIXME: define masks for attributes */
     attributes |= 0x00000001;	/* Editable */
 
@@ -1018,16 +1023,15 @@ install_new_value (struct varobj *var, s
    expression to see if it's changed.  Then go all the way
    through its children, reconstructing them and noting if they've
    changed.
-   Return value:
-    -1 if there was an error updating the varobj
-    -2 if the type changed
-    Otherwise it is the number of children + parent changed
+   Return value: 
+    < 0 for error values, see varobj.h.
+    Otherwise it is the number of children + parent changed.
 
    Only root variables can be updated... 
 
    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. */
+   returns TYPE_CHANGED, 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)
@@ -1048,12 +1052,15 @@ varobj_update (struct varobj **varp, str
 
   /* sanity check: have we been passed a pointer? */
   if (changelist == NULL)
-    return -1;
+    return WRONG_PARAM;
 
   /*  Only root variables can be updated... */
   if (!is_root_p (*varp))
     /* Not a root var */
-    return -1;
+    return WRONG_PARAM;
+
+  if (!(*varp)->root->is_valid)
+    return INVALID;
 
   /* Save the selected stack frame, since we will need to change it
      in order to evaluate expressions. */
@@ -1090,7 +1097,7 @@ varobj_update (struct varobj **varp, str
       /* This means the varobj itself is out of scope.
 	 Report it.  */
       VEC_free (varobj_p, result);
-      return -1;
+      return NOT_IN_SCOPE;
     }
 
   VEC_safe_push (varobj_p, stack, *varp);
@@ -1140,7 +1147,7 @@ varobj_update (struct varobj **varp, str
   *cv = 0;
 
   if (type_changed)
-    return -2;
+    return TYPE_CHANGED;
   else
     return changed;
 }
@@ -1409,6 +1416,7 @@ new_root_variable (void)
   var->root->frame = null_frame_id;
   var->root->use_selected_frame = 0;
   var->root->rootvar = NULL;
+  var->root->is_valid = 1;
 
   return var;
 }
@@ -1692,7 +1700,10 @@ variable_editable (struct varobj *var)
 static char *
 my_value_of_variable (struct varobj *var)
 {
-  return (*var->root->lang->value_of_variable) (var);
+  if (var->root->is_valid)
+    return (*var->root->lang->value_of_variable) (var);
+  else
+    return NULL;
 }
 
 static char *
@@ -2504,3 +2515,44 @@ When non-zero, varobj debugging is enabl
 			    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="false".  */
+void 
+varobj_invalidate (void)
+{
+  struct varobj **all_rootvarobj;
+  struct varobj **varp;
+
+  if (varobj_list (&all_rootvarobj) > 0)
+  {
+    varp = all_rootvarobj;
+    while (*varp != NULL)
+      {
+        /* 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;
+}
Index: varobj.h
===================================================================
--- varobj.h	(revision 553)
+++ varobj.h	(working copy)
@@ -38,7 +38,16 @@ enum varobj_type
     USE_CURRENT_FRAME,          /* Use the current frame */
     USE_SELECTED_FRAME          /* Always reevaluate in selected frame */
   };
-    
+
+/* Error return values for varobj_update function.  */
+enum varobj_update_error
+  {
+    NOT_IN_SCOPE = -1,          /* varobj not in scope, can not be updated.  */
+    TYPE_CHANGED = -2,          /* varobj type has changed.  */
+    INVALID = -3,               /* varobj is not valid anymore.  */
+    WRONG_PARAM = -4            /* function is called with wrong arguments.  */
+  };
+
 /* String representations of gdb's format codes (defined in varobj.c) */
 extern char *varobj_format_string[];
 
@@ -99,4 +108,6 @@ extern int varobj_list (struct varobj **
 
 extern int varobj_update (struct varobj **varp, struct varobj ***changelist);
 
+extern void varobj_invalidate (void);
+
 #endif /* VAROBJ_H */
Index: symfile.c
===================================================================
--- symfile.c	(revision 553)
+++ symfile.c	(working copy)
@@ -52,6 +52,7 @@
 #include "observer.h"
 #include "exec.h"
 #include "parser-defs.h"
+#include "varobj.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -2602,6 +2603,10 @@ 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
Index: mi/mi-cmd-var.c
===================================================================
--- mi/mi-cmd-var.c	(revision 553)
+++ mi/mi-cmd-var.c	(working copy)
@@ -34,9 +34,9 @@ const char mi_no_values[] = "--no-values
 const char mi_simple_values[] = "--simple-values";
 const char mi_all_values[] = "--all-values";
 
-extern int varobjdebug;		/* defined in varobj.c */
+extern int varobjdebug;		/* defined in varobj.c.  */
 
-static int varobj_update_one (struct varobj *var,
+static void varobj_update_one (struct varobj *var,
 			      enum print_values print_values);
 
 static int mi_print_value_p (struct type *type, enum print_values print_values);
@@ -535,11 +535,9 @@ mi_cmd_var_update (char *command, char *
     return MI_CMD_DONE;
 }
 
-/* Helper for mi_cmd_var_update() Returns 0 if the update for
-   the variable fails (usually because the variable is out of
-   scope), and 1 if it succeeds. */
+/* Helper for mi_cmd_var_update().  */
 
-static int
+static void
 varobj_update_one (struct varobj *var, enum print_values print_values)
 {
   struct varobj **changelist;
@@ -549,37 +547,39 @@ varobj_update_one (struct varobj *var, e
 
   nc = varobj_update (&var, &changelist);
 
-  /* nc == 0 means that nothing has changed.
-     nc == -1 means that an error occured in updating the variable.
-     nc == -2 means the variable has changed type. */
+  /* nc >= 0  represents the number of changes reported into changelist.
+     nc < 0   means that an error occured or the the variable has 
+              changed type (TYPE_CHANGED).  */
   
   if (nc == 0)
-    return 1;
-  else if (nc == -1)
+    return;
+  else if (nc < 0)
     {
       if (mi_version (uiout) > 1)
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
       ui_out_field_string (uiout, "name", varobj_get_objname(var));
-      ui_out_field_string (uiout, "in_scope", "false");
-      if (mi_version (uiout) > 1)
-        do_cleanups (cleanup);
-      return -1;
-    }
-  else if (nc == -2)
-    {
-      if (mi_version (uiout) > 1)
-        cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-      ui_out_field_string (uiout, "name", varobj_get_objname (var));
-      ui_out_field_string (uiout, "in_scope", "true");
-      ui_out_field_string (uiout, "new_type", varobj_get_type(var));
-      ui_out_field_int (uiout, "new_num_children", 
-			   varobj_get_num_children(var));
+
+      switch (nc)
+      {
+        case NOT_IN_SCOPE:
+        case WRONG_PARAM:
+          ui_out_field_string (uiout, "in_scope", "false");
+	  break;
+        case INVALID:
+          ui_out_field_string (uiout, "in_scope", "invalid");
+ 	  break;
+        case TYPE_CHANGED:
+	  ui_out_field_string (uiout, "in_scope", "true");
+          ui_out_field_string (uiout, "new_type", varobj_get_type(var));
+          ui_out_field_int (uiout, "new_num_children", 
+			    varobj_get_num_children(var));
+	  break;
+      }
       if (mi_version (uiout) > 1)
         do_cleanups (cleanup);
     }
   else
     {
-      
       cc = changelist;
       while (*cc != NULL)
 	{
@@ -595,7 +595,5 @@ varobj_update_one (struct varobj *var, e
 	  cc++;
 	}
       xfree (changelist);
-      return 1;
     }
-  return 1;
 }
# Copyright 1999, 2000, 2001, 2002, 2004, 2005 Free Software
# Foundation, Inc.
#
# This Program Is Free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

# Test essential Machine interface (MI) operations
#
# Verify that once binary file has changed, GDB correctly handles
# previously defined MI variables.
#


load_lib mi-support.exp
set MIFLAGS "-i=mi"

gdb_exit
if [mi_gdb_start] {
    continue
}

set testfile "var-cmd"
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}
if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
    untested mi-var-invalidate.exp
    return -1
}
# Just change the output binary.
set binfile_bis ${objdir}/${subdir}/${testfile}_bis
if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile_bis}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
    untested mi-var-invalidate.exp
    return -1
}

set testfile2 "basics"
set srcfile2 ${testfile2}.c
set binfile2 ${objdir}/${subdir}/${testfile2}
if  { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
    untested mi-var-invalidate.exp
    return -1
}

mi_delete_breakpoints
mi_gdb_reinitialize_dir $srcdir/$subdir
mi_gdb_load ${binfile}

# Desc:  Create global variable
mi_gdb_test "-var-create global_simple * global_simple" \
	"\\^done,name=\"global_simple\",numchild=\"6\",type=\"simpleton\"" \
	"create global variable"

mi_runto do_locals_tests

# Desc: create local variables
mi_gdb_test "-var-create linteger * linteger" \
	"\\^done,name=\"linteger\",numchild=\"0\",type=\"int\"" \
	"create local variable linteger"

#
# reload the same binary
# global variable must be keeped, other invalidated
#
mi_delete_breakpoints
mi_gdb_load ${binfile_bis}
mi_runto main

# Check local variable are "invalid"
mi_gdb_test "-var-update linteger" \
	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"invalid\"\}\\\]" \
	"linteger not anymore in scope due to binary changes"

mi_gdb_test "-var-info-type linteger" \
	"\\^done,type=\"\"" \
	"not type for invalid variable linteger"

# Check global variable are still correct.
mi_gdb_test "-var-update global_simple" \
	"\\^done,changelist=\\\[\]" \
	"global_simple still alive"

mi_gdb_test "-var-info-type global_simple" \
	"\\^done,type=\"simpleton\"" \
	"type simpleton for valid variable global_simple"


#
# load an other binary
# all variables must be invalidated
#
mi_delete_breakpoints
mi_gdb_load ${binfile2}
# Check local variable are "invalid"
mi_gdb_test "-var-update linteger" \
	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"invalid\"\}\\\]" \
	"linteger not valid anymore due to binary changes"

mi_gdb_test "-var-info-type linteger" \
	"\\^done,type=\"\"" \
	"not type for invalid variable linteger"

# Check global variable are still correct.
mi_gdb_test "-var-update global_simple" \
	"\\^done,changelist=\\\[\{name=\"global_simple\",in_scope=\"invalid\"\}\\\]" \
	"global_simple not anymore in scope due to binary changes"

mi_gdb_test "-var-info-type global_simple" \
	"\\^done,type=\"\"" \
	"not type for invalid variable global_simple"

mi_gdb_exit
return 0

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]