don't run watchpoint commands when the watchpoint goes out of scope

[Pedro Alves <pedro at codesourcery dot com>]

While diffing gdb.log's between async/sync runs, I noticed
this bug:

(gdb) PASS: gdb.base/commands.exp: end commands on watch
continue
Continuing.
Hardware watchpoint 11: local_var

Old value = 0
New value = 1
factorial (value=1) at ../../../src/gdb/testsuite/gdb.base/run.c:81
81          return (value);
$38 = 1

Watchpoint 11 deleted because the program has left the block in
which its expression is valid.
0x0000000000400556 in main (argc=2, argv=0x7fffffffd9e8, envp=0x7fffffffda00) at ../../../src/gdb/testsuite/gdb.base/run.c:57
57          printf ("%d\n", factorial (1));
No symbol "value" in current context.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
(gdb) PASS: gdb.base/commands.exp: continue with watch


That 'No symbol "value" in current context' caught my
attention.  We stopped because the watchpoint is no longer
in scope, but, where's that "value" reference coming from?

Turns out to be two problems:

 - The test sets a "print value" command in the watchpoint, and
   that error happens because we tried to execute it when the
   watchpoint went out of scope.
   It's bogus to try to run the watchpoint's commands in 
   this case.  We are running its commands because watchpoints
   that go out of scope are also placed on the bpstat chain.
   Fixed by deleting the commands from the watchpoint when
   that happens.  The current scheme it that the watchpoint itself will
   be deleted on next stop, but meanwhile, it's commands shouldn't
   run.

 - The "commands" command sets breakpoint commands using
   map_breakpoint_numbers.  map_breakpoint_numbers
   calls its callback on each breakpoint that matches,
   _and_ on each of the breakpoint's related breakpoints.
   This ends up setting the commands in the watchpoint's
   internal scope breakpoint, which is the related breakpoint
   of the watchpoint, and the breakpoint that caused the stop
   above...  Walking the related breakpoints in map_breakpoint_numbers
   is bogus.  It should be the map_breakpoint_numbers' callback
   responsibility to iterate over the related breakpoints if it
   so needs/wants.

Tested on x86_64-linux and applied.

-- 
Pedro Alves

2011-05-24  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* breakpoint.c (watchpoint_check): If the watchpoint went out of
	scope, clear its command list.
	(map_breakpoint_numbers): Don't walk the related breakpoints list
	of each breakpoint.

	gdb/testsuite/
	* gdb.base/commands.exp (watchpoint_command_test): Check that the
	watchpoint's command list didn't execute when the watchpoint went
	out of scope.

---
 gdb/breakpoint.c                    |   21 +++------------------
 gdb/testsuite/gdb.base/commands.exp |   18 +++++++++++++++---
 2 files changed, 18 insertions(+), 21 deletions(-)

Index: src/gdb/testsuite/gdb.base/commands.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/commands.exp	2011-05-24 15:48:26.228753680 +0100
+++ src/gdb/testsuite/gdb.base/commands.exp	2011-05-24 15:48:38.238753678 +0100
@@ -294,6 +294,9 @@ proc watchpoint_command_test {} {
 	    pass "begin commands on watch"
 	}
     }
+    # See the 'No symbol "value...' fail below.  This command will
+    # fail if it's executed in the wrong frame.  If adjusting the
+    # test, make sure this property holds.
     gdb_test_multiple "print value" "add print command to watch" {
 	-re ">$" {
 	    pass "add print command to watch"
@@ -308,9 +311,18 @@ proc watchpoint_command_test {} {
 	"" \
 	"end commands on watch"
 
-    gdb_test "continue" \
-	"Continuing.*\[Ww\]atchpoint $wp_id deleted because the program has left the block in.*which its expression is valid.*run.c:(57|82).*" \
-	"continue with watch"
+    set test "continue with watch"
+    gdb_test_multiple "continue" "$test" {
+	-re "No symbol \"value\" in current context.\r\n$gdb_prompt $" {
+	    # Happens if GDB actually runs the watchpoints commands,
+	    # even though the watchpoint was deleted for not being in
+	    # scope.
+	    fail $test
+	}
+ 	-re "Continuing.*\[Ww\]atchpoint $wp_id deleted because the program has left the block in.*which its expression is valid.*run.c:(57|82).*$gdb_prompt $" {
+	    pass $test
+	}
+   }
 }
 
 proc test_command_prompt_position {} {
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2011-05-24 15:48:24.638753681 +0100
+++ src/gdb/breakpoint.c	2011-05-24 15:48:38.238753678 +0100
@@ -3880,6 +3880,8 @@ watchpoint_check (void *p)
 		   " deleted because the program has left the block in\n\
 which its expression is valid.\n");     
 
+      /* Make sure the watchpoint's commands aren't executed.  */
+      decref_counted_command_line (&b->commands);
       watchpoint_del_at_next_stop (b);
 
       return WP_DELETED;
@@ -11599,25 +11601,8 @@ map_breakpoint_numbers (char *args, void
 	  ALL_BREAKPOINTS_SAFE (b, tmp)
 	    if (b->number == num)
 	      {
-		struct breakpoint *related_breakpoint;
-
 		match = 1;
-		related_breakpoint = b;
-		do
-		  {
-		    struct breakpoint *next_related_b;
-
-		    /* FUNCTION can be also delete_breakpoint.  */
-		    next_related_b = related_breakpoint->related_breakpoint;
-		    function (related_breakpoint, data);
-
-		    /* For delete_breakpoint of the last entry of the ring we
-		       were traversing we would never get back to B.  */
-		    if (next_related_b == related_breakpoint)
-		      break;
-		    related_breakpoint = next_related_b;
-		  }
-		while (related_breakpoint != b);
+		function (b, data);
 		break;
 	      }
 	  if (match == 0)