[binutils-gdb] Fix PR threads/19354: "info threads" error with multiple inferiors

[Pedro Alves <palves at sourceware dot org>]

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

commit a6904d5a6aa63e4bef7207407250d8b76b56f8a9
Author: Pedro Alves <palves@redhat.com>
Date:   Thu Dec 17 14:20:52 2015 +0000

    Fix PR threads/19354: "info threads" error with multiple inferiors
    
    Note: this applies on top of:
     [PATCH] Remove support for LinuxThreads and vendor 2.4 kernels w/ backported NPTL
     https://sourceware.org/ml/gdb-patches/2015-12/msg00214.html
    
    We try to avoid using libthread_db.so to list threads in the inferior
    when debugging live processes, but the code that decides whether to
    use it decides incorrectly if you have more than one inferior, and the
    current inferior doesn't have execution yet.  The result is visible
    as:
    
     (gdb) add-inferior
     Added inferior 2
     (gdb) inferior 2
     [Switching to inferior 2 [<null>] (<noexec>)]
     (gdb) info inferiors
       Num  Description       Executable
       1    process 15397     /home/pedro/gdb/tests/threads
     * 2    <null>
     (gdb) info threads
     Cannot find new threads: generic error
     (gdb)
    
    Fix this by checking whether each inferior has execution rather than
    just the current inferior.
    
    By moving the core updating to linux-nat.c's update_thread_list
    implementation, this also ends up fixing the
    lwp-last-seen-running-on-core updating in the case we're debugging a
    program that uses raw clone rather than pthreads, as linux-thread-db.c
    isn't pushed in the target stack in that scenario.
    
    Tested on x86_64 Fedora 20.
    
    gdb/ChangeLog:
    2015-12-17  Pedro Alves  <palves@redhat.com>
    
    	PR threads/19354
    	* linux-nat.c (linux_nat_update_thread_list): Update process cores
    	each lwp was last seen running on here.
    	* linux-thread-db.c (update_thread_core): Delete.
    	(thread_db_update_thread_list_td_ta_thr_iter): Rename to ...
    	(thread_db_update_thread_list): ... this.  Skip inferiors with
    	execution.  Also call the target beneath.
    	(thread_db_update_thread_list): Delete.
    
    gdb/testsuite/ChangeLog:
    2015-12-17  Pedro Alves  <palves@redhat.com>
    
    	PR threads/19354
    	* gdb.multi/info-threads.exp: New file.

Diff:
---
 gdb/ChangeLog                            | 11 +++++++
 gdb/linux-nat.c                          |  7 +++++
 gdb/linux-thread-db.c                    | 53 ++++++++++++--------------------
 gdb/testsuite/ChangeLog                  |  5 +++
 gdb/testsuite/gdb.multi/info-threads.exp | 39 +++++++++++++++++++++++
 5 files changed, 81 insertions(+), 34 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index aafe95a..3968c47 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
 2015-12-17  Pedro Alves  <palves@redhat.com>
 
+	PR threads/19354
+	* linux-nat.c (linux_nat_update_thread_list): Update process cores
+	each lwp was last seen running on here.
+	* linux-thread-db.c (update_thread_core): Delete.
+	(thread_db_update_thread_list_td_ta_thr_iter): Rename to ...
+	(thread_db_update_thread_list): ... this.  Skip inferiors with
+	execution.  Also call the target beneath.
+	(thread_db_update_thread_list): Delete.
+
+2015-12-17  Pedro Alves  <palves@redhat.com>
+
 	* configure.ac: Remove tkill checks.
 	* configure, config.in: Regenerate.
 	* linux-nat.c: Remove HAVE_TKILL_SYSCALL check.  Update top level
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index af1b764..1f9bb47 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3719,10 +3719,17 @@ linux_nat_thread_alive (struct target_ops *ops, ptid_t ptid)
 static void
 linux_nat_update_thread_list (struct target_ops *ops)
 {
+  struct lwp_info *lwp;
+
   /* We add/delete threads from the list as clone/exit events are
      processed, so just try deleting exited threads still in the
      thread list.  */
   delete_exited_threads ();
+
+  /* Update the processor core that each lwp/thread was last seen
+     running on.  */
+  ALL_LWPS (lwp)
+    lwp->core = linux_common_core_of_thread (lwp->ptid);
 }
 
 static char *
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index ce3f6a1..68b6988 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1324,17 +1324,11 @@ thread_db_find_new_threads_1 (ptid_t ptid)
   thread_db_find_new_threads_2 (ptid, 0);
 }
 
