labath added a comment. In D98822#2658426 <https://reviews.llvm.org/D98822#2658426>, @mgorny wrote:
> In D98822#2658252 <https://reviews.llvm.org/D98822#2658252>, @labath wrote: > >> In D98822#2646052 <https://reviews.llvm.org/D98822#2646052>, @mgorny wrote: >> >>> It includes refactoring clone/fork/vfork event monitoring into a single >>> function. Right now, only forks are supported — the handler creates a local >>> `NativeProcessLinux` instance (@labath, any suggestions how to make this >>> less hacky?), duplicates parent's breakpoints into it (just like the child >>> gets parent's memory) and uses it to clear these breakpoints and detach the >>> child process. >> >> I don't find this particularly hacky. However, I have doubts about the >> breakpoint clearing aspect. If we add that now, I think it will be tricky to >> move that process to the client in the future (how will the client know >> whether the breakpoints were auto-removed?) Given that breakpoints in >> forked process have been borked since forever, I don't see harm in keeping >> that broken for a little while longer. Even a client with extremely limited >> of multiple processes (like the one we had in mind) should not have a >> problem with sending the appropriate `z` packets before it detaches. This >> patch could still come first (it's great that you've separated that out) -- >> it just needs to ensure that there are no breakpoints in the child path >> which would need clearing. > > Actually, that's not a problem at all. If client doesn't indicate > `fork-events` support via `qSupported`, we handle breakpoints server-side. If > it does, we deliver `fork` stop reason and then the client is responsible for > dealing with breakpoints. Yeah, but then we have to maintain two copies of breakpoint-unsetting code in perpetuity. Given that we've managed to come this far with this being completely broken, I think we can wait a little longer until the client support is in place. >> Btw, have you by any chance looked at how gdb determines which clone()s >> constitute a new thread? > > If I read the code correctly, GDB always assumes `PTRACE_EVENT_CLONE` is a > new thread, and does not support spawning processes via it. Ah, cute. I guess someone should let them know about this (and maybe also tell kernel folks that differentiating cloned processes is really hard). :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98822/new/ https://reviews.llvm.org/D98822 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
