Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PATCHv4 2/2] gdb: make thread_info executing and resumed state more consistent
Date: Thu, 21 Apr 2022 17:45:56 +0100	[thread overview]
Message-ID: <0555ba407582d1bea8011ed3d8c61bc1cefbf21c.1650559183.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1650559183.git.aburgess@redhat.com>

From: Andrew Burgess <andrew.burgess@embecosm.com>

This commit was inspired by this comment from Simon:

  https://sourceware.org/pipermail/gdb-patches/2021-July/180694.html

The comment is about two thread_info flags m_executing and m_resumed.
The m_resumed flag indicates that at the infrun level GDB has set the
thread running, while the m_executing flag indicates that the thread
is actually running at the target level.

It is very common that a thread can be m_resumed, but not m_executing,
that is, core GDB thinks the thread is, or should be, running, but at
the target level the thread isn't currently running.

The comment Simon made was in reply to a patch I posted where I found
a case where a thread was marked m_executing, but not m_resumed, that
is, at the infrun level GDB thought the thread was stopped, but at the
target level the thread was actually running.  Simon suggests that
this feels like an invalid state, and after thinking about it, I
agree.

So, the goal of this commit is to add some asserts to GDB.  The core
idea here is that the resumed and executing flags should only change
in the following pattern, to begin with everything is set false:

  Resumed: false
  Executing: false

Then infrun marks the thread resumed:

  Resumed: true
  Executing: false

Then a target starts the thread executing:

  Resumed: true
  Executing: true

The thread stops, the target spots this and marks the thread no longer
executing:

  Resumed: true
  Executing: false

And finally, infrun acknowledges that the thread is now no longer
resumed:

  Resumed: false
  Executing: false

Notice that at no point is resumed false, while executing is true.

And so we can add these asserts:

 1. The resumed flag should only change state when the executing flag
 is false, and

 2. The executing flag should only change state when the resumed flag
 is true.

I added these asserts and ....

.... it turns out these rules are broken all over the place in GDB, we
have problems like:

 (a) When new threads appear they are marked executing, but not
 resumed, and

 (b) In some places we mark a single thread as resumed, but then
 actually set multiple threads executing.

For (a) it could be argued that this is a legitimate state - this is
actually the problem I addressed in the original patch that Simon was
replying too, however, we don't need to support this as a separate
state, so if we can make this case follow the expected set of state
transitions then it allows us to reduce the number of states that GDB
can be in, which I think is a good thing.

Case (b) seems to just be a bug to me.

My initial approach was to retain the idea that threads are always
created in the non-executing, non-resumed state, and then find all the
places where new threads are created (which should be in the resumed
state), and mark the thread resumed in that case.

However, after looking at the changes for a while, it felt like it
would be simpler to create threads as resumed by default and instead
change the threads back to non-resumed in the few cases where this was
appropriate.  The appropriate case being (as far as I can tell), just
the initial threads created as part of starting up a new inferior.

So, that's what this patch does.  The thread creation routines now
take a flag to indicate if the new thread should be created resumed or
not.  Almost all threads are started in the resumed state, except for
a few cases associated with initial target creation.

In an ideal world, I would have liked that all threads created as part
of the ::create_inferior process would also be created in the
non-resumed state, but that doesn't seem easily achievable right now.
Though I could easily see changing the Linux target, other targets that
I can't test will be harder to get right.  So, for now, after calling
::create_process I assert that the threads are currently resumed, and
then mark the threads as non-resumed.

I do have a plan that I might be able to make further improvements in
the ::create_inferior area, however, I want to see whether this patch
is accepted first before investing further time on this.  I think that
where this patch gets to is good enough for now.

On Testing:

I've tested this primarily on x86-64 GNU/Linux with the unix,
native-gdbserver, and native-extended-gdbserver boards, and see no
regressions.

I have also tried to test some other targets, though the testing has
been pretty minimal:

I installed FreeBSD in an x86-64 VM.  I can build GDB fine, but when I
try to run dejagnu the VM falls over with an out of swap space error,
no matter how much swap I give the machine.  However, before falling
over a few hundred tests do run, and I see no regressions in those
tests.  I've also manually checked that the gdb.base/attach.exp test
works (to do some basic attach testing).  So I'm reasonably confident
that this target should be mostly OK.

As I was working on GNU/Hurd for another bug I had an i386 GNU/Hurd VM
available, so I built GDB in here too.  Due to other issues with this
target the dejagnu testing is pretty useless here, but I checked that
I can start multi-threaded targets, step/continue past thread
creation, and also that I can attach to a multi-threaded target.

I have also built this branch on Windows, using mingw.  I was unable
to get dejagnu testing working on this target, but again I did some
basic testing that GDB could start up with multi-threaded inferiors,
and correctly see new threads as they appear.

I also built a target with a simulator, and checked that I could
startup and run basic tests their too.

I did try making use of the GCC machine farm to test on AIX and
Solaris, but I was unable to get GDB building on any of the machines I
tried in the farm.

