From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16171 invoked by alias); 10 Jul 2013 10:31:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 16156 invoked by uid 89); 10 Jul 2013 10:31:58 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_CP autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 10 Jul 2013 10:31:56 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1Uwrgf-0000Fm-QC from Ali_Anwar@mentor.com ; Wed, 10 Jul 2013 03:31:53 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Wed, 10 Jul 2013 03:31:53 -0700 Received: from [137.202.157.39] (147.34.91.1) by SVR-ORW-FEM-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server (TLS) id 14.2.247.3; Wed, 10 Jul 2013 03:31:52 -0700 Message-ID: <51DD37FA.8010306@codesourcery.com> Date: Wed, 10 Jul 2013 10:31:00 -0000 From: ali_anwar User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: Pedro Alves CC: Tom Tromey , Subject: Re: Updated patch for Bug 13217 - thread apply all detach throws a SEGFAULT References: <5062EA9B.1060505@codesourcery.com> <871uhnwkf3.fsf@fleche.redhat.com> <50C62BBB.6010404@codesourcery.com> <87hantek1p.fsf@fleche.redhat.com> <50C75319.9080903@codesourcery.com> <50C7628F.5080004@redhat.com> In-Reply-To: <50C7628F.5080004@redhat.com> Content-Type: multipart/mixed; boundary="------------080106050500000509060301" X-Virus-Found: No X-SW-Source: 2013-07/txt/msg00271.txt.bz2 --------------080106050500000509060301 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2192 On 12/11/2012 09:42 PM, Pedro Alves wrote: > On 12/11/2012 03:36 PM, ali_anwar wrote: >> + if (thread_count ()) >> + { >> + struct thread_info *tp_array; >> + struct thread_info *tp; >> + int i, k; >> + >> + /* Save a copy of the thread_list in case we execute detach >> + command. */ >> + tp_array = xmalloc (sizeof (struct thread_info) * thread_count ()); > > No need to compute the thread count twice, you can cache it. No need to > copy the whole thread structure. Make this an array of a thread > pointers, and then, > >> + for (i = 0, tp = thread_list; tp; i++, tp = tp->next) >> + tp_array[i] = *tp; > > ALL_THREADS (tp) > { > tp_array[i] = tp; > tp->refcount++; > } > > This increments the refcount of each current thread, so that attempts to > delete it just mark it as deleted (so the C object remains valid). > >> + >> + for (k = 0; k != i; k++) >> + if (thread_alive (&tp_array[k])) > > and then write: > > 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->num, target_pid_to_str (inferior_ptid)); > execute_command (cmd, from_tty); > strcpy (cmd, saved_cmd); /* Restore exact command used > previously. */ > } > } > > And put this in a cleanup: > > for (k = 0; k != i; k++) > tp_array[k]->refcount--; > > So that if the command throws an error, we still leave with the correct > refcounts. > > The advantages are: > > - less memory necessary for the array. > - handles the corner case of the target reusing a ptid (see > add_thread_silent). IOW, this way, even if the command happens to > make the target reuse a ptid, "thread apply all" won't run the command > on that new threads my mistake. > > I have tried to implement what you suggested in the attach patch. Does it look reasonable? Thanks, -Ali --------------080106050500000509060301 Content-Type: text/x-patch; name="Thread_apply_all.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Thread_apply_all.patch" Content-length: 3787 Index: gdb/thread.c =================================================================== RCS file: /cvs/src/src/gdb/thread.c,v retrieving revision 1.153 diff -u -r1.153 thread.c --- gdb/thread.c 11 Mar 2013 08:17:08 -0000 1.153 +++ gdb/thread.c 10 Jul 2013 09:51:08 -0000 @@ -65,6 +65,12 @@ static void restore_current_thread (ptid_t); static void prune_threads (void); +struct thread_array_cleanup { + struct thread_info **tp_array; + int count; +}; + + struct thread_info* inferior_thread (void) { @@ -1125,6 +1131,15 @@ xfree (old); } +static void +make_cleanup_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) { @@ -1176,13 +1191,13 @@ 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; + int tc; + struct thread_array_cleanup ta_cleanup; if (cmd == NULL || *cmd == '\000') error (_("Please specify a command following the thread ID list")); @@ -1195,17 +1210,41 @@ 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); + ta_cleanup.tp_array = tp_array; + ta_cleanup.count = tc; + + ALL_THREADS (tp) + { + tp_array[i] = tp; + tp->refcount++; + i++; + } + 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. */ + } + + make_cleanup (xfree, tp_array); + make_cleanup (make_cleanup_thread_refcount, &ta_cleanup); + } 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 .*" --------------080106050500000509060301--