From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17342 invoked by alias); 25 May 2011 20:52:20 -0000 Received: (qmail 17320 invoked by uid 22791); 25 May 2011 20:52:19 -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; Wed, 25 May 2011 20:52:04 +0000 Received: (qmail 8933 invoked from network); 25 May 2011 20:52:03 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 25 May 2011 20:52:03 -0000 From: Pedro Alves To: Tom Tromey Subject: Re: don't run watchpoint commands when the watchpoint goes out of scope Date: Wed, 25 May 2011 20:52:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-8-generic; KDE/4.6.2; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <201105241607.21740.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201105252152.00366.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/msg00592.txt.bz2 On Tuesday 24 May 2011 19:56:22, Tom Tromey wrote: > >>>>> "Pedro" == Pedro Alves writes: > > Pedro> Walking the related breakpoints in map_breakpoint_numbers > Pedro> is bogus. It should be the map_breakpoint_numbers' callback > Pedro> responsibility to iterate over the related breakpoints if it > Pedro> so needs/wants. > > I totally agree; but I wonder what relies on this. > Annotation indicates that this code has been there forever. Hmm, I thought I had made sure nothing relies on this, but I may have misread the code. Sorry about that. On "delete", we'd previously delete watchpoint's "related" breakpoints, but now we're leaving in place with disp == del on next stop (done directly from within delete_breakpoint. That shouldn't hurt, other than causing a stop on the scope breakpoint once for nothing. On "enable"/"disable", it shouldn't make a difference for watchpoint "related" breakpoints either, since we never disable those breakpoints. The new ifunc "related" breakpoints is the large piece that I apparently missed (most of it is outside breakpoint.c). It looks like that it may happen that we may end up with "related" breakpoints pending off a bp_gnu_ifunc_resolver breakpoint, and we should delete them all with "delete". Thing is, "delete FOO" used to delete all the related breakpoints (because it goes through map_breakpoint_numbers), while "delete" does not, because that just calls delete_breakpoint directly on each user breakpoint, and delete_breakpoint doesn't go through ifunc related breakpoints deleting them. Hmm. Either make delete_breakpoint itself always handle related breakpoints, or make the "delete" command (and others) walk related breakpoints in the paths that don't use map_breakpoint_numbers? This looks like will need a bit more fixing than I'd like to propose myself to at the moment (my fix-bugs-as-I-trip-on-them stack is already nested too deep). Better keep the old bugs than add new different bugs, so this patch restores the old behavior of iterating over the "related" breakpoints, except in the case of the "commands" command. I'll apply it tomorrow, barring comments. Tested on x86_64-linux, no regressions. Thanks, -- Pedro Alves 2011-05-25 Pedro Alves gdb/ * breakpoint.c (iterate_related_breakpoints): New. (do_map_delete_breakpoint): New. (delete_command): Pass do_map_delete_breakpoint to map_breakpoint_numbers. (do_disable_breakpoint): New. (do_map_disable_breakpoint): Iterate over the breakpoint's related breakpoints. (do_enable_breakpoint): Rename to ... (enable_breakpoint_disp): ... this. (enable_breakpoint): Adjust. (do_enable_breakpoint): New. (enable_once_breakpoint): Delete. (do_map_enable_breakpoint): New. (do_map_enable_once_breakpoint): New. (enable_once_command, enable_delete_command) (delete_trace_command): Iterate over the breakpoint's related breakpoints. --- gdb/breakpoint.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 15 deletions(-) Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2011-05-25 18:17:31.000000000 +0100 +++ src/gdb/breakpoint.c 2011-05-25 20:56:06.637364386 +0100 @@ -176,7 +176,7 @@ static void hbreak_command (char *, int) static void thbreak_command (char *, int); -static void do_enable_breakpoint (struct breakpoint *, enum bpdisp); +static void enable_breakpoint_disp (struct breakpoint *, enum bpdisp); static void stop_command (char *arg, int from_tty); @@ -10812,8 +10812,42 @@ make_cleanup_delete_breakpoint (struct b return make_cleanup (do_delete_breakpoint_cleanup, b); } -/* A callback for map_breakpoint_numbers that calls - delete_breakpoint. */ +/* Iterator function to call a user-provided callback function once + for each of B and its related breakpoints. */ + +static void +iterate_over_related_breakpoints (struct breakpoint *b, + void (*function) (struct breakpoint *, + void *), + void *data) +{ + struct breakpoint *related; + + related = b; + do + { + struct breakpoint *next; + + /* FUNCTION may delete RELATED. */ + next = related->related_breakpoint; + + if (next == related) + { + /* RELATED is the last ring entry. */ + function (related, data); + + /* FUNCTION may have deleted it, so we'd never reach back to + B. There's nothing left to do anyway, so just break + out. */ + break; + } + else + function (related, data); + + related = next; + } + while (related != b); +} static void do_delete_breakpoint (struct breakpoint *b, void *ignore) @@ -10821,6 +10855,15 @@ do_delete_breakpoint (struct breakpoint delete_breakpoint (b); } +/* A callback for map_breakpoint_numbers that calls + delete_breakpoint. */ + +static void +do_map_delete_breakpoint (struct breakpoint *b, void *ignore) +{ + iterate_over_related_breakpoints (b, do_delete_breakpoint, NULL); +} + void delete_command (char *arg, int from_tty) { @@ -10852,7 +10895,7 @@ delete_command (char *arg, int from_tty) } } else - map_breakpoint_numbers (arg, do_delete_breakpoint, NULL); + map_breakpoint_numbers (arg, do_map_delete_breakpoint, NULL); } static int @@ -11672,13 +11715,21 @@ disable_breakpoint (struct breakpoint *b observer_notify_breakpoint_modified (bpt); } +/* A callback for iterate_over_related_breakpoints. */ + +static void +do_disable_breakpoint (struct breakpoint *b, void *ignore) +{ + disable_breakpoint (b); +} + /* A callback for map_breakpoint_numbers that calls disable_breakpoint. */ static void do_map_disable_breakpoint (struct breakpoint *b, void *ignore) { - disable_breakpoint (b); + iterate_over_related_breakpoints (b, do_disable_breakpoint, NULL); } static void @@ -11710,7 +11761,7 @@ disable_command (char *args, int from_tt } static void -do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition) +enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition) { int target_resources_ok; @@ -11771,7 +11822,13 @@ do_enable_breakpoint (struct breakpoint void enable_breakpoint (struct breakpoint *bpt) { - do_enable_breakpoint (bpt, bpt->disposition); + enable_breakpoint_disp (bpt, bpt->disposition); +} + +static void +do_enable_breakpoint (struct breakpoint *bpt, void *arg) +{ + enable_breakpoint (bpt); } /* A callback for map_breakpoint_numbers that calls @@ -11780,7 +11837,7 @@ enable_breakpoint (struct breakpoint *bp static void do_map_enable_breakpoint (struct breakpoint *b, void *ignore) { - enable_breakpoint (b); + iterate_over_related_breakpoints (b, do_enable_breakpoint, NULL); } /* The enable command enables the specified breakpoints (or all defined @@ -11816,27 +11873,39 @@ enable_command (char *args, int from_tty } static void -enable_once_breakpoint (struct breakpoint *bpt, void *ignore) +do_enable_breakpoint_disp (struct breakpoint *bpt, void *arg) +{ + enum bpdisp disp = *(enum bpdisp *) arg; + + enable_breakpoint_disp (bpt, disp); +} + +static void +do_map_enable_once_breakpoint (struct breakpoint *bpt, void *ignore) { - do_enable_breakpoint (bpt, disp_disable); + enum bpdisp disp = disp_disable; + + iterate_over_related_breakpoints (bpt, do_enable_breakpoint_disp, &disp); } static void enable_once_command (char *args, int from_tty) { - map_breakpoint_numbers (args, enable_once_breakpoint, NULL); + map_breakpoint_numbers (args, do_map_enable_once_breakpoint, NULL); } static void -enable_delete_breakpoint (struct breakpoint *bpt, void *ignore) +do_map_enable_delete_breakpoint (struct breakpoint *bpt, void *ignore) { - do_enable_breakpoint (bpt, disp_del); + enum bpdisp disp = disp_del; + + iterate_over_related_breakpoints (bpt, do_enable_breakpoint_disp, &disp); } static void enable_delete_command (char *args, int from_tty) { - map_breakpoint_numbers (args, enable_delete_breakpoint, NULL); + map_breakpoint_numbers (args, do_map_enable_delete_breakpoint, NULL); } static void @@ -12362,7 +12431,7 @@ delete_trace_command (char *arg, int fro } } else - map_breakpoint_numbers (arg, do_delete_breakpoint, NULL); + map_breakpoint_numbers (arg, do_map_delete_breakpoint, NULL); } /* Helper function for trace_pass_command. */