Here is a description of all the individual changes:

  * aix-thread.c (sync_threadlists): New thread starts resumed, this
  change was untested.
  * bsd-kvm.c (bsd_kvm_target_open): New thread starts not-resumed,
  this change was untested.
  * bsd-uthread.c (bsd_uthread_target::wait): New thread starts
  resumed, this change was untested.
  (bsd_uthread_target::update_thread_list): Likewise.
  * corelow.c (add_to_thread_list): New threads start non-resumed.
  (core_target_open): Likewise.
  * darwin-nat.c (darwin_nat_target::check_new_threads): New thread
  starts resumed, this change was untested.
  * fbsd-nat.c (fbsd_add_threads): New threads start resumed.
  (fbsd_nat_target::wait): Likewise.
  * fork-child.c (gdb_startup_inferior): All threads should be
  marked resumed at this point.
  * gdbthread.h (thread_info::set_stop_pc): When setting the stop_pc
  a thread should be neither resumed, or executing.
  (clear_stop_pc): Due to where this method is called, the thread
  should be resumed, but definitely executing.  Though it's tempting
  to say we don't care about the resumed status, if we do ever call
  this function before the thread is resumed then that's a
  significant change, and having the assert fail will draw attention
  to that, and give us reason to think about what we're doing.
  (add_thread): Add additional parameter.
  (add_thread_silent): Likewise.
  (add_thread_with_info): Likewise.
  * gnu-nat.c (gnu_nat_target::inf_validate_procs): New threads are
  started resumed.  Only minimal testing of GNU/Hurd was done.
  (gnu_nat_target::create_inferior): Likewise.
  (gnu_nat_target::attach): Force threads back to non-resumed after
  an attach.  This is a hack, but avoids me having to make
  significant changes to GNU/Hurd thread management, which I'm
  reluctant to do given the difficulty testing the target.
  * go32-nat.c (go32_nat_target::create_inferior): New threads are
  started resumed, this change was untested.
  * inf-ptrace.c (inf_ptrace_target::create_inferior): New threads
  are staarted resumed.
  (inf_ptrace_target::attach): Likewise.
  * infcmd.c (post_create_inferior): Current thread should not be
  resumed at this point. Additionally, only set the stop_pc once per
  stop, there's an updated comment, extra debug output, and some of
  the code is re-indented.
  (run_command_1): Mark threads non-resumed after starting up the
  inferior as part of the run command.
  * infrun.c (follow_fork_inferior): After a fork all threads in the
  child should already have been marked as non-resumed.
  (follow_exec): Upon entry the thread performing the exec should
  already be marked as non-resumed.  After the exec the selected
  thread should also be non-resumed.
  (struct scoped_mark_thread_resumed): This new class is used to
  ensure that all the required threads are marked resumed when
  required, this addresses issue (b) above.  I make use of this new
  class in...
  (do_target_resume): Use scoped_mark_thread_resumed to mark all
  threads resumed prior to actually calling into the target to resume
  the threads.  Placing this call here allows me to remove some calls
  to thread_info::set_resumed() in other places...
  (resume_1): Remove calls to thread_info::set_resumed() from here.
  (handle_one): New threads are started resumed.
  (handle_inferior_event): Likewise.  Also assert that the thread is
  not resumed when handling an exec event.
  (finish_step_over): When we need to place a thread back into the
  resumed state so that we can later find its pending event, we must
  mark the thread resumed after setting the stop_pc, see the asserts
  added to set_stop_pc for why.
  (keep_going_stepped_thread): Remove a call to set_resumed thanks
  to our changes in do_target_resume.
  (stop_all_threads): Add some additional debug output.
  * linux-nat.c (attach_proc_task_lwp_callback): New threads are
  added resumed.
  (linux_handle_extended_wait): Likewise.
  (linux_nat_filter_event): Likewise.
  * linux-thread-db.c (record_thread): Likewise.
  * netbsd-nat.c (nbsd_add_threads): Likewise, this change was
  untested.
  (nbsd_nat_target::wait): Likewise.
  * nto-procfs.c (nto_procfs_target::update_thread_list): Likewise.
  * obsd-nat.c (obsd_nat_target::update_thread_list): Likewise.
  (obsd_nat_target::wait): Likewise.
  * process-stratum-target.c
  (process_stratum_target::follow_exec): After an exec, the new
  thread is add non-resumed.
  (process_stratum_target::follow_fork): Likewise after a fork.
  * procfs.c (do_attach): New threads are created resumed, this is
  untested.
  (procfs_target::wait): Likewise.
  (procfs_target::create_inferior): Likewise.
  (procfs_notice_thread): Likewise.
  * ravenscar-thread.c
  (ravenscar_thread_target::add_active_thread): New threads are
  created resumed, this is untested.
  (ravenscar_thread_target::add_thread): Likewise.
  * remote-sim.c (gdbsim_target::create_inferior): New thread is
  created resumed.
  * remote.c (remote_target::remote_add_thread): Likewise.
  (remote_target::add_current_inferior_and_thread): Add extra
  parameter, use this to control if the thread is started resumed or
  not.
  (remote_target::process_initial_stop_replies): Mark threads
  non-resumed.
  (remote_target::start_remote_1): Create thread resumed.
  * sol-thread.c (sol_thread_target::wait): Create thread resumed,
  this is untested.
  (sol_update_thread_list_callback): Likewise.
  * thread.c (add_thread_silent): Add extra parameter, print
  additional thread debug, mark the new thread resumed or not based
  on the parameter.
  (add_thread_with_info): Add an extra parameter, pass it through to
  add_thread_silent.
  (add_thread): Likewise.
  (thread_info::set_executing): Short cut the case where the
  executing flag is not changing state.  A thread must be resumed
  before it can be executing.  Print some additional debug
  information.
  (thread_info::set_resumed): A thread should not be executing when
  we adjust its resumed status.  Also, print some additional debug
  information.
  * tracectf.c (ctf_target_open): New thread is created non-resumed.
  * tracefile-tfile.c (tfile_target_open): Likewise.
  * windows-nat.c (windows_add_thread): New threads are created
  resumed, this has had only minimal testing.
---
 gdb/aix-thread.c             |  4 +-
 gdb/bsd-kvm.c                |  2 +-
 gdb/bsd-uthread.c            |  4 +-
 gdb/corelow.c                |  9 +++-
 gdb/darwin-nat.c             |  2 +-
 gdb/fbsd-nat.c               |  4 +-
 gdb/fork-child.c             |  3 ++
 gdb/gdbthread.h              | 24 +++++++---
 gdb/gnu-nat.c                | 13 +++--
 gdb/go32-nat.c               |  2 +-
 gdb/inf-ptrace.c             |  4 +-
 gdb/infcmd.c                 | 65 +++++++++++++++++++------
 gdb/infrun.c                 | 93 +++++++++++++++++++++++++++++++-----
 gdb/linux-nat.c              |  8 ++--
 gdb/linux-thread-db.c        |  2 +-
 gdb/netbsd-nat.c             |  4 +-
 gdb/nto-procfs.c             |  2 +-
 gdb/obsd-nat.c               |  4 +-
 gdb/process-stratum-target.c |  4 +-
 gdb/procfs.c                 | 12 ++---
 gdb/ravenscar-thread.c       |  4 +-
 gdb/remote-sim.c             |  2 +-
 gdb/remote.c                 | 21 ++++----
 gdb/sol-thread.c             |  4 +-
 gdb/thread.c                 | 40 ++++++++++++----
 gdb/tracectf.c               |  3 +-
 gdb/tracefile-tfile.c        |  2 +-
 gdb/windows-nat.c            | 12 ++++-
 28 files changed, 258 insertions(+), 95 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 4e41cde6694..bd785e86375 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -809,7 +809,7 @@ sync_threadlists (void)
 	    = current_inferior ()->process_target ();
 	  thread = add_thread_with_info (proc_target,
 					 ptid_t (infpid, 0, pbuf[pi].pthid),
-					 priv);
+					 priv, true);
 
 	  pi++;
 	}
@@ -843,7 +843,7 @@ sync_threadlists (void)
 	    {
 	      process_stratum_target *proc_target
 		= current_inferior ()->process_target ();
-	      thread = add_thread (proc_target, pptid);
+	      thread = add_thread (proc_target, pptid, true);
 
 	      aix_thread_info *priv = new aix_thread_info;
 	      thread->priv.reset (priv);
diff --git a/gdb/bsd-kvm.c b/gdb/bsd-kvm.c
index 45c7f116472..d791317420c 100644
--- a/gdb/bsd-kvm.c
+++ b/gdb/bsd-kvm.c
@@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
   core_kd = temp_kd;
   current_inferior ()->push_target (&bsd_kvm_ops);
 
-  thread_info *thr = add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
+  thread_info *thr = add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid, false);
   switch_to_thread (thr);
 
   target_fetch_registers (get_current_regcache (), -1);
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index 759b562cdd0..ea60f0afd49 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -422,7 +422,7 @@ bsd_uthread_target::wait (ptid_t ptid, struct target_waitstatus *status,
   /* Don't let the core see a ptid without a corresponding thread.  */
   thread_info *thread = find_thread_ptid (beneath, ptid);
   if (thread == NULL || thread->state == THREAD_EXITED)
-    add_thread (beneath, ptid);
+    add_thread (beneath, ptid, true);
 
   return ptid;
 }
