Discussion: "pauseIfNeeded" and similar behavior for non-stop mode

[jreineckearm]

I'd like to understand a little more about the background of why we call things like pauseIfNeeded for non-stop mode.
If I understand correctly, one of the characteristics of non-stop mode is that it allows more commands/operations when one or more threads are running?
Wouldn't it be responsibility of the gdbserver/stub to react appropriately when it can't do something while the target is running?

One worry is that we artificially suppress capabilities of a non-stop GDB (remote) session that may vary from stub to stub/gdb server to gdb server.
Also, depending on your debug target, and as mentioned by others before, unnecessarily pausing the target system may have undesired side effects up to physical damage.

In a first step, I'd like to understand reasons for the status quo and if there is room to change the behavior unconditionally. Of course, we can always add a new config setting to be less rigorous about pausing. Which we then should consider for both non-stop and all-stop modes (basically, GDB and a remote target tell us what they can do in that situation). Appreciate, that our error handling may not be ready for such a change right now.

@jonahgraham , can you help with some background here?

[cwalther]

I can’t really answer most of these questions, but I can add some more random thoughts:

Related to #409.

The decision probably cannot be on non-stop mode alone, because, empirically, with a native target (local debugging), GDB even in non-stop mode still cannot set breakpoints while all threads are running. Maybe it could be done for remote non-stop, as, equally empirically, gdbserver (which is the stub used in the tests and probably has the most similar restrictions to GDB native) does seem to allow it. But do we know that for all stubs?

This uncertainty is why my first thought was to add a configuration option. But the questions don’t stop there. Currently, we do pauseIfNeeded for setting breakpoints and for detaching. Would one option be sufficient to cover both, or are there targets/stubs that can do one but not the other?

Alternatively, could we optimistically probe for the behavior? Try the operation, and only when it fails, pause first and try again? (And if that worked, then store the finding so that next time we don’t have to try again.) That might work in all asynchronous modes. Not in sync mode, where disallowed commands don’t fail but get stuck until the target stops.

One important tidbit to keep in mind is that remote all-stop async mode is similar to sync modes in that most commands are not allowed while all threads are running. The difference is that disallowed commands fail immediately, while in sync mode they get stuck. This is due to the code that issues error message

Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.

in remote.c remote_target::putpkt_binary. If I understand correctly, that is because the remote protocol does not have the three modes non-stop / all-stop async / all-stop sync, but only becomes asynchronous in non-stop mode.

As far as Indel is concerned, this might all become irrelevant depending on how I end up solving the problem that GDB itself temporarily stops threads when we don’t want it to – present an always stopped dummy thread, present threads as stopped even though they are not, pretend to stop threads when GDB asks for it but not actually do it, or something like that. In some of these cases, the situation that all threads are running from the point of view of GDB and the adapter would never occur and both would never get the urge to pause any threads. But I still think it’s worth solving this problem more generally in the adapter, for users who don’t have as complete control over their stub as I do, that’s why I started in this order.

How do others deal with this problem? Eclipse CDT DSF, Microsoft’s GDB adapter, GDB’s own DAP support?

[jonahgraham]

GDB even in non-stop mode still cannot set breakpoints while all threads are running.

This is my best understanding.

DSF has a method executeWithTargetAvailable to allow centralizing the operation of suspending if needed.

cdt-gdb-adapter initially took a very short-cut version of the above behaviour and tried to do it entirely local to methods such as inserting breakpoints. In #339 @Luke-zhang-mchp improved that code to make it more robust.

Please also see the notes that Marc K wrote while implementing this in CDT, they are brief and useful.

In the code is this comment:

Section to support making operations even when the target is unavailable.

Although one would expect to be able to make commands all the time when
in non-stop mode, it turns out that GDB has trouble with some commands
like breakpoints. The safe way to do it is to make sure we have at least
one thread suspended.

Basically, we must make sure one thread is suspended before making
certain operations (currently breakpoints). If that is not the case, we must
first suspend one thread, then perform the specified operations,
and finally resume that thread..
See https://bugs.eclipse.org/bugs/show_bug.cgi?id=242943
and https://bugs.eclipse.org/bugs/show_bug.cgi?id=282273

Note that for multi-process, we need to interrupt all processes
that share the same binary before doing a breakpoint operation on any of
those processes. For simplicity, the logic below interrupts one thread of
every process being debugged, without differentiating on the executable.
Although it may seem wasteful to interrupt all processes when not necessary,
in truth it is not so much; when making a breakpoint operation in Eclipse, that
operation is propagated to all processes anyway, so they will all need to be
interrupted. The case where we are wasteful is when we start or stop debugging
a process (starting a process, attaching to one, auto-attaching to one,
detaching from one, terminating one); in those cases, we only want to apply the
breakpoint operation to that one process and any other using the same binary.
The wastefulness is not such a big deal for that case, and is worth the simpler
solution.
Of course, it can always be improved later on.
See http://bugs.eclipse.org/337893

And the bug has this useful comment too:

Well, this one proved to be a doozy but I think I got something decent.

Here were the design I tried to respect:
1- All breakpoint commands should be supported even when the target is running; that means create/delete, enable/disable, change condition, change ignore count
2- Suspending and Resume the target to execute those breakpoint commands should be done 'silently'; what that means is that no SuspendedDMEvent/ResumedDMEvent should be broadcast.
3- Although suspending/resuming the target is needed for breakpoints, it may apply to some other case in the future.

To be able to not broadcast SuspendedDMEvent/ResumedDMEvent, the RunControl service must be aware of what is going on. So, I added a new method to the IMIRunControl interface to allow to suspend/resume the target generically

IMIRunControl.executeWithTargetAvailable(IDMContext,Sequence.Step[],RequestMonitor) does the following:

1- verify if the target should be suspended
2- suspend the target if needed
3- executed the specified Sequence.Step[]
4- resume the target if it was suspended
5- make sure the relevant MIRunning/MIStopped event are kept silent.

I will continue to run some tests, but at this point, this patch seems to work well. I also added JUnit tests which all pass for GDB 6.6 to 7.0. I'll be committing early in the week.

[jreineckearm]

Thanks, @cwalther and @jonahgraham . This is useful input. Let me digest a little.