[02/28] gcore, handle exited threads better

Message ID 20200414175434.8047-3-palves@redhat.com
State New
Headers show
Series
  • Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412)
Related show

Commit Message

Rogerio Alves via Gdb-patches April 14, 2020, 5:54 p.m.
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:
yyyy-mm-dd  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.
---
 gdb/linux-tdep.c | 60 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 27 deletions(-)

-- 
2.14.5

Patch

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index f204831e39..3e39a54ba2 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1402,18 +1402,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.  */
@@ -1861,6 +1849,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.  */
 
@@ -1870,7 +1882,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;
@@ -1898,26 +1909,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)