@@ -480,7 +480,7 @@ bsd_uthread_target::update_thread_list ()
 	  if (inferior_ptid.tid () == 0)
 	    thread_change_ptid (proc_target, inferior_ptid, ptid);
 	  else
-	    add_thread (proc_target, ptid);
+	    add_thread (proc_target, ptid, true);
 	}
 
       addr = bsd_uthread_read_memory_address (addr + offset);
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 8c33fb7ebb2..0d041d4030d 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -365,7 +365,8 @@ add_to_thread_list (asection *asect, asection *reg_sect)
 
   ptid_t ptid (pid, lwpid);
 
-  thread_info *thr = add_thread (inf->process_target (), ptid);
+  /* Pass false to indicate the thread should be added non-resumed.  */
+  thread_info *thr = add_thread (inf->process_target (), ptid, false);
 
 /* Warning, Will Robinson, looking at BFD private data! */
 
@@ -544,7 +545,7 @@ core_target_open (const char *arg, int from_tty)
       if (thread == NULL)
 	{
 	  inferior_appeared (current_inferior (), CORELOW_PID);
-	  thread = add_thread_silent (target, ptid_t (CORELOW_PID));
+	  thread = add_thread_silent (target, ptid_t (CORELOW_PID), false);
 	}
 
       switch_to_thread (thread);
@@ -553,6 +554,10 @@ core_target_open (const char *arg, int from_tty)
   if (current_program_space->exec_bfd () == nullptr)
     locate_exec_from_corefile_build_id (core_bfd, from_tty);
 
+  /* Threads in a core file are not started resumed.  */
+  for (thread_info *thread : current_inferior ()->threads ())
+    gdb_assert (!thread->resumed ());
+
   post_create_inferior (from_tty);
 
   /* Now go through the target stack looking for threads since there
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 17723656930..b3d24a5d990 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -351,7 +351,7 @@ darwin_nat_target::check_new_threads (inferior *inf)
 	  pti->msg_state = DARWIN_RUNNING;
 
 	  /* Add the new thread.  */
-	  add_thread_with_info (this, ptid_t (inf->pid, 0, new_id), pti);
+	  add_thread_with_info (this, ptid_t (inf->pid, 0, new_id), pti, true);
 	  new_thread_vec.push_back (pti);
 	  new_ix++;
 	  continue;
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 8e5107c26f8..8fd54ba5b8c 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -908,7 +908,7 @@ fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
 	    continue;
 #endif
 	  fbsd_lwp_debug_printf ("adding thread for LWP %u", lwps[i]);
-	  add_thread (target, ptid);
+	  add_thread (target, ptid, true);
 	}
     }
 }
@@ -1331,7 +1331,7 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 		{
 		  fbsd_lwp_debug_printf ("adding thread for LWP %u",
 					 pl.pl_lwpid);
-		  add_thread (this, wptid);
+		  add_thread (this, wptid, true);
 		}
 	      ourstatus->set_spurious ();
 	      return wptid;
diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 89003fa617d..c1c41d8eb69 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -126,6 +126,9 @@ gdb_startup_inferior (pid_t pid, int num_traps)
   inferior *inf = current_inferior ();
   process_stratum_target *proc_target = inf->process_target ();
 
+  for (thread_info *thread : inf->threads ())
+    gdb_assert (thread->resumed ());
+
   ptid_t ptid = startup_inferior (proc_target, pid, num_traps, NULL, NULL);
 
   /* Mark all threads non-executing.  */
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 1a33eb61221..2d3e8794939 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -368,6 +368,8 @@ class thread_info : public refcounted_object,
 
   void set_stop_pc (CORE_ADDR stop_pc)
   {
+    gdb_assert (!m_resumed);
+    gdb_assert (!m_executing);
     m_suspend.stop_pc = stop_pc;
   }
 
@@ -375,6 +377,11 @@ class thread_info : public refcounted_object,
 
   void clear_stop_pc ()
   {
+    /* This method is called from within thread_info::set_executing, and
+       so, at that point the thread should always be resumed, but not yet
+       executing.  */
+    gdb_assert (m_resumed);
+    gdb_assert (!m_executing);
     m_suspend.stop_pc.reset ();
   }
 
@@ -601,22 +608,25 @@ using inferior_ref
 /* Create an empty thread list, or empty the existing one.  */
 extern void init_thread_list (void);
 
-/* Add a thread to the thread list, print a message
-   that a new thread is found, and return the pointer to
-   the new thread.  Caller my use this pointer to 
-   initialize the private thread data.  */
+/* Add a thread to the thread list, print a message that a new thread is
+   found, and return the pointer to the new thread.  Caller my use this
+   pointer to initialize the private thread data.  When RESUMED_P is true
+   the thread is created in a resumed state, otherwise, the thread is
+   created in a non-resumed state.  */
 extern struct thread_info *add_thread (process_stratum_target *targ,
-				       ptid_t ptid);
+				       ptid_t ptid, bool resumed_p);
 
 /* Same as add_thread, but does not print a message about new
    thread.  */
 extern struct thread_info *add_thread_silent (process_stratum_target *targ,
-					      ptid_t ptid);
+					      ptid_t ptid,
+					      bool resumed_p);
 
 /* Same as add_thread, and sets the private info.  */
 extern struct thread_info *add_thread_with_info (process_stratum_target *targ,
 						 ptid_t ptid,
-						 private_thread_info *);
+						 private_thread_info *,
+						 bool resumed_p);
 
 /* Delete thread THREAD and notify of thread exit.  If the thread is
    currently not deletable, don't actually delete it but still tag it
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 72314824278..19d2c46021e 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -1085,9 +1085,9 @@ gnu_nat_target::inf_validate_procs (struct inf *inf)
 	      thread_change_ptid (this, inferior_ptid, ptid);
 	    else if (inf->pending_execs != 0)
 	      /* This is a shell thread.  */
-	      add_thread_silent (this, ptid);
+	      add_thread_silent (this, ptid, true);
 	    else
-	      add_thread (this, ptid);
+	      add_thread (this, ptid, true);
 	  }
       }
 
