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: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Doug Evans <dje at google dot com>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Sat, 11 May 2013 00:14:39 +0200
- 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> <518D2BF4 dot 6090401 at redhat dot com>
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
> > <philippe.waroquiers@skynet.be> 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