-static int
-update_thread_core (struct lwp_info *info, void *closure)
-{
-  info->core = linux_common_core_of_thread (info->ptid);
-  return 0;
-}
-
-/* Update the thread list using td_ta_thr_iter.  */
+/* Implement the to_update_thread_list target method for this
+   target.  */
 
 static void
-thread_db_update_thread_list_td_ta_thr_iter (struct target_ops *ops)
+thread_db_update_thread_list (struct target_ops *ops)
 {
   struct thread_db_info *info;
   struct inferior *inf;
@@ -1356,34 +1350,25 @@ thread_db_update_thread_list_td_ta_thr_iter (struct target_ops *ops)
       if (thread == NULL || thread->executing)
 	continue;
 
+      /* It's best to avoid td_ta_thr_iter if possible.  That walks
+	 data structures in the inferior's address space that may be
+	 corrupted, or, if the target is running, the list may change
+	 while we walk it.  In the latter case, it's possible that a
+	 thread exits just at the exact time that causes GDB to get
+	 stuck in an infinite loop.  To avoid pausing all threads
+	 whenever the core wants to refresh the thread list, we
+	 instead use thread_from_lwp immediately when we see an LWP
+	 stop.  That uses thread_db entry points that do not walk
+	 libpthread's thread list, so should be safe, as well as more
+	 efficient.  */
+      if (target_has_execution_1 (thread->ptid))
+	continue;
+
       thread_db_find_new_threads_1 (thread->ptid);
     }
-}
 
-/* Implement the to_update_thread_list target method for this
-   target.  */
-
-static void
-thread_db_update_thread_list (struct target_ops *ops)
-{
-  /* It's best to avoid td_ta_thr_iter if possible.  That walks data
-     structures in the inferior's address space that may be corrupted,
-     or, if the target is running, the list may change while we walk
-     it.  In the latter case, it's possible that a thread exits just
-     at the exact time that causes GDB to get stuck in an infinite
-     loop.  To avoid pausing all threads whenever the core wants to
-     refresh the thread list, use thread_from_lwp immediately when we
-     see an LWP stop.  That uses  thread_db entry points that do not
-     walk libpthread's thread list, so should be safe, as well as
-     more efficient.  */
-  if (target_has_execution)
-    ops->beneath->to_update_thread_list (ops->beneath);
-  else
-    thread_db_update_thread_list_td_ta_thr_iter (ops);
-
-  if (target_has_execution)
-    iterate_over_lwps (minus_one_ptid /* iterate over all */,
-		       update_thread_core, NULL);
+  /* Give the beneath target a chance to do extra processing.  */
+  ops->beneath->to_update_thread_list (ops->beneath);
 }
 
 static char *
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 57c6ccd..43c22b1 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-12-17  Pedro Alves  <palves@redhat.com>
+
+	PR threads/19354
+	* gdb.multi/info-threads.exp: New file.
+
 2015-12-15  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.trace/ftrace.exp: Set arg0exp to "$x0" if target
diff --git a/gdb/testsuite/gdb.multi/info-threads.exp b/gdb/testsuite/gdb.multi/info-threads.exp
new file mode 100644
index 0000000..c00b8d7
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/info-threads.exp
@@ -0,0 +1,39 @@
+# Copyright 2009-2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for PR threads/19354.  If you have a threaded
+# inferior running (thread_db must be loaded), switch to another
+# inferior, one that is NOT running yet, and do "info threads", you
+# would get back an error.
+
+standard_testfile hello.c
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {pthreads debug}] } {
+    return -1
+}
+
+clean_restart ${testfile}
+
+if { ![runto_main] } then {
+    return -1
+}
+
+# Add another inferior.
+gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
+
+# "info threads" while inferior 1 has execution and inferior 2 is not
+# running yet should show inferior 1's thread, and give no error.
+gdb_test "info threads" "1    .* main .* at .*$srcfile:.*No selected thread.*"