@@ -2120,7 +2120,7 @@ gnu_nat_target::create_inferior (const char *exec_file,
   /* We have something that executes now.  We'll be running through
      the shell at this point (if startup-with-shell is true), but the
      pid shouldn't change.  */
-  thread_info *thr = add_thread_silent (this, ptid_t (pid));
+  thread_info *thr = add_thread_silent (this, ptid_t (pid), true);
   switch_to_thread (thr);
 
   /* Attach to the now stopped child, which is actually a shell...  */
@@ -2187,6 +2187,13 @@ gnu_nat_target::attach (const char *args, int from_tty)
 
   inf_update_procs (inf);
 
+  /* After attaching all threads are stopped.  It would be better if the
+     threads were added in the non-resumed state, but this is not currently
+     the case (the reason for this is just lack of understanding of how
+     threads are managed on HURD).  So we fix up the state here.  */
+  for (thread_info *thr : current_inferior ()->non_exited_threads ())
+    thr->set_resumed (false);
+
   thread_info *thr
     = find_thread_ptid (this, ptid_t (pid, inf_pick_first_thread ()));
   switch_to_thread (thr);
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 475425d9f3b..7bae000c2dd 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -755,7 +755,7 @@ go32_nat_target::create_inferior (const char *exec_file,
   if (!inf->target_is_pushed (this))
     inf->push_target (this);
 
-  thread_info *thr = add_thread_silent (ptid_t (SOME_PID));
+  thread_info *thr = add_thread_silent (ptid_t (SOME_PID), true);
   switch_to_thread (thr);
 
   clear_proceed_status (0);
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index b9767978744..858c99f2ec8 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -97,7 +97,7 @@ inf_ptrace_target::create_inferior (const char *exec_file,
   /* We have something that executes now.  We'll be running through
      the shell at this point (if startup-with-shell is true), but the
      pid shouldn't change.  */
-  thread_info *thr = add_thread_silent (this, ptid);
+  thread_info *thr = add_thread_silent (this, ptid, true);
   switch_to_thread (thr);
 
   unpusher.release ();
@@ -167,7 +167,7 @@ inf_ptrace_target::attach (const char *args, int from_tty)
 
   /* Always add a main thread.  If some target extends the ptrace
      target, it should decorate the ptid later with more info.  */
-  thread_info *thr = add_thread_silent (this, ptid_t (pid));
+  thread_info *thr = add_thread_silent (this, ptid_t (pid), true);
   switch_to_thread (thr);
 
   /* Don't consider the thread stopped until we've processed its
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1beade2acec..fe29a1ef214 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -241,28 +241,50 @@ post_create_inferior (int from_tty)
   infrun_debug_show_threads ("threads in the newly created inferior",
 			     current_inferior ()->non_exited_threads ());
 
+  /* When attaching to a multi-threaded process the "first" thread will be
+     stopped as part of the attach.  We then call stop_all_threads, which
+     will request that all threads stop.  When the first stop is processed
+     we will end up in here.
+
+     So, we expect that the "first" thread of the inferior should be
+     stopped, as well as one other random thread.  All of the other threads
+     should still be considered resumed, but will have a stop event
+     incoming.
+
+     As a consequence, we only make an assertion here about the currently
+     selected thread of the inferior.  Of the remaining threads, we only
+     expect one to be stopped, but we don't assert that.  */
+  gdb_assert (!inferior_thread ()->resumed ());
+
   /* If the target hasn't taken care of this already, do it now.
      Targets which need to access registers during to_open,
      to_create_inferior, or to_attach should do it earlier; but many
      don't need to.  */
   target_find_description ();
 
-  /* Now that we know the register layout, retrieve current PC.  But
-     if the PC is unavailable (e.g., we're opening a core file with
-     missing registers info), ignore it.  */
+  /* Now that we know the register layout, update the stop_pc if it is not
+     already set.  If GDB attached to a running process then the stop_pc
+     will have been set while processing the stop events triggered during
+     the attach.  If this is a core file, or we're just starting a new
+     process, then the stop_pc will not currently be set.
+
+     But, if the PC is unavailable (e.g., we're opening a core file with
+     missing registers info), ignore it.  Obviously, if we're trying to
+     debug a running process and we can't read the PC then this is bad and
+     shouldn't be ignored, but we'll soon hit errors trying to read the PC
+     elsewhere in GDB, so ignoring this here is fine.  */
   thread_info *thr = inferior_thread ();
-
-  thr->clear_stop_pc ();
-  try
-    {
-      regcache *rc = get_thread_regcache (thr);
-      thr->set_stop_pc (regcache_read_pc (rc));
-    }
-  catch (const gdb_exception_error &ex)
-    {
-      if (ex.error != NOT_AVAILABLE_ERROR)
-	throw;
-    }
+  if (!thr->stop_pc_p ())
+    try
+      {
+	regcache *rc = get_thread_regcache (thr);
+	thr->set_stop_pc (regcache_read_pc (rc));
+      }
+    catch (const gdb_exception_error &ex)
+      {
+	if (ex.error != NOT_AVAILABLE_ERROR)
+	  throw;
+      }
 
   if (current_program_space->exec_bfd ())
     {
@@ -460,6 +482,19 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
   infrun_debug_show_threads ("immediately after create_process",
 			     current_inferior ()->non_exited_threads ());
 
+  /* All threads in the new inferior should be resumed, but not executing.
+     We now mark the thread(s) as non-resumed.
+
+     The thread will be marked resumed again later when we call proceed,
+     however, if that fails then we want to ensure the thread is left in a
+     non-resumed state.  */
+  for (thread_info *thread : current_inferior ()->threads ())
+    {
+      gdb_assert (thread->resumed ());
+      gdb_assert (!thread->executing ());
+      thread->set_resumed (false);
+    }
+
   /* We're starting off a new process.  When we get out of here, in
      non-stop mode, finish the state of all threads of that process,
      but leave other threads alone, as they may be stopped in internal
diff --git a/gdb/infrun.c b/gdb/infrun.c
index a814f5cbc2b..b5d1f126bdc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -608,7 +608,13 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
   /* If there is a child inferior, target_follow_fork must have created a thread
      for it.  */
   if (child_inf != nullptr)
-    gdb_assert (!child_inf->thread_list.empty ());
+    {
+      gdb_assert (!child_inf->thread_list.empty ());
+
+      /* Any added thread should have been marked non-resumed already.  */
+      for (thread_info *thread : child_inf->threads ())
+	gdb_assert (!thread->resumed ());
+    }
 
   /* Clear the parent thread's pending follow field.  Do this before calling
      target_detach, so that the target can differentiate the two following
@@ -1175,6 +1181,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
      breakpoint or similar, it's gone now.  We cannot truly
      step-to-next statement through an exec().  */
   thread_info *th = inferior_thread ();
+  gdb_assert (!th->resumed ());
   th->control.step_resume_breakpoint = NULL;
   th->control.exception_resume_breakpoint = NULL;
   th->control.single_step_breakpoints = NULL;
@@ -1256,6 +1263,11 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
   gdb_assert (current_inferior () == inf);
   gdb_assert (current_program_space == inf->pspace);
 
+  /* After the exec, all threads in the inferior should be in a non-resumed
+     state.  */
+  for (thread_info *thread : inf->non_exited_threads ())
+    gdb_assert (!thread->resumed ());
+
   /* Attempt to open the exec file.  SYMFILE_DEFER_BP_RESET is used
      because the proper displacement for a PIE (Position Independent
      Executable) main symbol file will only be computed by
@@ -2249,6 +2261,54 @@ internal_resume_ptid (int user_step)
   return user_visible_resume_ptid (user_step);
 }
 
+/* Before calling into target code to set inferior threads executing we
+   must mark all threads as resumed.  If an exception is thrown while
+   trying to set the threads executing then we should mark the threads as
+   non-resumed.
+
+   Create an instance of this struct before calling the target_* API to
+   set the threads executing.  If the targets are successfully set
+   executing then call the .commit() method.  In all other cases, any
+   threads that were marked resumed will be returned to their non-resumed
+   state.  */
+
+struct scoped_mark_thread_resumed
+{
+  /* Constructor.  All threads matching PTID will be marked as resumed.  */
+  scoped_mark_thread_resumed (process_stratum_target *targ, ptid_t ptid)
+    : m_target (targ), m_ptid (ptid)
+  {
+    gdb_assert (m_target != nullptr);
+    set_resumed (m_target, m_ptid, true);
+  }
+
+  /* Destructor.  If this instance was not committed (by calling the commit
+     method) then mark all threads matching M_PTID as no longer being
+     resumed.  */
+  ~scoped_mark_thread_resumed ()
+  {
+    if (m_target != nullptr)
+      set_resumed (m_target, m_ptid, false);
+  }
+
+  /* Called once all of the threads have successfully be set executing (by
+     calling the target_* API).  After this call the threads this object
+     marked as resumed will be left in the resumed state when the
+     destructor runs.  */
+  void commit ()
+  {
+    m_target = nullptr;
+  }
+
+private:
+
+  /* The target used for marking threads as resumed or non-resumed.  */
+  process_stratum_target *m_target;
+
+  /* The thread (or threads) to mark as resumed.  */
+  ptid_t m_ptid;
+};
+
 /* Wrapper for target_resume, that handles infrun-specific
    bookkeeping.  */
 
@@ -2257,6 +2317,11 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
 {
   struct thread_info *tp = inferior_thread ();
 
+  /* Create a scoped_mark_thread_resumed to mark all threads matching
+     RESUME_PTID as resumed.  */
+  process_stratum_target *curr_target = current_inferior ()->process_target ();
+  scoped_mark_thread_resumed scoped_resume (curr_target, resume_ptid);
+
   gdb_assert (!tp->stop_requested);
 
   /* Install inferior's terminal modes.  */
@@ -2296,6 +2361,9 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
 		       step, gdb_signal_to_symbol_string (sig));
 
   target_resume (resume_ptid, step, sig);
+
+  /* Call commit so SCOPED_RESUME leaves threads marked as resumed.  */
+  scoped_resume.commit ();
 }
 
 /* Resume the inferior.  SIG is the signal to give the inferior
@@ -2455,7 +2523,6 @@ resume_1 (enum gdb_signal sig)
 
 	      resume_ptid = internal_resume_ptid (user_step);
 	      do_target_resume (resume_ptid, false, GDB_SIGNAL_0);
-	      tp->set_resumed (true);
 	      return;
 	    }
 	}
@@ -2664,7 +2731,6 @@ resume_1 (enum gdb_signal sig)
     }
 
   do_target_resume (resume_ptid, step, sig);
-  tp->set_resumed (true);
 }
 
 /* Resume the inferior.  SIG is the signal to give the inferior
@@ -4938,7 +5004,7 @@ handle_one (const wait_one_event &event)
 	     Don't bother adding if it individually exited.  */
 	  if (t == nullptr
 	      && event.ws.kind () != TARGET_WAITKIND_THREAD_EXITED)
-	    t = add_thread (event.target, event.ptid);
+	    t = add_thread (event.target, event.ptid, true);
 	}
 
       if (t != nullptr)
@@ -4956,7 +5022,7 @@ handle_one (const wait_one_event &event)
     {
       thread_info *t = find_thread_ptid (event.target, event.ptid);
       if (t == NULL)
-	t = add_thread (event.target, event.ptid);
+	t = add_thread (event.target, event.ptid, true);
 
       t->stop_requested = 0;
       t->set_executing (false);
@@ -5367,7 +5433,7 @@ handle_inferior_event (struct execution_control_state *ecs)
       ecs->event_thread = find_thread_ptid (ecs->target, ecs->ptid);
       /* If it's a new thread, add it to the thread database.  */
       if (ecs->event_thread == NULL)
-	ecs->event_thread = add_thread (ecs->target, ecs->ptid);
+	ecs->event_thread = add_thread (ecs->target, ecs->ptid, true);
 
       /* Disable range stepping.  If the next step request could use a
 	 range, this will be end up re-enabled then.  */
@@ -5797,6 +5863,10 @@ handle_inferior_event (struct execution_control_state *ecs)
 	 execd thread for that case (this is a nop otherwise).  */
       ecs->event_thread = inferior_thread ();
 
+      /* If we did switch threads above, then the new thread should still
+	 be in the non-resumed state.  */
+      gdb_assert (!ecs->event_thread->resumed ());
+
       ecs->event_thread->set_stop_pc
 	(regcache_read_pc (get_thread_regcache (ecs->event_thread)));
 
@@ -6050,12 +6120,6 @@ finish_step_over (struct execution_control_state *ecs)
 
 	  /* Record the event thread's event for later.  */
 	  save_waitstatus (tp, ecs->ws);
-	  /* This was cleared early, by handle_inferior_event.  Set it
-	     so this pending event is considered by
-	     do_target_wait.  */
-	  tp->set_resumed (true);
-
-	  gdb_assert (!tp->executing ());
 
 	  regcache = get_thread_regcache (tp);
 	  tp->set_stop_pc (regcache_read_pc (regcache));
@@ -6066,6 +6130,10 @@ finish_step_over (struct execution_control_state *ecs)
 			       tp->ptid.to_string ().c_str (),
 			       currently_stepping (tp));
 
+	  /* This was cleared early, by handle_inferior_event.  Set it so
+	     this pending event is considered by do_target_wait.  */
+	  tp->set_resumed (true);
+
 	  /* This in-line step-over finished; clear this so we won't
 	     start a new one.  This is what handle_signal_stop would
 	     do, if we returned false.  */
@@ -7720,7 +7788,6 @@ keep_going_stepped_thread (struct thread_info *tp)
 				     get_frame_address_space (frame),
 				     tp->stop_pc ());
 
-      tp->set_resumed (true);
       resume_ptid = internal_resume_ptid (tp->control.stepping_command);
       do_target_resume (resume_ptid, false, GDB_SIGNAL_0);
     }
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index dff8d07d3f7..e1a38fb6efd 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1041,7 +1041,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 	  /* Also add the LWP to gdb's thread list, in case a
 	     matching libthread_db is not found (or the process uses
 	     raw clone).  */
