From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9516 invoked by alias); 11 Dec 2012 16:43:13 -0000 Received: (qmail 9504 invoked by uid 22791); 11 Dec 2012 16:43:11 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 11 Dec 2012 16:42:59 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBBGgwTK011912 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 11 Dec 2012 11:42:58 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qBBGguC3001326; Tue, 11 Dec 2012 11:42:57 -0500 Message-ID: <50C7628F.5080004@redhat.com> Date: Tue, 11 Dec 2012 16:43:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: ali_anwar CC: Tom Tromey , gdb-patches@sourceware.org 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> In-Reply-To: <50C75319.9080903@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-12/txt/msg00345.txt.bz2 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. > + { > + 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. */ > + } > + xfree (tp_array); This xfree should be on the cleanup as well. > + } > do_cleanups (old_chain); -- Pedro Alves