Fix "thread apply all" with exited threads
authorPedro Alves <palves@redhat.com>
Tue, 24 Mar 2015 21:01:29 +0000 (21:01 +0000)
committerPedro Alves <palves@redhat.com>
Tue, 24 Mar 2015 21:01:29 +0000 (21:01 +0000)
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".

gdb/ChangeLog
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.threads/no-unwaited-for-left.exp
gdb/thread.c

index 2cd79a5242e4a1042bd3482680072cb06b1a99de..a5abe28e14d1a71a03aacbbd475bac7136d12e22 100644 (file)
@@ -1,3 +1,8 @@
+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
index 824d6fc54bb1fd9f27e7221c4f41833445b3bda3..9d9c086446dc2f037fd5885913b35300c9ebb491 100644 (file)
@@ -1,3 +1,7 @@
+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
index 3bb333689ad6aab277f76855293a4e3e379848fc..518301e7bccce101f93d3307dc31a3cfe42fbe8c 100644 (file)
@@ -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"
index 01eb2ba94f4f2f1b3745665b1a31059fd863a93d..ec398f578d2f97e05d0147139af179e21a6765b1 100644 (file)
@@ -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++)