-	  add_thread (linux_target, lp->ptid);
+	  add_thread (linux_target, lp->ptid, true);
 	  set_running (linux_target, lp->ptid, true);
 	  set_executing (linux_target, lp->ptid, true);
 	}
@@ -1924,7 +1924,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
 	      /* The process is not using thread_db.  Add the LWP to
 		 GDB's list.  */
 	      target_post_attach (new_lp->ptid.lwp ());
-	      add_thread (linux_target, new_lp->ptid);
+	      add_thread (linux_target, new_lp->ptid, true);
 	    }
 
 	  /* Even if we're stopping the thread for some reason
@@ -2793,7 +2793,7 @@ linux_nat_filter_event (int lwpid, int status)
 	      lp = add_lwp (ptid_t (lwpid, lwpid));
 	      lp->stopped = 1;
 	      lp->resumed = 1;
-	      add_thread (linux_target, lp->ptid);
+	      add_thread (linux_target, lp->ptid, true);
 	    }
 	  else
 	    {
@@ -2826,7 +2826,7 @@ linux_nat_filter_event (int lwpid, int status)
 
 		  lp = add_lwp (ptid_t (lwpid, lwpid));
 		  lp->resumed = 1;
-		  add_thread (linux_target, lp->ptid);
+		  add_thread (linux_target, lp->ptid, true);
 		  break;
 		}
 	    }
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 11da82a1abc..6cbde7cbcb3 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1366,7 +1366,7 @@ record_thread (struct thread_db_info *info,
      thread with this PTID, but it's marked exited, then the kernel
      reused the tid of an old thread.  */
   if (tp == NULL || tp->state == THREAD_EXITED)
