The one issue with moving this higher up is that some targets are not interruptible while running (e.g. the Mac OS X Kernel debugging target (KDP).) So calling DisableAllBreakpointSites can't do its job. The kernel stub will take care of this when the debugger connection closes, but you need to make sure that you don't block trying to disable breakpoints, which you can't do. However, as long as DisableBreakpointSite for the KDP side of things does the right thing, it should be fine to call DisableAllBreakpointSites in the Process class Detach before calling DoDetach. Probably also fine to move clearing the thread plans there as well, that's the other bit of cleanup everybody does.
Not sure what you mean about not being able to use DisableAllBreakpointSites, however. It will call the virtual DisableBreakpointSite, which does do the right thing. Jim On Mar 25, 2014, at 6:25 AM, Andrew MacPherson <[email protected]> wrote: > Thanks Ed, maybe it should be moved into Process:Detach() in fact? I would > think that everyone would want to clear all breakpoint sites before > detaching. Though I guess we couldn't use DisableAllBreakpointSites() there > because DisableBreakpointSite() in the base Process class just errors out. We > could use Target::CleanupProcess() or else just get the BreakpointLists from > the Target and call ClearAllBreakpointSites() on them though. What do you > think? > > > On Tue, Mar 25, 2014 at 1:53 PM, Ed Maste <[email protected]> wrote: > On 25 March 2014 06:36, Andrew MacPherson <[email protected]> wrote: > > When detaching from a debugged process any breakpoint sites need to be > > cleared before detaching so that they don't generate uncaught SIGTRAPs. > > Target::CleanupProcess() seems to do the necessary cleanup so call this from > > the ProcessLinux::WillDetach() method. > > > > If this is the right fix and if it applies to other OSes as well maybe the > > cleanup call should be moved into an earlier Process class in the hierarchy. > > I fixed a similar issue on FreeBSD in r201724 by calling > DisableAllBreakpointSites() in ProcessFreeBSD::DoDetach, based on > ProcessGDBRemote::DoDetach. I think you're right that this should be > moved earlier, probably not in individual Process classes at all. > > -Ed > > _______________________________________________ > lldb-dev mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
