From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26825 invoked by alias); 24 May 2011 15:07:47 -0000 Received: (qmail 26817 invoked by uid 22791); 24 May 2011 15:07:45 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 May 2011 15:07:30 +0000 Received: (qmail 25337 invoked from network); 24 May 2011 15:07:29 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 May 2011 15:07:29 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: don't run watchpoint commands when the watchpoint goes out of scope Date: Tue, 24 May 2011 15:07:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-28-generic; KDE/4.6.2; x86_64; ; ) MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201105241607.21740.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-05/txt/msg00569.txt.bz2 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 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)