-    tp = add_thread_with_info (info->process_target, ptid, priv);
+    tp = add_thread_with_info (info->process_target, ptid, priv, true);
   else
     tp->priv.reset (priv);
 
diff --git a/gdb/netbsd-nat.c b/gdb/netbsd-nat.c
index f2eeb3ff61e..0844559e352 100644
--- a/gdb/netbsd-nat.c
+++ b/gdb/netbsd-nat.c
@@ -125,7 +125,7 @@ nbsd_add_threads (nbsd_nat_target *target, pid_t pid)
 	    if (inferior_ptid.lwp () == 0)
 	      thread_change_ptid (target, inferior_ptid, ptid);
 	    else
-	      add_thread (target, ptid);
+	      add_thread (target, ptid, true);
 	  }
       };
 
@@ -651,7 +651,7 @@ nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	  ourstatus->set_spurious ();
       else
 	{
-	  add_thread (this, wptid);
+	  add_thread (this, wptid, true);
 	  ourstatus->set_thread_created ();
 	}
       return wptid;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 61374c9fad4..a71e86f3bee 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -411,7 +411,7 @@ nto_procfs_target::update_thread_list ()
       ptid = ptid_t (pid, 0, tid);
       new_thread = find_thread_ptid (this, ptid);
       if (!new_thread)
-	new_thread = add_thread (ptid);
+	new_thread = add_thread (ptid, true);
       update_thread_private_data (new_thread, tid, status.state, 0);
       status.tid++;
     }
diff --git a/gdb/obsd-nat.c b/gdb/obsd-nat.c
index c313e807d9b..f3ff1a3720d 100644
--- a/gdb/obsd-nat.c
+++ b/gdb/obsd-nat.c
@@ -62,7 +62,7 @@ obsd_nat_target::update_thread_list ()
 	  if (inferior_ptid.lwp () == 0)
 	    thread_change_ptid (this, inferior_ptid, ptid);
 	  else
-	    add_thread (this, ptid);
+	    add_thread (this, ptid, true);
 	}
 
       if (ptrace (PT_GET_THREAD_NEXT, pid, (caddr_t)&pts, sizeof pts) == -1)
@@ -133,7 +133,7 @@ obsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	  if (in_thread_list (this, ptid_t (pid)))
 	    thread_change_ptid (this, ptid_t (pid), wptid);
 	  else
-	    add_thread (this, wptid);
+	    add_thread (this, wptid, true);
 	}
     }
   return wptid;
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 50bb13e4b83..b9eb8fbc990 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -100,7 +100,7 @@ process_stratum_target::follow_exec (inferior *follow_inf, ptid_t ptid,
 	 may decide to unpush itself from the original inferior's target stack
 	 after that, at its discretion.  */
       follow_inf->push_target (orig_inf->process_target ());
-      thread_info *t = add_thread (follow_inf->process_target (), ptid);
+      thread_info *t = add_thread (follow_inf->process_target (), ptid, false);
 
       /* Leave the new inferior / thread as the current inferior / thread.  */
       switch_to_thread (t);
@@ -118,7 +118,7 @@ process_stratum_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
   if (child_inf != nullptr)
     {
       child_inf->push_target (this);
-      add_thread_silent (this, child_ptid);
+      add_thread_silent (this, child_ptid, false);
     }
 }
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index f6ca1345a21..5c42421470f 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -1864,7 +1864,7 @@ do_attach (ptid_t ptid)
 
   /* Add it to gdb's thread list.  */
   ptid = ptid_t (pi->pid, lwpid, 0);
-  thread_info *thr = add_thread (&the_procfs_target, ptid);
+  thread_info *thr = add_thread (&the_procfs_target, ptid, true);
   switch_to_thread (thr);
 }
 
@@ -2222,7 +2222,7 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status,
 		    temp_ptid = ptid_t (pi->pid, temp_tid, 0);
 		    /* If not in GDB's thread list, add it.  */
 		    if (!in_thread_list (this, temp_ptid))
-		      add_thread (this, temp_ptid);
+		      add_thread (this, temp_ptid, true);
 
 		    target_continue_no_signal (ptid);
 		    goto wait_again;
@@ -2281,7 +2281,7 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status,
 		    /* If not in GDB's thread list, add it.  */
 		    temp_ptid = ptid_t (pi->pid, temp_tid, 0);
 		    if (!in_thread_list (this, temp_ptid))
-		      add_thread (this, temp_ptid);
+		      add_thread (this, temp_ptid, true);
 
 		    status->set_stopped (GDB_SIGNAL_0);
 		    return retval;
