From 8df017996f662ce6ab23aea4abeb8f7ac1f62651 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 18 Jun 2020 21:28:18 +0100 Subject: [PATCH] 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 * 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/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 + + * 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 * 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) -- 2.30.2