[PATCH 02/28] gcore, handle exited threads better

Pedro Alves
2020-04-14T17:54:43Z

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(-)

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)
--
2.14.5