From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3540 invoked by alias); 8 Feb 2012 20:56:30 -0000 Received: (qmail 3521 invoked by uid 22791); 8 Feb 2012 20:56:29 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 08 Feb 2012 20:56:13 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1RvEYl-0000ab-TY from Luis_Gustavo@mentor.com ; Wed, 08 Feb 2012 12:56:11 -0800 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by svr-orw-fem-01.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 8 Feb 2012 12:56:11 -0800 Received: from [0.0.0.0] ([172.16.63.104]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 8 Feb 2012 12:56:10 -0800 Message-ID: <4F32E167.6050600@mentor.com> Date: Wed, 08 Feb 2012 20:56:00 -0000 From: Luis Gustavo Reply-To: "Gustavo, Luis" User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15 MIME-Version: 1.0 To: Pedro Alves CC: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [rfc target-side break conditions 3/5 v2] GDB-side changes References: <4F230A13.9060400@mentor.com> <4F3107C2.2060400@mentor.com> <4F32CDF2.8090906@redhat.com> In-Reply-To: <4F32CDF2.8090906@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: 2012-02/txt/msg00114.txt.bz2 Fixed all the other comments... On 02/08/2012 05:33 PM, Pedro Alves wrote: >> /* Delete a breakpoint and clean up all traces of it in the data >> - structures. */ >> + structures. If UPDATE is true, proceed to update the list of >> + locations, otherwise don't update it. */ >> >> -void >> -delete_breakpoint (struct breakpoint *bpt) >> +static void >> +delete_breakpoint_1 (struct breakpoint *bpt, int update) >> { >> struct breakpoint *b; >> >> @@ -11955,8 +12451,12 @@ delete_breakpoint (struct breakpoint *bp >> belong to this breakpoint. Do this before freeing the breakpoint >> itself, since remove_breakpoint looks at location's owner. It >> might be better design to have location completely >> - self-contained, but it's not the case now. */ >> - update_global_location_list (0); >> + self-contained, but it's not the case now. >> + >> + If we have conditions being evaluated on the target, we will also >> + synchronize the list of conditions due to a deleted duplicate >> + breakpoint. */ >> + update_global_location_list (update); >> >> bpt->ops->dtor (bpt); >> /* On the chance that someone will soon try again to delete this >> @@ -11965,6 +12465,24 @@ delete_breakpoint (struct breakpoint *bp >> xfree (bpt); >> } >> >> +/* Delete a breakpoint but don't update the list of locations. */ >> + >> +void >> +delete_breakpoint (struct breakpoint *bpt) >> +{ >> + delete_breakpoint_1 (bpt, 0); >> +} >> + >> +/* Delete a breakpoint and update the list of locations. We use >> + this one to synchronize the list of conditions with the target >> + in case the target is responsible for evaluating them. */ >> + >> +void >> +delete_breakpoint_with_update (struct breakpoint *bpt) >> +{ >> + delete_breakpoint_1 (bpt, 1); >> +} >> + >> static void >> do_delete_breakpoint_cleanup (void *b) >> { >> @@ -12017,7 +12535,10 @@ iterate_over_related_breakpoints (struct >> static void >> do_delete_breakpoint (struct breakpoint *b, void *ignore) >> { >> - delete_breakpoint (b); >> + if (is_breakpoint (b)) >> + delete_breakpoint_with_update (b); >> + else >> + delete_breakpoint (b); >> } >> >> /* A callback for map_breakpoint_numbers that calls >> @@ -12056,7 +12577,7 @@ delete_command (char *arg, int from_tty) >> { >> ALL_BREAKPOINTS_SAFE (b, b_tmp) >> if (user_breakpoint_p (b)) >> - delete_breakpoint (b); >> + do_delete_breakpoint (b, NULL); >> } >> } >> else >> @@ -12840,6 +13361,9 @@ disable_breakpoint (struct breakpoint *b >> >> bpt->enable_state = bp_disabled; >> >> + /* Mark breakpoint locations modified. */ >> + mark_breakpoint_modified (bpt); >> + >> if (target_supports_enable_disable_tracepoint () >> && current_trace_status ()->running&& is_tracepoint (bpt)) >> { >> @@ -12849,7 +13373,10 @@ disable_breakpoint (struct breakpoint *b >> target_disable_tracepoint (location); >> } >> >> - update_global_location_list (0); >> + if (is_breakpoint (bpt)) >> + update_global_location_list (1); >> + else >> + update_global_location_list (0); > > I'm confused. How does this (and all similar places) address the issued I > pointed out before? If you're passing one to update_global_location_list > when deleting a breakpoint, you're pretty much defeating the whole > purpose of the update_global_location_list's argument in the first place. > Updates will only take place when the user explicitly removes/disables a breakpoint. Functions that are deleting breakpoints (like remove_thread_event_breakpoints) won't cause insertion of any breakpoints since they go through the "delete_breakpoint" path, not the delete_breakpoint_with_update one. Is disabling breakpoints also something we would like to do without triggering insertions? If so, i'm inclined to go with the same solution as for deleting user breakpoints.