[mi] Some error path fixes

[Daniel Jacobowitz <drow at false dot org>]

Two patches I've had locally that fix up problems reading variables.
I couldn't find a way to write a test for the eval.c part; I wrote this
a long time ago, when we called error for variables that were currently
unavailable, at which time it was much easier to trigger the crash.  The
varobj.c part is covered by the new tests.  gdb_evaluate_expression
does not set *value = NULL on error.

(runtest --tool_exec "valgrind ../gdb" works reasonably well, though you
have to inspect the logs by hand.)

Does this look wrong to anyone?

I think there's still something suspicious about the error handling.  If you
take a look at the output from the "update endvar" test, you'll never get an
in_scope="false", which I would have expected for a variable that couldn't
be updated.  That's because all is well until install_new_value. If we go
from var->error = 0 to var->error = 1, at least, shouldn't we report that?

-- 
Daniel Jacobowitz
CodeSourcery

2006-12-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (eval.o): Update dependencies.
	* eval.c: Include "ui-out.h" and "exceptions.h".
	(evaluate_subexp_standard): Use TRY_CATCH around value_of_variable.
	Use value_zero if an error occurs when avoiding side effects.
	* varobj.c (c_value_of_root): Initialize new_val.

2006-12-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.mi/mi-var-cmd.exp: Add tests for unreadable varobjs.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.856
diff -u -p -r1.856 Makefile.in
--- Makefile.in	28 Nov 2006 22:14:31 -0000	1.856
+++ Makefile.in	7 Dec 2006 21:54:55 -0000
@@ -1975,7 +1975,7 @@ environ.o: environ.c $(defs_h) $(environ
 eval.o: eval.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
 	$(value_h) $(expression_h) $(target_h) $(frame_h) $(language_h) \
 	$(f_lang_h) $(cp_abi_h) $(infcall_h) $(objc_lang_h) $(block_h) \
-	$(parser_defs_h) $(cp_support_h)
+	$(parser_defs_h) $(cp_support_h) $(exceptions_h) $(uiout_h)
 event-loop.o: event-loop.c $(defs_h) $(event_loop_h) $(event_top_h) \
 	$(gdb_string_h) $(exceptions_h) $(gdb_assert_h) $(gdb_select_h)
 event-top.o: event-top.c $(defs_h) $(top_h) $(inferior_h) $(target_h) \
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.64
diff -u -p -r1.64 eval.c
--- eval.c	10 Oct 2006 03:17:53 -0000	1.64
+++ eval.c	7 Dec 2006 21:54:55 -0000
@@ -37,6 +37,8 @@
 #include "block.h"
 #include "parser-defs.h"
 #include "cp-support.h"
+#include "ui-out.h"
+#include "exceptions.h"
 
 /* This is defined in valops.c */
 extern int overload_resolution;
@@ -465,8 +467,26 @@ evaluate_subexp_standard (struct type *e
 	 value_rtti_target_type () if we are dealing with a pointer
 	 or reference to a base class and print object is on. */
 
-	return value_of_variable (exp->elts[pc + 2].symbol,
-				  exp->elts[pc + 1].block);
+      {
+	volatile struct gdb_exception except;
+	struct value *ret = NULL;
+
+	TRY_CATCH (except, RETURN_MASK_ERROR)
+	  {
+	    ret = value_of_variable (exp->elts[pc + 2].symbol,
+				     exp->elts[pc + 1].block);
+	  }
+
+	if (except.reason < 0)
+	  {
+	    if (noside == EVAL_AVOID_SIDE_EFFECTS)
+	      ret = value_zero (SYMBOL_TYPE (exp->elts[pc + 2].symbol), not_lval);
+	    else
+	      throw_exception (except);
+	  }
+
+	return ret;
+      }
 
     case OP_LAST:
       (*pos) += 2;
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.63
diff -u -p -r1.63 varobj.c
--- varobj.c	6 Dec 2006 09:01:50 -0000	1.63
+++ varobj.c	7 Dec 2006 21:54:55 -0000
@@ -1928,7 +1928,7 @@ c_name_of_child (struct varobj *parent, 
 static struct value *
 c_value_of_root (struct varobj **var_handle)
 {
-  struct value *new_val;
+  struct value *new_val = NULL;
   struct varobj *var = *var_handle;
   struct frame_info *fi;
   int within_scope;
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.20
diff -u -p -r1.20 mi-var-cmd.exp
--- testsuite/gdb.mi/mi-var-cmd.exp	28 Nov 2006 17:23:10 -0000	1.20
+++ testsuite/gdb.mi/mi-var-cmd.exp	7 Dec 2006 21:54:55 -0000
@@ -599,5 +599,48 @@ mi_gdb_test "-var-update selected_a" \
 	"\\^done,changelist=\\\[\{name=\"selected_a\",in_scope=\"true\",new_type=\"int\",new_num_children=\"0\"\}\\\]" \
 	"update selected_a in do_special_tests"
 
+# Test whether bad varobjs crash GDB.
+
+# A varobj we fail to read during -var-update should be considered
+# out of scope.
+mi_gdb_test "-var-create null_ptr * **0" \
+    {\^done,name="null_ptr",numchild="0",type="int"} \
+    "create null_ptr"
+
+# Allow this to succeed, if address zero is readable, although it
+# will not test what it was meant to.  Most important is that GDB
+# does not crash.
+mi_gdb_test "-var-update null_ptr" \
+    {\^done,changelist=\[{.*}\]} \
+    "update null_ptr"
+
+mi_gdb_test "-var-delete null_ptr" \
+    "\\^done,ndeleted=\"1\"" \
+    "delete null_ptr"
+
+# When we fail to read a varobj created from a named variable,
+# we evaluate its type instead.  Make sure that doesn't blow
+# up by trying to read it again.  We can use _end when not running
+# the program to simulate an unreadable variable, if this platform
+# provides _end, but cope if it's missing.
+
+mi_gdb_test "kill" \
+    {&"kill\\n".*\^done} \
+    "kill program before endvar"
+
+mi_gdb_test "-var-create endvar * _end" \
+    {(\^done,name="endvar",numchild="0",type=".*"|&".*unable to.*".*\^error,msg=".*")} \
+    "create endvar"
+
+# Allow this to succeed whether the value is readable, unreadable, or
+# missing.  Most important is that GDB does not crash.
+mi_gdb_test "-var-update endvar" \
+    {(\^done,changelist=\[.*\]|^".*".*\^error,msg=".*not found")} \
+    "update endvar"
+
+mi_gdb_test "-var-delete endvar" \
+    "\\^done,ndeleted=\"1\"" \
+    "delete endvar"
+
 mi_gdb_exit
 return 0