Re: Updated patch for Bug 13217 - thread apply all detach throws a SEGFAULT

[ali_anwar <ali_anwar at codesourcery dot com>]

On 09/27/2012 08:32 PM, Tom Tromey wrote:
> > > > > > "Ali" == ali anwar <ali_anwar@codesourcery.com> writes:
>
> Ali> +static int
> Ali> +thread_valid (struct thread_info *tp)
> Ali> +{
> Ali> +  struct thread_info  *utp;
> Ali> +  for (utp = thread_list; utp; utp = utp->next)
> Ali> +     if (tp == utp)
> Ali> +       return 1;
> Ali> +  return 0;
> Ali> +}
>
> Ali> +  for (tp = thread_list; thread_valid(tp); tp = tp->next)
>
> It seems like this introduces quadratic behavior here.
> I think it would be better to avoid this.
>
> Ali> +	if (thread_count() == 0)
> Ali> +	  break;
>
> Here too.
>
> Tom

Please find attached the modified patch. This time the complexity is 
linear. The thread list is saved first and then it is verified that each 
thread still exists or not.

Testing:
* Tested on i686-pc-linux
* Also checked with valgrind
* Executed the testuite

Thanks,
-Ali


Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14894
diff -u -r1.14894 ChangeLog
--- gdb/ChangeLog	9 Dec 2012 18:39:57 -0000	1.14894
+++ gdb/ChangeLog	10 Dec 2012 18:13:27 -0000
@@ -1,3 +1,9 @@
+2012-12-10  Ali Anwar  <ali_anwar@codesourcery.com>
+
+	PR threads/13217
+	* thread.c (thread_apply_all_command): Check for valid threads 
+	and thread count.
+
 2012-12-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* configure.ac (CC_HAS_LONG_LONG): Replace by AC_MSG_ERROR.
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.149
diff -u -r1.149 thread.c
--- gdb/thread.c	27 Jul 2012 00:52:36 -0000	1.149
+++ gdb/thread.c	10 Dec 2012 18:13:27 -0000
@@ -1174,11 +1174,9 @@
    thread apply 1 2 7 4 backtrace       Apply backtrace cmd to threads 1,2,7,4
    thread apply 2-7 9 p foo(1)  Apply p foo(1) cmd to threads 2->7 & 9
    thread apply all p x/i $pc   Apply x/i $pc cmd to all threads.  */
-
 static void
 thread_apply_all_command (char *cmd, int from_tty)
 {
-  struct thread_info *tp;
   struct cleanup *old_chain;
   char *saved_cmd;
 
@@ -1193,18 +1191,30 @@
      execute_command.  */
   saved_cmd = xstrdup (cmd);
   make_cleanup (xfree, saved_cmd);
-  for (tp = thread_list; tp; tp = tp->next)
-    if (thread_alive (tp))
-      {
-	switch_to_thread (tp->ptid);
-
-	printf_filtered (_("\nThread %d (%s):\n"),
-			 tp->num, target_pid_to_str (inferior_ptid));
-	execute_command (cmd, from_tty);
-	strcpy (cmd, saved_cmd);	/* Restore exact command used
-					   previously.  */
-      }
 
+  if (thread_count ())
+    {
+      struct thread_info tp_array [thread_count ()];
+      struct thread_info *tp;
+      int i, k;
+
+      /* Save a copy of the thread_list in case we execute detach
+         command.  */
+      for (i = 0, tp = thread_list; tp; i++, tp = tp->next)
+        tp_array [i] = *tp;
+
+      for(k = 0; k != i; k++)
+        if (thread_alive (&tp_array [k]))
+          {
+            switch_to_thread ((&tp_array [k])->ptid);
+
+            printf_filtered (_("\nThread %d (%s):\n"),
+                             (&tp_array [k])->num, target_pid_to_str (inferior_ptid));
+            execute_command (cmd, from_tty);
+            strcpy (cmd, saved_cmd);        /* Restore exact command used
+                                               previously.  */
+           }
+    }
   do_cleanups (old_chain);
 }
Index: gdb/testsuite/gdb.threads/threadapply.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/threadapply.exp,v
retrieving revision 1.15
diff -u -r1.15 threadapply.exp
--- gdb/testsuite/gdb.threads/threadapply.exp	26 Jun 2012 19:23:20 -0000	1.15
+++ gdb/testsuite/gdb.threads/threadapply.exp	10 Dec 2012 18:13:33 -0000
@@ -63,3 +63,4 @@
 gdb_test "up" ".*in main.*" "go up in the stack frame" 
 gdb_test "thread apply all print 1"  "Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1" "run a simple print command on all threads"
 gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"
+gdb_test "thread apply all detach" "Detaching from.*" "detach all threads"