[binutils-gdb] Fix "thread apply all" with exited threads

[Pedro Alves <palves at sourceware dot org>]

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

commit a25d8bf9c5b2c9d3671f4508c9132485c65c3773
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Mar 24 21:01:29 2015 +0000

    Fix "thread apply all" with exited threads
    
    I noticed that "thread apply all" sometimes crashes.
    
    The problem is that thread_apply_all_command doesn take exited threads
    into account, and we qsort and then walk more elements than there
    really ever were put in the array.  Valgrind shows:
    
     The current thread <Thread ID 3> has terminated.  See `help thread'.
     (gdb) thread apply all p 1
    
     Thread 1 (Thread 0x7ffff7fc2740 (LWP 29579)):
     $1 = 1
     ==29576== Use of uninitialised value of size 8
     ==29576==    at 0x639CA8: set_thread_refcount (thread.c:1337)
     ==29576==    by 0x5C2C7B: do_my_cleanups (cleanups.c:155)
     ==29576==    by 0x5C2CE8: do_cleanups (cleanups.c:177)
     ==29576==    by 0x63A191: thread_apply_all_command (thread.c:1477)
     ==29576==    by 0x50374D: do_cfunc (cli-decode.c:105)
     ==29576==    by 0x506865: cmd_func (cli-decode.c:1893)
     ==29576==    by 0x7562CB: execute_command (top.c:476)
     ==29576==    by 0x647DA4: command_handler (event-top.c:494)
     ==29576==    by 0x648367: command_line_handler (event-top.c:692)
     ==29576==    by 0x7BF7C9: rl_callback_read_char (callback.c:220)
     ==29576==    by 0x64784C: rl_callback_read_char_wrapper (event-top.c:171)
     ==29576==    by 0x647CB5: stdin_event_handler (event-top.c:432)
     ==29576==
     ...
    
    This can happen easily today as linux-nat.c/linux-thread-db.c are
    forgetting to purge non-current exited threads.  But even with that
    fixed, we can always do "thread apply all" with an exited thread
    selected, which won't be deleted until the user switches to another
    thread.  That's what the test added by this commit exercises.
    
    Tested on x86_64 Fedora 20.
    
    gdb/ChangeLog:
    2015-03-24  Pedro Alves  <palves@redhat.com>
    
    	* thread.c (thread_apply_all_command): Take exited threads into
    	account.
    
    gdb/testsuite/ChangeLog:
    2015-03-24  Pedro Alves  <palves@redhat.com>
    
    	* gdb.threads/no-unwaited-for-left.exp: Test "thread apply all".

Diff:
---
 gdb/ChangeLog                                      |  5 +++++
 gdb/testsuite/ChangeLog                            |  4 ++++
 gdb/testsuite/gdb.threads/no-unwaited-for-left.exp |  4 ++++
 gdb/thread.c                                       | 15 ++++++++++-----
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2cd79a5..a5abe28 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-03-24  Pedro Alves  <palves@redhat.com>
 
+	* thread.c (thread_apply_all_command): Take exited threads into
+	account.
+
+2015-03-24  Pedro Alves  <palves@redhat.com>
+
 	* infrun.c (resume, proceed): Mention
 	switch_back_to_stepped_thread, not switch_back_to_stepping.
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 824d6fc..9d9c086 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2015-03-24  Pedro Alves  <palves@redhat.com>
 
+	* gdb.threads/no-unwaited-for-left.exp: Test "thread apply all".
+
+2015-03-24  Pedro Alves  <palves@redhat.com>
+
 	* gdb.threads/schedlock.exp (test_step): No longer expect that
 	"set scheduler-locking step" with "next" over a function call runs
 	threads unlocked.
diff --git a/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp b/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp
index 3bb3336..518301e 100644
--- a/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp
+++ b/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp
@@ -65,3 +65,7 @@ gdb_test "continue" \
 gdb_test "info threads" \
 	 "\r\n\[ \t\]*Id\[ \t\]+Target\[ \t\]+Id\[ \t\]+Frame\[ \t\]*\r\n *3 *Thread \[^\r\n\]* \[^\r\n\]*.*The current thread <Thread ID 1> has terminated.*" \
 	 "only thread 3 left, main thread terminated"
+
+# Make sure thread apply all works when we have exited threads in the
+# list.
+gdb_test "thread apply all print 999" " = 999" "thread apply all with exited thread"
diff --git a/gdb/thread.c b/gdb/thread.c
index 01eb2ba..ec398f5 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1434,9 +1434,10 @@ thread_apply_all_command (char *cmd, int from_tty)
      execute_command.  */
   saved_cmd = xstrdup (cmd);
   make_cleanup (xfree, saved_cmd);
-  tc = thread_count ();
 
-  if (tc)
+  /* Note this includes exited threads.  */
+  tc = thread_count ();
+  if (tc != 0)
     {
       struct thread_info **tp_array;
       struct thread_info *tp;
@@ -1446,8 +1447,6 @@ thread_apply_all_command (char *cmd, int from_tty)
          command.  */
       tp_array = xmalloc (sizeof (struct thread_info *) * tc);
       make_cleanup (xfree, tp_array);
-      ta_cleanup.tp_array = tp_array;
-      ta_cleanup.count = tc;
 
       ALL_NON_EXITED_THREADS (tp)
         {
@@ -1455,9 +1454,15 @@ thread_apply_all_command (char *cmd, int from_tty)
           tp->refcount++;
           i++;
         }
+      /* Because we skipped exited threads, we may end up with fewer
+	 threads in the array than the total count of threads.  */
+      gdb_assert (i <= tc);
 
-      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+      if (i != 0)
+	qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
 
+      ta_cleanup.tp_array = tp_array;
+      ta_cleanup.count = i;
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)