This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFA: fix gdb_assert caused by 'catch signal ...' and fork
- From: Pedro Alves <palves at redhat dot com>
- To: Doug Evans <dje at google dot com>
- Cc: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Fri, 10 May 2013 18:18:44 +0100
- Subject: Re: RFA: fix gdb_assert caused by 'catch signal ...' and fork
- References: <1368136582 dot 30058 dot 7 dot camel at soleil> <CADPb22TKv1ohVo751c3YD0oD2YakVtRvM9kcT32-8QUZmtzvwg at mail dot gmail dot com>
On 05/10/2013 05:39 PM, Doug Evans wrote:
> On Thu, May 9, 2013 at 2:56 PM, Philippe Waroquiers
> <philippe.waroquiers@skynet.be> 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 <philippe.waroquiers@skynet.be>
>> +
>> + * breakpoints.c (detach_breakpoints): Do not
>> + detach breakpoints locations with loc_type bp_loc_other.
>> +
>> 2013-05-09 Doug Evans <dje@google.com>
>>
>> * 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