[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

Tue Mar 30 06:16:18 PDT 2021

mgorny added a comment.

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.

> 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.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98822/new/

https://reviews.llvm.org/D98822