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

Pedro Alves
2011-05-24T15:07:47Z

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)