[PATCH] gdb/Windows: Make windows-nat skip iterating GPU threads

[PATCH] gdb/Windows: Make windows-nat skip iterating GPU threads

[Pedro Alves]

When the AMD GPU dbgapi target is pushed on the stack, the GPU threads
are put in the same inferior thread list as the CPU threads.

all_windows_threads() returns an iterator that walks all threads that
have an inferior whose process_target() returns the Windows target.
So it ends up walking GPU threads as well, which is incorrect and
leads to crashes inside gdb/windows-nat.c.  E.g.:

 (gdb) interrupt -a
 Thread 1 received signal SIGSEGV, Segmentation fault.
 0x00007ff6bf27ad9b in windows_nat_target::stop_one_thread (this=0x11b7850, th=0x0, stopping_kind=windows_nat::SK_EXTERNAL) at .../gdb/windows-nat.c:1984
 ...

 (gdb) detach
 Thread 1 received signal SIGSEGV, Segmentation fault.
 0x00007ff774ead9ee in windows_nat_target::detach (this=0x12a77c0, inf=0x1299460, from_tty=0) at .../gdb/windows-nat.c:3047
 ...

Fix this by:

 - Making as_windows_thread_info use dynamic_cast instead of
   gdb::checked_static_cast, so only CPU threads return non-NULL.

 - Making the windows threads iterator operator++ skip threads for
   which as_windows_thread_info() returns NULL.  Same for
   all_windows_threads_range::begin().

 - Adjusting a few spots that iterate over all threads instead of
   using all_windows_threads (because they need info from the
   thread_info object) to likewise skip non-Windows threads.

No new test, as the existing gdb.rocm/ tests would catch this.  We
just can't run them yet due to a few more missing Windows pieces.

Change-Id: If98caf405a7373d95d885dfd31ac491fe4b01c60
---
 gdb/windows-nat.c | 38 ++++++++++++++++++++++++++++++++++----
 gdb/windows-nat.h | 19 ++++++++++++-------
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3399b8c3303..7cdee85c9f0 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -320,6 +320,31 @@ static const struct xlate_exception xlate[] =
 
 #endif /* 0 */
 
+void
+all_windows_threads_iterator::advance ()
+{
+  /* Skip threads owned by targets on the target stack other than
+     windows-nat.  E.g., GPU threads.  */
+  do
+    {
+      ++m_base_iter;
+    }
+  while (m_base_iter != all_non_exited_threads_iterator {}
+	 && as_windows_thread_info (&*m_base_iter) == nullptr);
+}
+
+all_windows_threads_iterator
+all_windows_threads_range::begin () const
+{
+  /* Find the first thread owned by the windows-nat target, if
+     any.  */
+  auto begin = m_base_range.begin ();
+  while (begin != m_base_range.end ()
+	 && as_windows_thread_info (&*begin) == nullptr)
+    ++begin;
+  return all_windows_threads_iterator (begin);
+}
+
 void
 check (BOOL ok, const char *file, int line)
 {
@@ -1332,10 +1357,11 @@ windows_nat_target::stop_interrupt (ptid_t ptid, bool stop_on_first_match)
 {
   for (thread_info &thr : all_non_exited_threads (this))
     {
-      if (!thr.ptid.matches (ptid))
+      auto *w_th = as_windows_thread_info (&thr);
+      if (w_th == nullptr || !thr.ptid.matches (ptid))
 	continue;
 
-      if (stop_one_thread (as_windows_thread_info (&thr), SK_EXTERNAL))
+      if (stop_one_thread (w_th, SK_EXTERNAL))
 	{
 	  if (stop_on_first_match)
 	    return;
@@ -1420,7 +1446,8 @@ windows_nat_target::get_windows_debug_event
 	continue;
 
       auto *th = as_windows_thread_info (&thread);
-      if (thread.internal_state () == THREAD_INT_RUNNING
+      if (th != nullptr
+	  && thread.internal_state () == THREAD_INT_RUNNING
 	  && th->suspended
 	  && th->pending_status.kind () != TARGET_WAITKIND_IGNORE)
 	{
@@ -2308,9 +2335,12 @@ windows_nat_target::detach (inferior *inf, int from_tty)
      flag.  */
   for (thread_info &thr : inf->non_exited_threads ())
     {
+      auto *w_th = as_windows_thread_info (&thr);
+      if (w_th == nullptr)
+	continue;
+
       if (thr.internal_state () != THREAD_INT_RUNNING)
 	{
-	  windows_thread_info *w_th = windows_process->find_thread (thr.ptid);
 	  gdb_signal signo = get_detach_signal (this, thr.ptid);
 
 	  if (signo != w_th->last_sig
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index cbe5429fd84..8972e56feea 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -106,16 +106,19 @@ struct windows_private_thread_info : private_thread_info, windows_thread_info
   {}
 };
 
-/* Get the windows_thread_info object associated with THR.  */
+/* If THR belongs to the windows-nat target, returns the
+   windows_thread_info object associated with it.  Otherwise returns
+   NULL.  */
 
 static inline windows_thread_info *
 as_windows_thread_info (thread_info *thr)
 {
   /* Cast to windows_private_thread_info, which inherits from
-     private_thread_info, and is implicitly convertible to
-     windows_thread_info, the return type.  */
+     windows_thread_info, the return type.  We use dynamic_cast,
+     because the inferior's thread list may have threads from other
+     targets on the target stack.  */
   private_thread_info *priv = thr->priv.get ();
-  return gdb::checked_static_cast<windows_private_thread_info *> (priv);
+  return dynamic_cast<windows_private_thread_info *> (priv);
 }
 
 struct windows_per_inferior : public windows_nat::windows_process_info
@@ -440,7 +443,7 @@ class all_windows_threads_iterator
 
   all_windows_threads_iterator &operator++ ()
   {
-    ++m_base_iter;
+    advance ();
     return *this;
   }
 
@@ -451,6 +454,9 @@ class all_windows_threads_iterator
   { return !(*this == other); }
 
 private:
+  /* Advance to the next windows-nat thread.  */
+  void advance ();
+
   all_non_exited_threads_iterator m_base_iter;
 };
 
@@ -463,8 +469,7 @@ class all_windows_threads_range : public all_non_exited_threads_range
     : m_base_range (base_range)
   {}
 
-  all_windows_threads_iterator begin () const
-  { return all_windows_threads_iterator (m_base_range.begin ()); }
+  all_windows_threads_iterator begin () const;
   all_windows_threads_iterator end () const
   { return all_windows_threads_iterator (m_base_range.end ()); }
 

base-commit: fd3cf86c638b7fdc0ccb64e178ec43ccd7e9d3f9
-- 
2.54.0