[3/3] gdb: avoid redundant calls to target_stop during attach

  When attaching to a non-stop target (e.g. the native Linux target in
the default mode or the remote target after "maint set target-non-stop
on" or "set non-stop on"), GDB sends a stop request to the target at

  (from infcmd.c:attach_command)
  if (target_is_non_stop_p ())
    {
      /* If we find that the current thread isn't stopped, explicitly
         do so now, because we're going to install breakpoints and
         poke at memory.  */

      if (async_exec)
        /* The user requested an `attach&'; stop just one thread.  */
        target_stop (inferior_ptid);
      else
        /* The user requested an `attach', so stop all threads of this
           inferior.  */
        target_stop (ptid_t (inferior_ptid.pid ()));
    }

Suppose we are in all-stop-on-top-of-non-stop mode with multiple
threads.  After sending the stop requests to the threads and
completing the attach command, but before giving the prompt to the
user, GDB waits for the target in 'wait_sync_command_done' and
receives a stop event.  After handling the event, GDB attempts to stop
all threads at

  (top-gdb) bt
  #0  stop_all_threads (reason=0x555557023ef0 "presenting stop to user in all-stop", inf=0x0) at src/gdb/infrun.c:5594
  #1  0x00005555561c0cb5 in stop_all_threads_if_all_stop_mode () at src/gdb/infrun.c:4326
  #2  0x00005555561c19a9 in fetch_inferior_event () at src/gdb/infrun.c:4676
  #3  0x00005555561993b3 in inferior_event_handler (event_type=INF_REG_EVENT) at src/gdb/inf-loop.c:42
  #4  0x000055555621b0e3 in handle_target_event (error=0, client_data=0x0) at src/gdb/linux-nat.c:4316
  #5  0x0000555556f37bf6 in handle_file_event (file_ptr=0x5555587344f0, ready_mask=1) at src/gdbsupport/event-loop.cc:573
  #6  0x0000555556f381f0 in gdb_wait_for_event (block=0) at src/gdbsupport/event-loop.cc:694
  #7  0x0000555556f36ea7 in gdb_do_one_event (mstimeout=-1) at src/gdbsupport/event-loop.cc:217
  #8  0x000055555662ce24 in wait_sync_command_done () at src/gdb/top.c:427
  #9  0x000055555662ced0 in maybe_wait_sync_command_done (was_sync=0) at src/gdb/top.c:444
  #10 0x000055555662d591 in execute_command (p=0x7fffffffe2ba "2", from_tty=1) at src/gdb/top.c:577
  #11 0x000055555626fe2d in catch_command_errors (command=0x55555662ceed <execute_command(char const*, int)>, arg=0x7fffffffe2ad "attach 2892262", from_tty=1, do_bp_actions=true) at src/gdb/main.c:513
  #12 0x000055555627007c in execute_cmdargs (cmdarg_vec=0x7fffffffdad0, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffda1c) at src/gdb/main.c:612
  #13 0x000055555627163b in captured_main_1 (context=0x7fffffffdd20) at src/gdb/main.c:1293
  #14 0x0000555556271871 in captured_main (data=0x7fffffffdd20) at src/gdb/main.c:1314
  #15 0x0000555556271920 in gdb_main (args=0x7fffffffdd20) at src/gdb/main.c:1343
  #16 0x0000555555cf8d0b in main (argc=9, argv=0x7fffffffde58) at src/gdb/gdb.c:39

In stop_all_threads, the thread states are

    [infrun] infrun_debug_show_threads: enter
      [infrun] infrun_debug_show_threads: non-exited threads:
      [infrun] infrun_debug_show_threads:   thread 2892262.2892262.0, executing = 0, resumed = 0, stop_requested = 0, state = STOPPED
      [infrun] infrun_debug_show_threads:   thread 2892262.2892271.0, executing = 1, resumed = 0, stop_requested = 0, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892272.0, executing = 1, resumed = 0, stop_requested = 0, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892273.0, executing = 1, resumed = 0, stop_requested = 0, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892274.0, executing = 1, resumed = 0, stop_requested = 0, state = RUNNING
    [infrun] infrun_debug_show_threads: exit

Note that the first thread appears as 'executing = 0, state =
STOPPED', because it is the thread for which the stop event was
received first.  Also note that for all of the threads we have
'stop_requested = 0' even though we had sent stop requests already in
'attach_command'.  This causes GDB to call 'target_stop' again for
each thread (except the first eventing one, for which 'executing = 0').

The goal of this patch is to avoid the unnecessary calls to
'target_stop', but first a note about this motivation:

The Linux native target keeps track of whether a stop request was
already sent to a thread, and if so, does not send a request again.
The remote target peeks the pending stop events and if there is
already an event queued for the thread, skips sending a vCont packet.
That is, calling 'target_stop' is harmless.  From that perspective, it
may seem like this patch is implementing a very minor optimization.
However, the actual motivation of the patch is to improve the internal
state book-keeping in GDB and to make the execution flow easier to
follow for the developer.

To avoid the redundant calls to 'target_stop', the solution we take is
to set the 'stop_requested' field of threads in 'attach_command' after
stopping the target.  We use the existing
'stop_current_target_threads_ns' function for this purpose.

When we use this approach, in the same 'stop_all_threads' above, this
time we get

    [infrun] infrun_debug_show_threads: enter
      [infrun] infrun_debug_show_threads: non-exited threads:
      [infrun] infrun_debug_show_threads:   thread 2892262.2892262.0, executing = 0, resumed = 0, stop_requested = 1, state = STOPPED
      [infrun] infrun_debug_show_threads:   thread 2892262.2892271.0, executing = 1, resumed = 0, stop_requested = 1, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892272.0, executing = 1, resumed = 0, stop_requested = 1, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892273.0, executing = 1, resumed = 0, stop_requested = 1, state = RUNNING
      [infrun] infrun_debug_show_threads:   thread 2892262.2892274.0, executing = 1, resumed = 0, stop_requested = 1, state = RUNNING
    [infrun] infrun_debug_show_threads: exit

The 'stop_requested = 1' values help GDB avoid skip the 'target_stop'
calls.  GDB goes on to fetch the pending stop events and finishes
'stop_all_threads' successfully.

Above, there is one glitch, though, at

      [infrun] infrun_debug_show_threads:   thread 2892262.2892262.0, executing = 0, resumed = 0, stop_requested = 1, state = STOPPED

The 'stop_requested' field remained set.  This causes a failure in
'gdb.threads/attach-non-stop.exp' where a thread that is expected to
become running remains stopped, because of the following check in
'proceed_after_attach':

  if (!thread->executing ()
      && !thread->stop_requested
      && thread->stop_signal () == GDB_SIGNAL_0)
    {
      switch_to_thread (thread);
      clear_proceed_status (0);
      proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
    }

I think the solution is to clear the 'stop_requested' field after we
process the stop event.  I first thought of doing this inside
'mark_non_executing_threads', but this does not work because the
'stop_requested' field is still used in the execution flow thereafter.
Then I settled at 'fetch_inferior_event' in the block where we are
sure that we should stop.

Continuing the investigation, let us now focus on the synchronous
attach command (i.e. "attach <PID>" without any "&") when the non-stop
mode is on.  In 'attach_command', GDB binds a continuation to the
inferior:

  /* Wait for stop.  */
  inferior->add_continuation ([=] ()
    {
      attach_post_wait (from_tty, mode);
    });

The 'mode' parameter is ATTACH_POST_WAIT_STOP in the sync mode.

In 'attach_post_wait', we have

  else if (mode == ATTACH_POST_WAIT_STOP)
    {
      /* The user requested a plain `attach', so be sure to leave
         the inferior stopped.  */

      /* At least the current thread is already stopped.  */

      /* In all-stop, by definition, all threads have to be already
         stopped at this point.  In non-stop, however, although the
         selected thread is stopped, others may still be executing.
         Be sure to explicitly stop all threads of the process.  This
         should have no effect on already stopped threads.  */
      if (non_stop)
        target_stop (ptid_t (inferior->pid));
      ...

If 'non_stop' is true, then 'target_is_non_stop_p ()' must have been
true as well, and because 'async_exec' is false, we must have done
'target_stop (ptid_t (inferior_ptid.pid ()))' in 'attach_command'
already.  This means, the call to 'target_stop' in the code above is
unnecessary and can be removed.  We make this simplification in the
patch.

Regression-tested on X86-64 Linux using the unix, native-gdbserver,
and native-extended-gdbserver board files.
---
 gdb/infcmd.c | 31 ++++++++++++++++---------------
 gdb/infrun.c |  1 +
 2 files changed, 17 insertions(+), 15 deletions(-)