From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23576 invoked by alias); 10 May 2013 17:18:51 -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 23565 invoked by uid 89); 10 May 2013 17:18:51 -0000 X-Spam-SWARE-Status: No, score=-8.4 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 10 May 2013 17:18:51 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4AHIlEm009439 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 10 May 2013 13:18:47 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4AHIian016104; Fri, 10 May 2013 13:18:45 -0400 Message-ID: <518D2BF4.6090401@redhat.com> Date: Fri, 10 May 2013 17:18:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Doug Evans CC: Philippe Waroquiers , gdb-patches Subject: Re: RFA: fix gdb_assert caused by 'catch signal ...' and fork References: <1368136582.30058.7.camel@soleil> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-05/txt/msg00394.txt.bz2 On 05/10/2013 05:39 PM, Doug Evans wrote: > On Thu, May 9, 2013 at 2:56 PM, Philippe Waroquiers > wrote: >> The attached patch fixes a gdb_assert caused by the combination of catch >> signal and fork: >> break-catch-sig.c:152: internal-error: signal_catchpoint_remove_location: Assertion `signal_catch_counts[iter] > 0' failed. >> >> The problem is that the signal_catch_counts is decremented by detach_breakpoints. >> The fix consists in not detaching breakpoint locations of type bp_loc_other. >> The patch introduces a new test. >> >> Ok to commit ? >> >> Index: gdb/ChangeLog >> =================================================================== >> RCS file: /cvs/src/src/gdb/ChangeLog,v >> retrieving revision 1.15539 >> diff -u -p -r1.15539 ChangeLog >> --- gdb/ChangeLog 9 May 2013 18:03:27 -0000 1.15539 >> +++ gdb/ChangeLog 9 May 2013 21:46:32 -0000 >> @@ -1,3 +1,8 @@ >> +2013-05-09 Philippe Waroquiers >> + >> + * breakpoints.c (detach_breakpoints): Do not >> + detach breakpoints locations with loc_type bp_loc_other. >> + >> 2013-05-09 Doug Evans >> >> * symfile.c (syms_from_objfile_1): Delete args offsets, num_offsets. >> 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 ...] > > 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. -- Pedro Alves