PR9681 - gdb lets you set watchpoints on nonexistent struct members

[Pedro Alves <pedro at codesourcery dot com>]

PR9681 is about a regression that makes gdb now let you set
watchpoints on nonexistent struct members.

From the report:

 This snapshot of gdb will happily allow you to put watchpoints on structure
 members which don't exist:
   (gdb) watch myfoo.nosuchmember
   Watchpoint 2: myfoo.nosuchmember

This bug was introduced with the support to setting watchpoints
in inaccessible memory, back around February/March:

 http://sourceware.org/ml/gdb-patches/2008-02/msg00472.html

The issue is that the watchpoint command is considering all errors
to be of type inaccessible-memory, due to the usage
of gdb_evaluate_expression.  Prior to that change, we used
evaluate_expression, which did let exceptions probagate.  In this
particular case of a nonexistent member, it's at evaluation time
that the member is found to be non-existent, although but it's at
parse time that we find that myfoo does or not exist, so
'watch mybar' didn't regress.

The root issue here, is that we're interested in letting
watchpoints the evaluate to inaccessible memory to still
succeed in being created.  All other errors should be treated
as real errors.  So, this patch adds a new MEMORY_ERROR exception/error
type --- this way we can more accuratelly filter the error type
we want to ignore.  There may be places throwing a generic error instead
of using memory_error, I didn't do a general audit, but those would
better be fixed anyway.

No regressions on x86_64-linux, new test included.

Ok?

-- 
Pedro Alves

2008-12-31  Pedro Alves  <pedro@codesourcery.com>

	PR breakpoints/9681:
	* exceptions.h (enum errors): New error type, MEMORY_ERROR.
	* corefile.c (memory_error): Rewrite to throw a MEMORY_ERROR.
	* breakpoint.c (fetch_watchpoint_value): Ignore MEMORY_ERRORs, but
	retrow all other exceptions.

2008-12-31  Pedro Alves  <pedro@codesourcery.com>

	PR breakpoints/9681:
	* gdb.base/watchpoint.exp: Add test regression.

---
 gdb/breakpoint.c                      |   24 ++++++++++++++++++++++--
 gdb/corefile.c                        |   28 ++++++++++------------------
 gdb/exceptions.h                      |    3 +++
 gdb/testsuite/gdb.base/watchpoint.exp |   12 ++++++++++++
 4 files changed, 47 insertions(+), 20 deletions(-)

Index: src/gdb/exceptions.h
===================================================================
--- src.orig/gdb/exceptions.h	2008-12-31 17:59:21.000000000 +0000
+++ src/gdb/exceptions.h	2008-12-31 18:03:18.000000000 +0000
@@ -72,6 +72,9 @@ enum errors {
   /* Problem parsing an XML document.  */
   XML_PARSE_ERROR,
 
+  /* Error accessing memory.  */
+  MEMORY_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
Index: src/gdb/corefile.c
===================================================================
--- src.orig/gdb/corefile.c	2008-12-31 17:59:21.000000000 +0000
+++ src/gdb/corefile.c	2008-12-31 18:03:18.000000000 +0000
@@ -205,30 +205,22 @@ Use the \"file\" or \"exec-file\" comman
 }
 
 
-/* Report a memory error with error().  */
+/* Report a memory error by throwing a MEMORY_ERROR error.  */
 
 void
 memory_error (int status, CORE_ADDR memaddr)
 {
-  struct ui_file *tmp_stream = mem_fileopen ();
-  make_cleanup_ui_file_delete (tmp_stream);
-
   if (status == EIO)
-    {
-      /* Actually, address between memaddr and memaddr + len
-         was out of bounds. */
-      fprintf_unfiltered (tmp_stream, "Cannot access memory at address ");
-      fputs_filtered (paddress (memaddr), tmp_stream);
-    }
+    /* Actually, address between memaddr and memaddr + len was out of
+       bounds.  */
+    throw_error (MEMORY_ERROR,
+		 _("Cannot access memory at address %s"),
+		 paddress (memaddr));
   else
-    {
-      fprintf_filtered (tmp_stream, "Error accessing memory address ");
-      fputs_filtered (paddress (memaddr), tmp_stream);
-      fprintf_filtered (tmp_stream, ": %s.",
-		       safe_strerror (status));
-    }
-
-  error_stream (tmp_stream);
+    throw_error (MEMORY_ERROR,
+		 _("Error accessing memory address %s: %s."),
+		 paddress (memaddr),
+		 safe_strerror (status));
 }
 
 /* Same as target_read_memory, but report an error if can't read.  */
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2008-12-31 17:59:21.000000000 +0000
+++ src/gdb/breakpoint.c	2008-12-31 18:03:18.000000000 +0000
@@ -755,7 +755,7 @@ is_hardware_watchpoint (struct breakpoin
    in *VAL_CHAIN.  RESULTP and VAL_CHAIN may be NULL if the caller does
    not need them.
 
-   If an error occurs while evaluating the expression, *RESULTP will
+   If a memory error occurs while evaluating the expression, *RESULTP will
    be set to NULL.  *RESULTP may be a lazy value, if the result could
    not be read from memory.  It is used to determine whether a value
    is user-specified (we should watch the whole value) or intermediate
@@ -776,6 +776,7 @@ fetch_watchpoint_value (struct expressio
 			struct value **resultp, struct value **val_chain)
 {
   struct value *mark, *new_mark, *result;
+  volatile struct gdb_exception ex;
 
   *valp = NULL;
   if (resultp)
@@ -786,7 +787,26 @@ fetch_watchpoint_value (struct expressio
   /* Evaluate the expression.  */
   mark = value_mark ();
   result = NULL;
-  gdb_evaluate_expression (exp, &result);
+
+  TRY_CATCH (ex, RETURN_MASK_ALL)
+    {
+      result = evaluate_expression (exp);
+    }
+  if (ex.reason < 0)
+    {
+      /* Ignore memory errors, we want watchpoints pointing at
+	 inaccessible memory to still be created; otherwise, throw the
+	 error to some higher catcher.  */
+      switch (ex.error)
+	{
+	case MEMORY_ERROR:
+	  break;
+	default:
+	  throw_exception (ex);
+	  break;
+	}
+    }
+
   new_mark = value_mark ();
   if (mark == new_mark)
     return;
Index: src/gdb/testsuite/gdb.base/watchpoint.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/watchpoint.exp	2008-12-31 18:03:17.000000000 +0000
+++ src/gdb/testsuite/gdb.base/watchpoint.exp	2008-12-31 18:22:50.000000000 +0000
@@ -649,6 +649,18 @@ proc test_inaccessible_watchpoint {} {
     # valid) memory.
 
     if [runto func4] then {
+	# Make sure we only allow memory access errors.
+	set msg "watchpoint refused to insert on nonexistent struct member"
+	gdb_test_multiple "watch struct1.nosuchmember" "watchpoint in nonexisting struct member" {
+	    -re ".*atchpoint \[0-9\]+: struct1.nosuchmember.*$gdb_prompt $" {
+		# PR breakpoints/9681
+		fail $msg
+	    }
+	    -re "There is no member named nosuchmember\\..*$gdb_prompt $" {
+		pass $msg
+	    }
+	}
+
 	gdb_test "watch *global_ptr" ".*atchpoint \[0-9\]+: \\*global_ptr"
 	gdb_test "next" ".*global_ptr = buf.*"
 	gdb_test_multiple "next" "next over ptr init" {