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

[ali_anwar <ali_anwar@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On 07/10/2013 10:55 PM, Tom Tromey wrote:
>>>>>> "Ali" == ali anwar <ali_anwar@codesourcery.com> writes:
>
>
> This installs the cleanups at the wrong point.
> They must be installed before any potential exception is thrown.
>
> That is, the xfree cleanup ought to be created just after initialization;
> and the refcount cleanup ought to be created after the refcounts are
> incremented.
>
> Also, make_cleanup_thread_refcount is misnamed.
> By convention, a "make_cleanup_" function creates a new cleanup.
> The function that does the work of cleaning up is given a different name.
>

Thanks for the Review.

Please find attached updated patch.

-Ali


[-- Attachment #2: PR13217.patch --]
[-- Type: text/x-patch, Size: 4334 bytes --]

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.15797
diff -u -r1.15797 ChangeLog
--- gdb/ChangeLog	11 Jul 2013 09:07:41 -0000	1.15797
+++ gdb/ChangeLog	11 Jul 2013 12:11:08 -0000
@@ -1,3 +1,11 @@
+2013-07-11  Ali Anwar  <ali_anwar@codesourcery.com>
+
+	PR threads/13217
+	* thread.c (thread_apply_all_command): Check for valid threads
+	and thread count.
+	(thread_array_cleanup): New struct.
+	(set_thread_refcount): New function.
+
 2013-07-11  Andrew Burgess  <aburgess@broadcom.com>
 
 	* infcmd.c (default_print_one_register_info): Reuse function
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.154
diff -u -r1.154 thread.c
--- gdb/thread.c	23 May 2013 17:12:51 -0000	1.154
+++ gdb/thread.c	11 Jul 2013 12:18:00 -0000
@@ -65,6 +65,19 @@
 static void restore_current_thread (ptid_t);
 static void prune_threads (void);
 
+/* Data to cleanup thread array.  */
+
+struct thread_array_cleanup
+{
+  /* Array of thread pointers used to set
+     reference count.  */
+  struct thread_info **tp_array;
+
+  /* Thread count in the array.  */
+  int count;
+};
+
+
 struct thread_info*
 inferior_thread (void)
 {
@@ -1132,6 +1145,18 @@
   xfree (old);
 }
 
+/* Set the thread reference count.  */
+
+static void
+set_thread_refcount (void *data)
+{
+  int k;
+  struct thread_array_cleanup *ta_cleanup = data;
+
+  for (k = 0; k != ta_cleanup->count; k++)
+    ta_cleanup->tp_array[k]->refcount--;
+}
+
 struct cleanup *
 make_cleanup_restore_current_thread (void)
 {
@@ -1187,9 +1212,10 @@
 static void
 thread_apply_all_command (char *cmd, int from_tty)
 {
-  struct thread_info *tp;
   struct cleanup *old_chain;
   char *saved_cmd;
+  int tc;
+  struct thread_array_cleanup ta_cleanup;
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
@@ -1202,17 +1228,44 @@
      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);
+  tc = thread_count ();
 
-	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 (tc)
+    {
+      struct thread_info **tp_array;
+      struct thread_info *tp;
+      int i, k;
+      i = 0;
+
+      /* Save a copy of the thread_list in case we execute detach
+         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_THREADS (tp)
+        {
+          tp_array[i] = tp;
+          tp->refcount++;
+          i++;
+        }
+
+      make_cleanup (set_thread_refcount, &ta_cleanup);
+
+      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);
+
+            /* Restore exact command used previously.  */
+            strcpy (cmd, saved_cmd);
+	  }
+    }
 
   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.16
diff -u -r1.16 threadapply.exp
--- gdb/testsuite/gdb.threads/threadapply.exp	1 Jan 2013 06:41:27 -0000	1.16
+++ gdb/testsuite/gdb.threads/threadapply.exp	10 Jul 2013 09:51:08 -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" "Thread .*"