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);
----------------
mgorny wrote:
> 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.
Oh, and regarding non-stop mode, I've left it as TODO for now. The whole stdio
forwarding needs to be fixed for non-stop mode anyway, and since we don't
expect to have two processes running simultaneously yet, I'm going to look into
it separately.
That said, this is probably going to be a "LLDB extension". FWICS gdb pretty
much uses `O` packets only to deliver debugger-specific messages and doesn't
forward stdio at all, nor special-cases `O` in non-stop mode. My initial idea
is to always send `O` as asynchronous notifications. I suppose we could then
enable stdio forwarding as soon as non-stop mode is enabled, and keep it
enabled until the very end.
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