@@ -2312,7 +2312,7 @@ procfs_target::wait (ptid_t ptid, struct target_waitstatus *status,
 		  /* We have a new thread.  We need to add it both to
 		     GDB's list and to our own.  If we don't create a
 		     procinfo, resume may be unhappy later.  */
-		  add_thread (this, retval);
+		  add_thread (this, retval, true);
 		  if (find_procinfo (retval.pid (),
 				     retval.lwp ()) == NULL)
 		    create_procinfo (retval.pid (),
@@ -2848,7 +2848,7 @@ procfs_target::create_inferior (const char *exec_file,
   /* We have something that executes now.  We'll be running through
      the shell at this point (if startup-with-shell is true), but the
      pid shouldn't change.  */
-  thread_info *thr = add_thread_silent (this, ptid_t (pid));
+  thread_info *thr = add_thread_silent (this, ptid_t (pid), true);
   switch_to_thread (thr);
 
   procfs_init_inferior (pid);
@@ -2863,7 +2863,7 @@ procfs_notice_thread (procinfo *pi, procinfo *thread, void *ptr)
 
   thread_info *thr = find_thread_ptid (&the_procfs_target, gdb_threadid);
   if (thr == NULL || thr->state == THREAD_EXITED)
-    add_thread (&the_procfs_target, gdb_threadid);
+    add_thread (&the_procfs_target, gdb_threadid, true);
 
   return 0;
 }
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index aae6e397327..e40eaaa0842 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -287,7 +287,7 @@ ravenscar_thread_target::add_active_thread ()
   thread_info *active_thr = find_thread_ptid (proc_target, active_ptid);
   if (active_thr == nullptr)
     {
-      active_thr = ::add_thread (proc_target, active_ptid);
+      active_thr = ::add_thread (proc_target, active_ptid, true);
       m_cpu_map[active_ptid.tid ()] = base_cpu;
     }
   return active_thr;
@@ -424,7 +424,7 @@ ravenscar_thread_target::add_thread (struct ada_task_info *task)
 {
   if (find_thread_ptid (current_inferior (), task->ptid) == NULL)
     {
-      ::add_thread (current_inferior ()->process_target (), task->ptid);
+      ::add_thread (current_inferior ()->process_target (), task->ptid, true);
       m_cpu_map[task->ptid.tid ()] = task->base_cpu;
     }
 }
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 4b8afd27688..2b8fa15a634 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -661,7 +661,7 @@ gdbsim_target::create_inferior (const char *exec_file,
 
   inferior_appeared (current_inferior (),
 		     sim_data->remote_sim_ptid.pid ());
-  thread_info *thr = add_thread_silent (this, sim_data->remote_sim_ptid);
+  thread_info *thr = add_thread_silent (this, sim_data->remote_sim_ptid, true);
   switch_to_thread (thr);
 
   insert_breakpoints ();	/* Needed to get correct instruction
diff --git a/gdb/remote.c b/gdb/remote.c
index 75d6bf3919d..9559a8e3aae 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -740,7 +740,8 @@ class remote_target : public process_stratum_target
   int remote_resume_with_vcont (ptid_t ptid, int step,
 				gdb_signal siggnal);
 
-  thread_info *add_current_inferior_and_thread (const char *wait_status);
+  thread_info *add_current_inferior_and_thread (const char *wait_status,
+						bool resumed_p);
 
   ptid_t wait_ns (ptid_t ptid, struct target_waitstatus *status,
 		  target_wait_flags options);
@@ -2568,16 +2569,17 @@ remote_target::remote_add_thread (ptid_t ptid, bool running, bool executing,
      might be confusing to the user.  Be silent then, preserving the
      age old behavior.  */
   if (rs->starting_up || silent_p)
-    thread = add_thread_silent (this, ptid);
+    thread = add_thread_silent (this, ptid, true);
   else
-    thread = add_thread (this, ptid);
+    thread = add_thread (this, ptid, true);
 
   /* We start by assuming threads are resumed.  That state then gets updated
      when we process a matching stop reply.  */
   get_remote_thread_info (thread)->set_resumed ();
 
-  set_executing (this, ptid, executing);
   set_running (this, ptid, running);
+  set_resumed (this, ptid, running);
+  set_executing (this, ptid, executing);
 
   return thread;
 }
@@ -4471,7 +4473,8 @@ remote_target::get_current_thread (const char *wait_status)
    The function returns pointer to the main thread of the inferior. */
 
 thread_info *
-remote_target::add_current_inferior_and_thread (const char *wait_status)
+remote_target::add_current_inferior_and_thread (const char *wait_status,
+						bool resumed_p)
 {
   struct remote_state *rs = get_remote_state ();
   bool fake_pid_p = false;
@@ -4502,7 +4505,7 @@ remote_target::add_current_inferior_and_thread (const char *wait_status)
   /* Add the main thread and switch to it.  Don't try reading
      registers yet, since we haven't fetched the target description
      yet.  */
-  thread_info *tp = add_thread_silent (this, curr_ptid);
+  thread_info *tp = add_thread_silent (this, curr_ptid, resumed_p);
   switch_to_thread_no_regs (tp);
 
   return tp;
@@ -4612,6 +4615,7 @@ remote_target::process_initial_stop_replies (int from_tty)
 	evthread->set_pending_waitstatus (ws);
 
       set_executing (this, event_ptid, false);
+      set_resumed (this, event_ptid, false);
       set_running (this, event_ptid, false);
       get_remote_thread_info (evthread)->set_not_resumed ();
     }
@@ -4906,7 +4910,8 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
 	  /* Target has no concept of threads at all.  GDB treats
 	     non-threaded target as single-threaded; add a main
 	     thread.  */
-	  thread_info *tp = add_current_inferior_and_thread (wait_status);
+	  thread_info *tp
+	    = add_current_inferior_and_thread (wait_status, true);
 	  get_remote_thread_info (tp)->set_resumed ();
 	}
       else
@@ -10476,7 +10481,7 @@ Remote replied unexpectedly while setting startup-with-shell: %s"),
 
   /* vRun's success return is a stop reply.  */
   stop_reply = run_worked ? rs->buf.data () : NULL;
-  add_current_inferior_and_thread (stop_reply);
+  add_current_inferior_and_thread (stop_reply, true);
 
   /* Get updated offsets, if the stub uses qOffsets.  */
   get_offsets ();
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 18754f4f93b..5a70bd145cb 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -456,7 +456,7 @@ sol_thread_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	    {
 	      process_stratum_target *proc_target
 		= current_inferior ()->process_target ();
-	      add_thread (proc_target, rtnval);
+	      add_thread (proc_target, rtnval, true);
 	    }
 	}
     }
@@ -1008,7 +1008,7 @@ sol_update_thread_list_callback (const td_thrhandle_t *th, void *ignored)
     {
       process_stratum_target *proc_target
 	= current_inferior ()->process_target ();
-      add_thread (proc_target, ptid);
+      add_thread (proc_target, ptid, true);
     }
 
   return 0;
diff --git a/gdb/thread.c b/gdb/thread.c
index 378c5ee2d13..cb363dbef1c 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -260,15 +260,15 @@ new_thread (struct inferior *inf, ptid_t ptid)
 }
 
 struct thread_info *
