mgorny added inline comments.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1196
+ if (!bool(m_extensions_supported &
+ NativeProcessProtocol::Extension::multiprocess))
+ assert(!m_stdio_handle_up);
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > I don't think this is right. I believe the important distinction is
> > > whether we have non-stop mode enabled or not, because in all-stop mode we
> > > are not able to send stdio while stopped, regardless of how many
> > > processes we're debugging.
> > Well, the code exploded in all-stop mode as well because in order to kill
> > multiple processes, we effectively resume them all. I suppose there could
> > be a way around it (synchronously killing one process after another and
> > waiting for everything to happen) but I'm not convinced this is really
> > worth the added complexity.
> I don't think this needs to be complex at all. What we need to basically do,
> is call StartSTDIOForwarding whenever the protocol allows us to send stdio,
> and StopSTDIOForwarding when we cannot. When we supported just a single
> process, the easiest way to achieve this was to hook into the started/stopped
> events of that process. Now that's no longer true, so maybe we just need to
> hook it up elsewhere.
>
> I think starting could be done directly from the packet handlers
> (c/s/vCont/...). And those handlers already have to check for nonstop mode,
> so any deviations could be handled there:
> ```
> if (all_stop) {
> StartSTDIOForwarding(); // Can forward from now until the stop-reply packet
> is send
> return Success;
> } else {
> SendOKResponse();
> // Can we forward now? Or maybe it can be sent all the time?
> }
> ```
>
> Stopping could probably stay coupled with the sending of the stop-reply
> packet (i.e., handling of the process event), since it's the stop reply which
> determines whether the stdio packet can be sent.
Thanks, this makes sense. Good news is that it seems that I can just wire it
into our current `SendContinueSuccessResponse()`, unless I've missed some other
case (I've grepped for everything calling `Resume()` or `Kill()`). So far I
didn't special-case non-stop mode, as I still need to rework `O` packets for it.
That said, do we want to enable forwarding for kill actions at all? I suppose
this was kinda implicit due to the whole Linux `PTRACE_EVENT_EXIT` ping-pong
but I honestly doubt any output can happen there.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127500/new/
https://reviews.llvm.org/D127500
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
