[binutils-gdb] gcore, handle exited threads better

[Pedro Alves]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8df017996f662ce6ab23aea4abeb8f7ac1f62651

commit 8df017996f662ce6ab23aea4abeb8f7ac1f62651
Author: Pedro Alves <palves@redhat.com>
Date:   Thu Jun 18 21:28:18 2020 +0100

    gcore, handle exited threads better
    
    An early (and since discarded) version of this series tried to make
    exited threads have distinct PTID between each other, and that change
    exposed a problem in linux-tdep.c...  This was exposed by the
    gdb.threads/gcore-stale-thread.exp testcase, which is exactly about
    calling gcore with an exited thread selected:
    
     (gdb) [Thread 0x7ffff7fb6740 (LWP 31523) exited]
     PASS: gdb.threads/gcore-stale-thread.exp: continue to breakpoint: break-here
     gcore /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.threads/gcore-stale-thread/gcore-stale-thread.core
     /home/pedro/gdb/binutils-gdb/build/../src/gdb/inferior.c:66: internal-error: void set_current_inferior(inferior*): Assertion `inf != NULL' failed.
     A problem internal to GDB has been detected,
    
    That was find_inferior_ptid being called on the "exited" ptid, which
    on that previous (and discarded attempt) had pid==-1.  The problem is
    that linux-tdep.c, where it looks for the signalled thread, isn't
    considering exited threads.  Also, while at it, that code isn't
    considering multi-target either, since it is using
    iterate_over_threads which iterates over all threads of all targets.
    Fixed by switching to range-for iteration instead.
    
    gdb/ChangeLog:
    2020-06-18  Pedro Alves  <palves@redhat.com>
    
            * linux-tdep.c (find_signalled_thread(thread_info *,void *)):
            Delete.
            (find_signalled_thread()): New, factored out from
            linux_make_corefile_notes and adjusted to handle exited threads.
            (linux_make_corefile_notes): Adjust to use the new
            find_signalled_thread.

Diff:
---
 gdb/ChangeLog    |  9 +++++++++
 gdb/linux-tdep.c | 60 +++++++++++++++++++++++++++++++-------------------------
 2 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5611c42049c..ab911076394 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-06-18  Pedro Alves  <palves@redhat.com>
+
+	* linux-tdep.c (find_signalled_thread(thread_info *,void *)):
+	Delete.
+	(find_signalled_thread()): New, factored out from
+	linux_make_corefile_notes and adjusted to handle exited threads.
+	(linux_make_corefile_notes): Adjust to use the new
+	find_signalled_thread.
+
 2020-06-18  Pedro Alves  <palves@redhat.com>
 
 	* linux-tdep.c (btrace_fetch): Save/restore current thread instead
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d51d953ee20..fd4337f100d 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1396,18 +1396,6 @@ linux_find_memory_regions (struct gdbarch *gdbarch,
 					 &data);
 }
 
-/* Determine which signal stopped execution.  */
-
-static int
-find_signalled_thread (struct thread_info *info, void *data)
-{
-  if (info->suspend.stop_signal != GDB_SIGNAL_0
-      && info->ptid.pid () == inferior_ptid.pid ())
-    return 1;
-
-  return 0;
-}
-
 /* This is used to pass information from
    linux_make_mappings_corefile_notes through
    linux_find_memory_regions_full.  */
@@ -1855,6 +1843,30 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
   return 1;
 }
 
+/* Find the signalled thread.  In case there's more than one signalled
+   thread, prefer the current thread, if it is signalled.  If no
+   thread was signalled, default to the current thread, unless it has
+   exited, in which case return NULL.  */
+
+static thread_info *
+find_signalled_thread ()
+{
+  thread_info *curr_thr = inferior_thread ();
+  if (curr_thr->state != THREAD_EXITED
+      && curr_thr->suspend.stop_signal != GDB_SIGNAL_0)
+    return curr_thr;
+
+  for (thread_info *thr : current_inferior ()->non_exited_threads ())
+    if (thr->suspend.stop_signal != GDB_SIGNAL_0)
+      return thr;
+
+  /* Default to the current thread, unless it has exited.  */
+  if (curr_thr->state != THREAD_EXITED)
+    return curr_thr;
+
+  return nullptr;
+}
+
 /* Build the note section for a corefile, and return it in a malloc
    buffer.  */
 
@@ -1864,7 +1876,6 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
   struct linux_corefile_thread_data thread_args;
   struct elf_internal_linux_prpsinfo prpsinfo;
   char *note_data = NULL;
-  struct thread_info *curr_thr, *signalled_thr;
 
   if (! gdbarch_iterate_over_regset_sections_p (gdbarch))
     return NULL;
@@ -1892,26 +1903,21 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
     }
 
   /* Like the kernel, prefer dumping the signalled thread first.
-     "First thread" is what tools use to infer the signalled thread.
-     In case there's more than one signalled thread, prefer the
-     current thread, if it is signalled.  */
-  curr_thr = inferior_thread ();
-  if (curr_thr->suspend.stop_signal != GDB_SIGNAL_0)
-    signalled_thr = curr_thr;
-  else
-    {
-      signalled_thr = iterate_over_threads (find_signalled_thread, NULL);
-      if (signalled_thr == NULL)
-	signalled_thr = curr_thr;
-    }
+     "First thread" is what tools use to infer the signalled
+     thread.  */
+  thread_info *signalled_thr = find_signalled_thread ();
 
   thread_args.gdbarch = gdbarch;
   thread_args.obfd = obfd;
   thread_args.note_data = note_data;
   thread_args.note_size = note_size;
-  thread_args.stop_signal = signalled_thr->suspend.stop_signal;
+  if (signalled_thr != nullptr)
+    thread_args.stop_signal = signalled_thr->suspend.stop_signal;
+  else
+    thread_args.stop_signal = GDB_SIGNAL_0;
 
-  linux_corefile_thread (signalled_thr, &thread_args);
+  if (signalled_thr != nullptr)
+    linux_corefile_thread (signalled_thr, &thread_args);
   for (thread_info *thr : current_inferior ()->non_exited_threads ())
     {
       if (thr == signalled_thr)