Re: MI testsuite failures [PATCH]

[Daniel Jacobowitz <drow@false.org>]

On Mon, Jan 08, 2007 at 06:53:11PM +1300, Nick Roberts wrote:
> Here's a patch with fixes for the testsuite.  I've made a further change to
> varobj.c to fix a failure in mi-var-child.exp.

Was this:

FAIL: gdb.mi/mi-var-child.exp: update all vars int_ptr_ptr and children changed
FAIL: gdb.mi/mi-var-child.exp: update all vars struct_declarations.long_array.0 changed

?

The patch below fixes those, and this one too:

FAIL: gdb.mi/mi-var-cmd.exp: assign same value to func (update)

I can't quite tell what your patch does, but I do see why these
failures happen.  Because the var->updated and the var->value
previously NULL cases were not updating print_value, you sometimes had
to -var-update twice to get a value to leave the list.

This is the same reason you had to add spurious -var-update's to the
testsuite.  We should only need this testsuite change:

> 	* gdb.mi/mi-var-cmd.exp: Add fields to changelists for string
> 	contents changes.

The first time we do that it's a good thing, i.e. we want lpcharacter
to be listed now.  The second time it's kind of dodgy: lpcharacter
is not a NULL terminated string and it just so happens that linteger
is right after lcharacter in memory, so -var-assign'ing to linteger
"changes" the string pointer to by lpcharacter.

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-08  Daniel Jacobowitz  <dan@codesourcery.com>

	* varobj.c (install_new_value): Always update print_value.
	(value_get_print_value): Immediately return NULL for missing
	values.

2007-01-08  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.mi/mi-var-cmd.exp: Expect lpcharacter to update when
	lcharacter or linteger change.  Correct duplicated test name.
	* gdb.mi/mi2-var-cmd.exp: Likewise.

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.76
diff -u -p -r1.76 varobj.c
--- varobj.c	5 Jan 2007 21:58:48 -0000	1.76
+++ varobj.c	8 Jan 2007 17:08:01 -0000
@@ -966,9 +966,13 @@ install_new_value (struct varobj *var, s
       /* If the value of the varobj was changed by -var-set-value, then the 
 	 value in the varobj and in the target is the same.  However, that value
 	 is different from the value that the varobj had after the previous
-	 -var-update. So need to the varobj as changed.  */	 
+	 -var-update. So need to the varobj as changed.  */
       if (var->updated)
-	changed = 1;
+	{
+	  xfree (var->print_value);
+	  var->print_value = value_get_print_value (value, var->format);
+	  changed = 1;
+	}
       else 
 	{
 	  /* Try to compare the values.  That requires that both
@@ -979,7 +983,11 @@ install_new_value (struct varobj *var, s
 	    /* Equal. */
 	    ;
 	  else if (var->value == NULL || value == NULL)
-	    changed = 1;
+	    {
+	      xfree (var->print_value);
+	      var->print_value = value_get_print_value (value, var->format);
+	      changed = 1;
+	    }
 	  else
 	    {
 	      char *print_value;
@@ -987,6 +995,7 @@ install_new_value (struct varobj *var, s
 	      gdb_assert (!value_lazy (value));
 	      print_value = value_get_print_value (value, var->format);
 
+	      gdb_assert (var->print_value != NULL && print_value != NULL);
 	      if (strcmp (var->print_value, print_value) != 0)
 		{
 		  xfree (var->print_value);
@@ -1687,12 +1696,19 @@ static char *
 value_get_print_value (struct value *value, enum varobj_display_formats format)
 {
   long dummy;
-  struct ui_file *stb = mem_fileopen ();
-  struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
+  struct ui_file *stb;
+  struct cleanup *old_chain;
   char *thevalue;
-	    
+
+  if (value == NULL)
+    return NULL;
+
+  stb = mem_fileopen ();
+  old_chain = make_cleanup_ui_file_delete (stb);
+
   common_val_print (value, stb, format_code[(int) format], 1, 0, 0);
   thevalue = ui_file_xstrdup (stb, &dummy);
+
   do_cleanups (old_chain);
   return thevalue;
 }
Index: testsuite/gdb.mi/mi-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp,v
retrieving revision 1.22
diff -u -p -r1.22 mi-var-cmd.exp
--- testsuite/gdb.mi/mi-var-cmd.exp	4 Jan 2007 21:59:10 -0000	1.22
+++ testsuite/gdb.mi/mi-var-cmd.exp	8 Jan 2007 17:08:01 -0000
@@ -261,8 +261,8 @@ mi_execute_to "exec-step 8" "end-steppin
 # Note: this test also checks that lpsimple->integer and lsimple.integer have
 #       changed (they are the same)
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
-	"update all vars: func and lpsimple changed"
+	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"update all vars: lsimple and others changed"
 
 
 ### 
@@ -278,8 +278,13 @@ mi_gdb_test "-var-assign linteger 3333" 
 	"\\^done,value=\"3333\"" \
 	"assign to linteger"
 
+# Allow lpcharacter to update, optionally.  Because it points to a
+# char variable instead of a zero-terminated string, if linteger is
+# directly after it in memory the printed characters may appear to
+# change.
+set lpchar_update "\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},"
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"\\^done,changelist=\\\[($lpchar_update)?\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
 	"update all vars: linteger changed after assign"
 
 mi_gdb_test "-var-assign linteger 3333" \
Index: testsuite/gdb.mi/mi2-var-cmd.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-var-cmd.exp,v
retrieving revision 1.6
diff -u -p -r1.6 mi2-var-cmd.exp
--- testsuite/gdb.mi/mi2-var-cmd.exp	4 Jan 2007 18:58:03 -0000	1.6
+++ testsuite/gdb.mi/mi2-var-cmd.exp	8 Jan 2007 17:08:01 -0000
@@ -261,8 +261,8 @@ mi_execute_to "exec-step 8" "end-steppin
 # Note: this test also checks that lpsimple->integer and lsimple.integer have
 #       changed (they are the same)
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
-	"update all vars: func and lpsimple changed"
+	"\\^done,changelist=\\\[\{name=\"lsimple.integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple->integer\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lsimple.character\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"ldouble\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lfloat\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"llong\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"lcharacter\",in_scope=\"true\",type_changed=\"false\"\},\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"update all vars: lsimple and others changed"
 
 
 ### 
@@ -278,8 +278,13 @@ mi_gdb_test "-var-assign linteger 3333" 
 	"\\^done,value=\"3333\"" \
 	"assign to linteger"
 
+# Allow lpcharacter to update, optionally.  Because it points to a
+# char variable instead of a zero-terminated string, if linteger is
+# directly after it in memory the printed characters may appear to
+# change.
+set lpchar_update "\{name=\"lpcharacter\",in_scope=\"true\",type_changed=\"false\"\},"
 mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"\\^done,changelist=\\\[($lpchar_update)?\{name=\"linteger\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
 	"update all vars: linteger changed after assign"
 
 mi_gdb_test "-var-assign linteger 3333" \