From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20453 invoked by alias); 10 May 2013 22:14:33 -0000 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 Received: (qmail 20438 invoked by uid 89); 10 May 2013 22:14:33 -0000 X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 Received: from mailrelay008.isp.belgacom.be (HELO mailrelay008.isp.belgacom.be) (195.238.6.174) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 10 May 2013 22:14:31 +0000 X-Belgacom-Dynamic: yes X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AjcDAGRwjVFtgYmV/2dsb2JhbAANRYZ6vGADAYESgxMBAQEEIwRSEAsOCgICJgICVwYTszdykSqBJo0jLDMHgkKBEwOhPYo1 Received: from 149.137-129-109.adsl-dyn.isp.belgacom.be (HELO [192.168.1.6]) ([109.129.137.149]) by relay.skynet.be with ESMTP; 11 May 2013 00:14:28 +0200 Subject: Re: RFA: fix gdb_assert caused by 'catch signal ...' and fork From: Philippe Waroquiers To: Pedro Alves Cc: Doug Evans , gdb-patches In-Reply-To: <518D2BF4.6090401@redhat.com> References: <1368136582.30058.7.camel@soleil> <518D2BF4.6090401@redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Fri, 10 May 2013 22:14:00 -0000 Message-ID: <1368224079.2230.49.camel@soleil> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00402.txt.bz2 On Fri, 2013-05-10 at 18:18 +0100, Pedro Alves wrote: > On 05/10/2013 05:39 PM, Doug Evans wrote: > > On Thu, May 9, 2013 at 2:56 PM, Philippe Waroquiers > > wrote: > >> Index: gdb/breakpoint.c > >> =================================================================== > >> RCS file: /cvs/src/src/gdb/breakpoint.c,v > >> retrieving revision 1.761 > >> diff -u -p -r1.761 breakpoint.c > >> --- gdb/breakpoint.c 7 May 2013 17:04:28 -0000 1.761 > >> +++ gdb/breakpoint.c 9 May 2013 21:46:33 -0000 > >> @@ -3537,6 +3537,9 @@ detach_breakpoints (ptid_t ptid) > >> if (bl->pspace != inf->pspace) > >> continue; > >> > >> + if (bl->loc_type == bp_loc_other) > >> + continue; > >> + > >> if (bl->inserted) > >> val |= remove_breakpoint_1 (bl, mark_inserted); > >> } > > > > I think a comment is required here explaining *why* we continue for > > bp_loc_other. > > [Assuming the patch is correct ...] Yes, adding a comment is a good idea. > > > > However, there's nothing in "bp_loc_other" that says we should always > > continue there. > > Other breakpoint kinds are marked bp_loc_other too. > > The other breakpoint kinds (software watchpoints, catchpoints, > tracepoints) don't require detaching. The state of bp_loc_other > breakpoints, at least at present, is always on the GDB side. > Detaching is required for those breakpoints that is assumed > get auto-cloned by the target/kernel to forked children. > > > Plus avoiding calling remove_breakpoint_1 feels like working around the problem. > > This doesn't feel like the right fix. > > GDB doesn't have an inferior or any other state corresponding > to the process whose breakpoints are being detached. > > An alternative I imagine would be something like adding > "detach breakpoint" target methods (and bl->owner->ops->detach_location, > etc.) and call that instead of remove_breakpoint_1, though it > seems like we'd get the same result (with the present state). But > I won't object to trying that. I do not master much of breakpoint.h/.c but it looks to me that this implies to add quite some code which will at the end either do nothing (for bp_loc_other) or call remove_breakpoint_1 (for others). What would be the advantages of the detach_breakpoint and detach_location target methods ? As long as there is no need (yet) for a different "detach" behaviour depending on specialised bp_location, it looks to me that the single "if" is simpler and corresponds to the description of detach_breakpoints in breakpoint.h. (maybe we just have to add 'software_breakpoint' and 'single_step_breakpoint' in the description in breakpoint.h ?) Or do I miss something about the interest/need for detach_* methods ? Philippe