-add_thread_silent (process_stratum_target *targ, ptid_t ptid)
+add_thread_silent (process_stratum_target *targ, ptid_t ptid, bool resumed_p)
 {
   gdb_assert (targ != nullptr);
 
   inferior *inf = find_inferior_ptid (targ, ptid);
 
-  threads_debug_printf ("add thread to inferior %d, ptid %s, target %s",
-			inf->num, ptid.to_string ().c_str (),
-			targ->shortname ());
+  threads_debug_printf
+    ("add thread to inferior %d, ptid %s, target %s, resumed_p = %d",
+     inf->num, ptid.to_string ().c_str (), targ->shortname (), resumed_p);
 
   /* We may have an old thread with the same id in the thread list.
      If we do, it must be dead, otherwise we wouldn't be adding a new
@@ -279,6 +279,14 @@ add_thread_silent (process_stratum_target *targ, ptid_t ptid)
     delete_thread (tp);
 
   tp = new_thread (inf, ptid);
+
+  /* Upon creation, all threads are non-executing, and non-resumed.  Before
+     notifying observers of the new thread, we set the resumed flag to the
+     desired value.  */
+  gdb_assert (!tp->executing ());
+  tp->set_resumed (resumed_p);
+
+  /* Announce the new thread to all observers.  */
   gdb::observers::new_thread.notify (tp);
 
   return tp;
@@ -286,9 +294,9 @@ add_thread_silent (process_stratum_target *targ, ptid_t ptid)
 
 struct thread_info *
 add_thread_with_info (process_stratum_target *targ, ptid_t ptid,
-		      private_thread_info *priv)
+		      private_thread_info *priv, bool resumed_p)
 {
-  thread_info *result = add_thread_silent (targ, ptid);
+  thread_info *result = add_thread_silent (targ, ptid, resumed_p);
 
   result->priv.reset (priv);
 
@@ -300,9 +308,9 @@ add_thread_with_info (process_stratum_target *targ, ptid_t ptid,
 }
 
 struct thread_info *
-add_thread (process_stratum_target *targ, ptid_t ptid)
+add_thread (process_stratum_target *targ, ptid_t ptid, bool resumed_p)
 {
-  return add_thread_with_info (targ, ptid, NULL);
+  return add_thread_with_info (targ, ptid, NULL, resumed_p);
 }
 
 private_thread_info::~private_thread_info () = default;
@@ -341,9 +349,18 @@ thread_info::deletable () const
 void
 thread_info::set_executing (bool executing)
 {
-  m_executing = executing;
+  if (executing == m_executing)
+    return;
+
+  gdb_assert (m_resumed);
+
   if (executing)
     this->clear_stop_pc ();
+
+  threads_debug_printf ("ptid = %s, executing = %d",
+			this->ptid.to_string ().c_str (),
+			executing);
+  m_executing = executing;
 }
 
 /* See gdbthread.h.  */
@@ -354,6 +371,8 @@ thread_info::set_resumed (bool resumed)
   if (resumed == m_resumed)
     return;
 
+  gdb_assert (!m_executing);
+
   process_stratum_target *proc_target = this->inf->process_target ();
 
   /* If we transition from resumed to not resumed, we might need to remove
@@ -361,6 +380,9 @@ thread_info::set_resumed (bool resumed)
   if (!resumed)
     proc_target->maybe_remove_resumed_with_pending_wait_status (this);
 
+  threads_debug_printf ("ptid = %s, resumed = %d",
+			this->ptid.to_string ().c_str (),
+			resumed);
   m_resumed = resumed;
 
   /* If we transition from not resumed to resumed, we might need to add
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 155c8b7db06..cf3ea22d6b0 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1169,7 +1169,8 @@ ctf_target_open (const char *dirname, int from_tty)
 
   inferior_appeared (current_inferior (), CTF_PID);
 
-  thread_info *thr = add_thread_silent (&ctf_ops, ptid_t (CTF_PID));
+  /* Pass false to indicate the thread starts in a non-resumed state.  */
+  thread_info *thr = add_thread_silent (&ctf_ops, ptid_t (CTF_PID), false);
   switch_to_thread (thr);
 
   merge_uploaded_trace_state_variables (&uploaded_tsvs);
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index f84ad2f4a3f..0fd90b23a94 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -557,7 +557,7 @@ tfile_target_open (const char *arg, int from_tty)
 
   inferior_appeared (current_inferior (), TFILE_PID);
 
-  thread_info *thr = add_thread_silent (&tfile_ops, ptid_t (TFILE_PID));
+  thread_info *thr = add_thread_silent (&tfile_ops, ptid_t (TFILE_PID), false);
   switch_to_thread (thr);
 
   if (ts->traceframe_count <= 0)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 1068558cd21..28aa1ae546c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -384,9 +384,9 @@ windows_add_thread (ptid_t ptid, HANDLE h, void *tlb, bool main_thread_p)
      the main thread silently (in reality, this thread is really
      more of a process to the user than a thread).  */
   if (main_thread_p)
-    add_thread_silent (&the_windows_nat_target, ptid);
+    add_thread_silent (&the_windows_nat_target, ptid, true);
   else
-    add_thread (&the_windows_nat_target, ptid);
+    add_thread (&the_windows_nat_target, ptid, true);
 
   /* It's simplest to always set this and update the debug
      registers.  */
@@ -1907,6 +1907,14 @@ windows_nat_target::attach (const char *args, int from_tty)
 #endif
 
   do_initial_windows_stuff (pid, 1);
+
+  /* After attaching all threads are stopped.  It would be better if the
+     threads were added in the non-resumed state, but this is not currently
+     the case (the reason for this is just lack of understanding of how
+     threads are managed on Windows).  So we fix up the state here.  */
+  for (thread_info *thr : current_inferior ()->non_exited_threads ())
+    thr->set_resumed (false);
+
   target_terminal::ours ();
 }
 
-- 
2.25.4


  parent reply	other threads:[~2022-04-21 16:47 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30 20:03 [PATCH 0/3] Changes to thread state tracking Andrew Burgess
2021-08-30 20:03 ` [PATCH 1/3] gdb: make thread_info::executing private Andrew Burgess
2021-09-01 13:53   ` Simon Marchi via Gdb-patches
2021-09-07 11:46     ` Andrew Burgess
2021-08-30 20:03 ` [PATCH 2/3] gdb: make thread_suspend_state::stop_pc optional Andrew Burgess
2021-09-01 14:23   ` Simon Marchi via Gdb-patches
2021-09-07 13:21     ` Andrew Burgess
2021-09-07 14:10       ` Simon Marchi via Gdb-patches
2021-09-08  9:50         ` Andrew Burgess
2021-08-30 20:03 ` [PATCH 3/3] gdb: make thread_info executing and resumed state more consistent Andrew Burgess
2021-09-01 15:09   ` Simon Marchi via Gdb-patches
2021-09-22 11:21     ` Andrew Burgess
2021-09-23 17:14       ` Simon Marchi via Gdb-patches
2021-09-29  8:09         ` Andrew Burgess
2021-10-08 19:33           ` Simon Marchi via Gdb-patches
2022-01-13 18:34   ` [PATCHv3] " Andrew Burgess via Gdb-patches
2022-01-14 17:10     ` Simon Marchi via Gdb-patches
2022-02-24 15:52       ` Andrew Burgess via Gdb-patches
2022-03-03 19:42         ` Simon Marchi via Gdb-patches
2022-03-07  7:39           ` Aktemur, Tankut Baris via Gdb-patches
2022-03-30  9:19         ` Andrew Burgess via Gdb-patches
2022-04-21 16:45     ` [PATCHv4 0/2] Make " Andrew Burgess via Gdb-patches
2022-04-21 16:45       ` [PATCHv4 1/2] gdb: add some additional thread status debug output Andrew Burgess via Gdb-patches
2022-04-21 20:35         ` Lancelot SIX via Gdb-patches
2022-04-22 17:50           ` Andrew Burgess via Gdb-patches
2022-05-03 14:04             ` Andrew Burgess via Gdb-patches
2022-04-21 16:45       ` Andrew Burgess via Gdb-patches [this message]
2022-04-26 13:28       ` gdb: make thread_info executing and resumed state more consistent Nidal Faour via Gdb-patches
2022-08-08 11:04       ` [PATCHv4 0/2] Make " Craig Blackmore
2022-08-08 12:01         ` Andrew Burgess via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0555ba407582d1bea8011ed3d8c61bc1cefbf21c.1650559183.git.aburgess@redhat.com \
    --to=gdb-patches@sourceware.org \
    --cc=aburgess@redhat.com \
    --cc=andrew.burgess@embecosm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox