From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24698 invoked by alias); 14 Apr 2017 22:40:40 -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 24518 invoked by uid 89); 14 Apr 2017 22:40:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=mua, stock, MUA X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Apr 2017 22:40:27 +0000 Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 53CAC10A7B9; Fri, 14 Apr 2017 18:40:27 -0400 (EDT) From: John Baldwin To: gdb-patches@sourceware.org, Luis Machado Subject: Re: [PATCH v2] PR threads/20743: Don't attempt to suspend or resume exited threads. Date: Fri, 14 Apr 2017 22:40:00 -0000 Message-ID: <6029590.iEtzOUCDnR@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: <62fcaaa0-35e3-2bed-fb5e-336a5c5ffbf4@codesourcery.com> References: <20170404173258.6512-1-jhb@FreeBSD.org> <62fcaaa0-35e3-2bed-fb5e-336a5c5ffbf4@codesourcery.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00470.txt.bz2 On Wednesday, April 12, 2017 01:11:45 PM Luis Machado wrote: > On 04/04/2017 12:32 PM, John Baldwin wrote: > > When resuming a native FreeBSD process, ignore exited threads when > > suspending/resuming individual threads prior to continuing the process. > > > > gdb/ChangeLog: > > > > PR threads/20743 > > * fbsd-nat.c (resume_one_thread_cb): Remove. > > (resume_all_threads_cb): Remove. > > (fbsd_resume): Use ALL_NON_EXITED_THREADS instead of > > iterate_over_threads. > ... > > @@ -711,13 +679,37 @@ fbsd_resume (struct target_ops *ops, > > if (ptid_lwp_p (ptid)) > > { > > /* If ptid is a specific LWP, suspend all other LWPs in the process. */ > > - iterate_over_threads (resume_one_thread_cb, &ptid); > > + struct thread_info *tp; > > + int request; > > + > > + ALL_NON_EXITED_THREADS (tp) > > + { > > + if (ptid_get_pid (tp->ptid) != ptid_get_pid (ptid)) > > + continue; > > + > > + if (ptid_get_lwp (tp->ptid) == ptid_get_lwp (ptid)) > > + request = PT_RESUME; > > + else > > + request = PT_SUSPEND; > > + > > + if (ptrace (request, ptid_get_lwp (tp->ptid), NULL, 0) == -1) > > + perror_with_name (("ptrace")); > > + } > > Identation of the ALL_NON_EXITED_THREADS block is off. I'd check the > identation of the entire block to make sure it is sane. Hmm, the raw code looks fine. I know that my MUA (kmail) messes up formatting of code as it displays tabs as 4 characters instead of 8? Here's the raw code with tabs expanded to spaces: ALL_NON_EXITED_THREADS (tp) { if (ptid_get_pid (tp->ptid) != ptid_get_pid (ptid)) continue; if (ptid_get_lwp (tp->ptid) == ptid_get_lwp (ptid)) request = PT_RESUME; else request = PT_SUSPEND; if (ptrace (request, ptid_get_lwp (tp->ptid), NULL, 0) == -1) perror_with_name (("ptrace")); } > A question i have is why did we have to remove the original functions. > Couldn't we have checked the non-exited-ness of the threads inside the > callback? That was what the V1 patch did, but you and Pedro requested it use ALL_NON_EXITED_THREADS instead, hence version 2. > Another bit... Since we're changing this code, might as well improve the > perror message so it is more meaningful? I could perhaps do a followup to include the ptrace op in the various perror's in this file (all of them use this, as do the various BSD nat.c files used for register fetch/store). > Otherwise i have no further comments. I assume you ran gdb's testsuite > against this change and verified the results are sane? There were no regressions at least. With the stock tree there are several unexpected failures already which I will get to at some point. -- John Baldwin