From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20042 invoked by alias); 4 Jun 2010 19:19:20 -0000 Received: (qmail 20024 invoked by uid 22791); 4 Jun 2010 19:19:15 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Jun 2010 19:19:09 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o54JIxwb026105 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 4 Jun 2010 15:19:00 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o54JIvxd029907 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 4 Jun 2010 15:18:58 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o54JIulp000596; Fri, 4 Jun 2010 21:18:56 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o54JItOq000595; Fri, 4 Jun 2010 21:18:55 +0200 Date: Fri, 04 Jun 2010 19:19:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [patch 1/3] Clear stale specific locs, not whole bpts [rediff] Message-ID: <20100604191855.GA29142@host0.dyn.jankratochvil.net> References: <20100503200201.GB30386@host0.dyn.jankratochvil.net> <20100517212243.GA25843@host0.dyn.jankratochvil.net> <201005232240.17414.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201005232240.17414.pedro@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-12-10) 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: 2010-06/txt/msg00131.txt.bz2 On Sun, 23 May 2010 23:40:17 +0200, Pedro Alves wrote: > On Monday 17 May 2010 22:22:43, Jan Kratochvil wrote: > > static void free_bp_location (struct bp_location *loc) > > { > > + iterate_over_threads (bpstat_remove_bp_location_callback, loc); > > I think this isn't the best place for this, considering moribund > breakpoint locations --- it's too late. When you delete a > breakpoint in non-stop/always-inserted modes, you'll delete > the breakpoint, but deleting the bp_location itself is deferred. > This means that in that case, the thread's bpstat chains may still > contain references to bp_locations that are now in the moribund list > (those will be still valid objects, not xfree'd yet), but their owner > breakpoint is gone, meaning bpstat walks can still dereference those > now invalid bp_location->owner pointers. ^^^^^^^ Not "invalid" but they are explicitly NULL: static void update_global_location_list (int should_insert) old_loc->owner = NULL; VEC_safe_push (bp_location_p, moribund_locations, old_loc); And the code expects such state with valid bp_location but NULL its ->owner. struct bpstat_what bpstat_what (bpstat bs) if (bs->breakpoint_at->owner == NULL) bs_class = bp_nostop; Other bs->breakpoint_at->owner dereferencing code either checks it is NULL or such code cannot meet bpstat referencing moribund bp_location. > Instead, can you move this to within in the `if (!found_object)' branch > of update_global_location_list? That is, move it close to where we > detected we had unlinked the bp_location from the global > location list? While I can do the simple move in next mail do you still request it? I try to follow the paradigm to always remove references exactly at the point one of the two peers dies. That makes the code both simple and foolproof for future modifications even by such programmers like me. Therefore if something is referencing bp_location then the reference should die at the point bp_location dies and that's all. Thanks, Jan 2010-06-04 Jan Kratochvil * breakpoint.h (owner): Extend the comment. --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -244,7 +244,8 @@ struct bp_location /* Each breakpoint location must belong to exactly one higher-level breakpoint. This and the DUPLICATE flag are more straightforward - than reference counting. */ + than reference counting. This pointer is NULL iff this bp_location is in + (and therefore only in) moribund_locations. */ struct breakpoint *owner; /* Conditional. Break only if this expression's value is nonzero.