From patchwork Wed Jan 7 13:22:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 4543 Received: (qmail 31226 invoked by alias); 7 Jan 2015 13:22:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 31123 invoked by uid 89); 7 Jan 2015 13:22:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 07 Jan 2015 13:22:19 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t07DMFgA000775 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 7 Jan 2015 08:22:15 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t07DMC4L007997; Wed, 7 Jan 2015 08:22:14 -0500 Message-ID: <54AD3303.8000801@redhat.com> Date: Wed, 07 Jan 2015 13:22:11 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 4/8] linux-nat.c: clean up pending status checking and resuming LWPs References: <1419625871-28848-1-git-send-email-palves@redhat.com> <1419625871-28848-5-git-send-email-palves@redhat.com> <87a91wuy2p.fsf@codesourcery.com> In-Reply-To: <87a91wuy2p.fsf@codesourcery.com> On 01/06/2015 08:12 AM, Yao Qi wrote: > Pedro Alves writes: > >> Whenever we resume an LWP, we must clear a few flags and flush the >> LWPs register cache. Instead of open coding that in many places, use >> an helper function. > > After this patch, the behavior is changed to flush the LWP's register > cache rather than register cache of all LWPs (registers_changed -> > registers_changed_ptid (lp->ptid)), right? Although I believe the > change is correct, we should mention it in the commit log. Good point. Here's the updated version. Nothing else changed. ----- From 645d6471aad217309c750a28d9ab08468acaee38 Mon Sep 17 00:00:00 2001 From: Pedro Alves Subject: [PATCH] linux-nat.c: clean up pending status checking and resuming LWPs Whenever we resume an LWP, we must clear a few flags and flush the LWP's register cache. We actually currently flush the register cache of all LWPs, but that's unnecessary. This patch makes us flush the register cache of only the LWP that is resumed. Instead of open coding all that in many places, we use a helper function. Likewise, we have two fields in the LWP structure where a pending status may be recorded. Add a helper predicate that checks both and use it throughout instead of open coding the checks. gdb/ 2015-01-07 Pedro Alves * linux-nat.c (linux_resume_one_lwp): New function. (resume_lwp): Use lwp_status_pending_p and linux_resume_one_lwp. (linux_nat_resume): Use lwp_status_pending_p and linux_resume_one_lwp. (linux_handle_syscall_trap): Use linux_resume_one_lwp. (linux_handle_extended_wait): Use linux_resume_one_lwp. (status_callback, running_callback): Use lwp_status_pending_p. (lwp_status_pending_p): New function. (stop_and_resume_callback): Use lwp_status_pending_p. (linux_nat_filter_event): Use linux_resume_one_lwp. (linux_nat_wait_1): Always use status_callback to look for an LWP with a pending status. Use linux_resume_one_lwp. (resume_stopped_resumed_lwps): Use lwp_status_pending_p and linux_resume_one_lwp. --- gdb/linux-nat.c | 170 +++++++++++++++++--------------------------------------- 1 file changed, 50 insertions(+), 120 deletions(-) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 5e60145..de9aec5 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -277,6 +277,8 @@ static void purge_lwp_list (int pid); static void delete_lwp (ptid_t ptid); static struct lwp_info *find_lwp_pid (ptid_t ptid); +static int lwp_status_pending_p (struct lwp_info *lp); + /* Trivial list manipulation functions to keep track of a list of new stopped processes. */ @@ -1453,6 +1455,25 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty) linux_ops->to_detach (ops, args, from_tty); } +/* Resume execution of the inferior process. If STEP is nonzero, + single-step it. If SIGNAL is nonzero, give it that signal. */ + +static void +linux_resume_one_lwp (struct lwp_info *lp, int step, enum gdb_signal signo) +{ + ptid_t ptid; + + lp->step = step; + if (linux_nat_prepare_to_resume != NULL) + linux_nat_prepare_to_resume (lp); + /* Convert to something the lower layer understands. */ + ptid = pid_to_ptid (ptid_get_lwp (lp->ptid)); + linux_ops->to_resume (linux_ops, ptid, step, signo); + lp->stopped_by_watchpoint = 0; + lp->stopped = 0; + registers_changed_ptid (lp->ptid); +} + /* Resume LP. */ static void @@ -1469,8 +1490,7 @@ resume_lwp (struct lwp_info *lp, int step, enum gdb_signal signo) "RC: Not resuming %s (vfork parent)\n", target_pid_to_str (lp->ptid)); } - else if (lp->status == 0 - && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE) + else if (!lwp_status_pending_p (lp)) { if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, @@ -1481,14 +1501,7 @@ resume_lwp (struct lwp_info *lp, int step, enum gdb_signal signo) : "0"), step ? "step" : "resume"); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, - pid_to_ptid (ptid_get_lwp (lp->ptid)), - step, signo); - lp->stopped = 0; - lp->step = step; - lp->stopped_by_watchpoint = 0; + linux_resume_one_lwp (lp, step, signo); } else { @@ -1582,7 +1595,6 @@ linux_nat_resume (struct target_ops *ops, gdb_assert (lp != NULL); /* Remember if we're stepping. */ - lp->step = step; lp->last_resume_kind = step ? resume_step : resume_continue; /* If we have a pending wait status for this thread, there is no @@ -1612,7 +1624,7 @@ linux_nat_resume (struct target_ops *ops, } } - if (lp->status || lp->waitstatus.kind != TARGET_WAITKIND_IGNORE) + if (lwp_status_pending_p (lp)) { /* FIXME: What should we do if we are supposed to continue this thread with a signal? */ @@ -1635,14 +1647,7 @@ linux_nat_resume (struct target_ops *ops, if (resume_many) iterate_over_lwps (ptid, linux_nat_resume_callback, lp); - /* Convert to something the lower layer understands. */ - ptid = pid_to_ptid (ptid_get_lwp (lp->ptid)); - - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, ptid, step, signo); - lp->stopped_by_watchpoint = 0; - lp->stopped = 0; + linux_resume_one_lwp (lp, step, signo); if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, @@ -1806,14 +1811,7 @@ linux_handle_syscall_trap (struct lwp_info *lp, int stopping) subsequent syscall. Simply resume using the inf-ptrace layer, which knows when to use PT_SYSCALL or PT_CONTINUE. */ - /* Note that gdbarch_get_syscall_number may access registers, hence - fill a regcache. */ - registers_changed (); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, pid_to_ptid (ptid_get_lwp (lp->ptid)), - lp->step, GDB_SIGNAL_0); - lp->stopped = 0; + linux_resume_one_lwp (lp, lp->step, GDB_SIGNAL_0); return 1; } @@ -2007,23 +2005,14 @@ linux_handle_extended_wait (struct lwp_info *lp, int status, fprintf_unfiltered (gdb_stdlog, "LHEW: resuming new LWP %ld\n", ptid_get_lwp (new_lp->ptid)); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (new_lp); - linux_ops->to_resume (linux_ops, pid_to_ptid (new_pid), - 0, GDB_SIGNAL_0); - new_lp->stopped = 0; + linux_resume_one_lwp (new_lp, 0, GDB_SIGNAL_0); } } if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LHEW: resuming parent LWP %d\n", pid); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, - pid_to_ptid (ptid_get_lwp (lp->ptid)), - 0, GDB_SIGNAL_0); - lp->stopped = 0; + linux_resume_one_lwp (lp, 0, GDB_SIGNAL_0); return 1; } @@ -2493,20 +2482,7 @@ status_callback (struct lwp_info *lp, void *data) if (!lp->resumed) return 0; - if (lp->waitstatus.kind != TARGET_WAITKIND_IGNORE) - { - /* A ptrace event, like PTRACE_FORK|VFORK|EXEC, syscall event, - or a pending process exit. Note that `W_EXITCODE(0,0) == - 0', so a clean process exit can not be stored pending in - lp->status, it is indistinguishable from - no-pending-status. */ - return 1; - } - - if (lp->status != 0) - return 1; - - return 0; + return lwp_status_pending_p (lp); } /* Return non-zero if LP isn't stopped. */ @@ -2515,9 +2491,7 @@ static int running_callback (struct lwp_info *lp, void *data) { return (!lp->stopped - || ((lp->status != 0 - || lp->waitstatus.kind != TARGET_WAITKIND_IGNORE) - && lp->resumed)); + || (lwp_status_pending_p (lp) && lp->resumed)); } /* Count the LWP's that have had events. */ @@ -2548,6 +2522,17 @@ select_singlestep_lwp_callback (struct lwp_info *lp, void *data) return 0; } +/* Returns true if LP has a status pending. */ + +static int +lwp_status_pending_p (struct lwp_info *lp) +{ + /* We check for lp->waitstatus in addition to lp->status, because we + can have pending process exits recorded in lp->status and + W_EXITCODE(0,0) happens to be 0. */ + return lp->status != 0 || lp->waitstatus.kind != TARGET_WAITKIND_IGNORE; +} + /* Select the Nth LWP that has had a SIGTRAP event. */ static int @@ -2711,7 +2696,7 @@ stop_and_resume_callback (struct lwp_info *lp, void *data) if (lp != NULL) { if (lp->last_resume_kind == resume_stop - && lp->status == 0) + && !lwp_status_pending_p (lp)) { /* The core wanted the LWP to stop. Even if it stopped cleanly (with SIGSTOP), leave the event pending. */ @@ -2723,7 +2708,7 @@ stop_and_resume_callback (struct lwp_info *lp, void *data) lp->status = W_STOPCODE (SIGSTOP); } - if (lp->status == 0) + if (!lwp_status_pending_p (lp)) { if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, @@ -2910,13 +2895,7 @@ linux_nat_filter_event (int lwpid, int status, int *new_pending_p) { /* This is a delayed SIGSTOP. */ - registers_changed (); - - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, - pid_to_ptid (ptid_get_lwp (lp->ptid)), - lp->step, GDB_SIGNAL_0); + linux_resume_one_lwp (lp, lp->step, GDB_SIGNAL_0); if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LLW: %s %s, 0, 0 (discard SIGSTOP)\n", @@ -2924,7 +2903,6 @@ linux_nat_filter_event (int lwpid, int status, int *new_pending_p) "PTRACE_SINGLESTEP" : "PTRACE_CONT", target_pid_to_str (lp->ptid)); - lp->stopped = 0; gdb_assert (lp->resumed); /* Discard the event. */ @@ -2945,19 +2923,13 @@ linux_nat_filter_event (int lwpid, int status, int *new_pending_p) /* This is a delayed SIGINT. */ lp->ignore_sigint = 0; - registers_changed (); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, pid_to_ptid (ptid_get_lwp (lp->ptid)), - lp->step, GDB_SIGNAL_0); + linux_resume_one_lwp (lp, lp->step, GDB_SIGNAL_0); if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LLW: %s %s, 0, 0 (discard SIGINT)\n", lp->step ? "PTRACE_SINGLESTEP" : "PTRACE_CONT", target_pid_to_str (lp->ptid)); - - lp->stopped = 0; gdb_assert (lp->resumed); /* Discard the event. */ @@ -3066,46 +3038,17 @@ linux_nat_wait_1 (struct target_ops *ops, block_child_signals (&prev_mask); retry: - lp = NULL; status = 0; /* First check if there is a LWP with a wait status pending. */ - if (ptid_equal (ptid, minus_one_ptid) || ptid_is_pid (ptid)) - { - /* Any LWP in the PTID group that's been resumed will do. */ - lp = iterate_over_lwps (ptid, status_callback, NULL); - if (lp) - { - if (debug_linux_nat && lp->status) - fprintf_unfiltered (gdb_stdlog, - "LLW: Using pending wait status %s for %s.\n", - status_to_str (lp->status), - target_pid_to_str (lp->ptid)); - } - } - else if (ptid_lwp_p (ptid)) + lp = iterate_over_lwps (ptid, status_callback, NULL); + if (lp != NULL) { if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, - "LLW: Waiting for specific LWP %s.\n", - target_pid_to_str (ptid)); - - /* We have a specific LWP to check. */ - lp = find_lwp_pid (ptid); - gdb_assert (lp); - - if (debug_linux_nat && lp->status) - fprintf_unfiltered (gdb_stdlog, "LLW: Using pending wait status %s for %s.\n", status_to_str (lp->status), target_pid_to_str (lp->ptid)); - - /* We check for lp->waitstatus in addition to lp->status, - because we can have pending process exits recorded in - lp->status and W_EXITCODE(0,0) == 0. We should probably have - an additional lp->status_p flag. */ - if (lp->status == 0 && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE) - lp = NULL; } if (!target_can_async_p ()) @@ -3321,12 +3264,7 @@ retry: other threads to run. On the other hand, not resuming newly attached threads may cause an unwanted delay in getting them running. */ - registers_changed (); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, - pid_to_ptid (ptid_get_lwp (lp->ptid)), - lp->step, signo); + linux_resume_one_lwp (lp, lp->step, signo); if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LLW: %s %s, %s (preempt 'handle')\n", @@ -3336,7 +3274,6 @@ retry: (signo != GDB_SIGNAL_0 ? strsignal (gdb_signal_to_host (signo)) : "0")); - lp->stopped = 0; goto retry; } @@ -3456,8 +3393,7 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, void *data) if (lp->stopped && lp->resumed - && lp->status == 0 - && lp->waitstatus.kind == TARGET_WAITKIND_IGNORE) + && !lwp_status_pending_p (lp)) { struct regcache *regcache = get_thread_regcache (lp->ptid); struct gdbarch *gdbarch = get_regcache_arch (regcache); @@ -3480,13 +3416,7 @@ resume_stopped_resumed_lwps (struct lwp_info *lp, void *data) paddress (gdbarch, pc), lp->step); - registers_changed (); - if (linux_nat_prepare_to_resume != NULL) - linux_nat_prepare_to_resume (lp); - linux_ops->to_resume (linux_ops, pid_to_ptid (ptid_get_lwp (lp->ptid)), - lp->step, GDB_SIGNAL_0); - lp->stopped = 0; - lp->stopped_by_watchpoint = 0; + linux_resume_one_lwp (lp, lp->step, GDB_SIGNAL_0); } return 0;