Re: RFA: fix gdb_assert caused by 'catch signal ...' and fork

[Doug Evans <dje@google.com>]

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.
Plus avoiding calling remove_breakpoint_1 feels like working around the problem.
This doesn't feel like the right fix.