prepare_for_detach and ongoing displaced stepping

I noticed that "detach" while a program was running sometimes resulted
in the process crashing.  I tracked it down to this change to
prepare_for_detach in commit 187b041e ("gdb: move displaced stepping
logic to gdbarch, allow starting concurrent displaced steps"):

    /* Is any thread of this process displaced stepping?  If not,
       there's nothing else to do.  */
 -  if (displaced->step_thread == nullptr)
 +  if (displaced_step_in_progress (inf))
      return;

The problem above is that the condition was inadvertently flipped.  It
should have been:

   if (!displaced_step_in_progress (inf))

So I fixed it, and wrote a testcase to exercise it.  The testcase has
a number of threads constantly stepping over a breakpoint, and then
GDB detaches the process, while threads are running and stepping over
the breakpoint.  And then I was surprised that my testcase would hang
-- GDB would get stuck in an infinite loop in prepare_for_detach,
here:

  while (displaced_step_in_progress (inf))
    {
      ...

What is going on is that since we now have two displaced stepping
buffers, as one displaced step finishes, GDB starts another, and
there's another one already in progress, and on and on, so the
displaced_step_in_progress condition never turns false.  This happens
because we go via the whole handle_inferior_event, which tries to
start new step overs when one finishes.  And also because while we
remove breakpoints from the target before prepare_for_detach is
called, handle_inferior_event ends up calling insert_breakpoints via
e.g. keep_going.

Thinking through all this, I came to the conclusion that going through
the whole handle_inferior_event isn't ideal.  A _lot_ is done by that
function, e.g., some thread may get a signal which is passed to the
inferior, and gdb decides to try to get over the signal handler, which
reinstalls breakpoints.  Or some process may exit.  We can end up
reporting these events via normal_stop while detaching, maybe end up
running some breakpoint commands, or maybe even something runs an
inferior function call.  Etc.  All this after the user has already
declared they don't want to debug the process anymore, by asking to
detach.

I came to the conclusion that it's better to do the minimal amount of
work possible, in a more controlled fashion, without going through
handle_inferior_event.  So in the new approach implemented by this
patch, if there are threads of the inferior that we're detaching in
the middle of a displaced step, stop them, and cancel the displaced
step.  This is basically what stop_all_threads already does, via
wait_one and (the now factored out) handle_one, so I'm reusing those.

gdb/ChangeLog:

	* infrun.c (struct wait_one_event): Move higher up.
	(prepare_for_detach): Abort in-progress displaced steps instead of
	letting them complete.
	(handle_one): If the inferior is detaching, don't add the thread
	back to the global step-over chain.
	(restart_threads): Don't restart threads if detaching.
	(handle_signal_stop): Remove inferior::detaching reference.

0 files changed, 0 insertions, 0 deletions