From patchwork Thu Mar 19 12:49:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 5698 Received: (qmail 35824 invoked by alias); 19 Mar 2015 12:49:48 -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 35811 invoked by uid 89); 19 Mar 2015 12:49:47 -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; Thu, 19 Mar 2015 12:49:46 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2JCnjaR004748 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 19 Mar 2015 08:49:45 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2JCnh8Z009117 for ; Thu, 19 Mar 2015 08:49:44 -0400 Message-ID: <550AC5E7.5030200@redhat.com> Date: Thu, 19 Mar 2015 12:49:43 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: [pushed] gdbserver/Linux: unbreak thread event randomization (Re: [pushed] native/Linux: internal error if resume is short-circuited) References: <1425671886-7798-1-git-send-email-palves@redhat.com> <1425671886-7798-5-git-send-email-palves@redhat.com> <550AC2EB.10801@redhat.com> In-Reply-To: <550AC2EB.10801@redhat.com> On 03/19/2015 12:36 PM, Pedro Alves wrote: > So I thought some more about this and realized that it's > possible to construct a test case that triggers the assertion, > even without the special-case of a process that disappears. So I > wrote that test, rewrote the git commit log in that direction, > and pushed it in, as below. When I first wrote that patch, I added an extra assert that surprisingly failed against gdbserver. It caught a bug straight from the OMG-can't-believe-this-wasnt-caught-before department... I've pushed in the gdbserver fix and the extra test assert, as below. I've made the test dump the counters to the logs, so I can keep an eye on the buildbots, to make sure the test isn't at real risk of being racy in practice. --- From 8bf3b159e55b42bb084f9da1af400a285025618f Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sun, 15 Mar 2015 19:35:26 +0000 Subject: [PATCH] gdbserver/Linux: unbreak thread event randomization Wanting to make sure the new continue-pending-status.exp test tests both cases of threads 2 and 3 reporting an event, I added counters to the test, to make it FAIL if events for both threads aren't seen. Assuming a well behaved backend, and given a reasonable number of iterations, it should PASS. However, running that against GNU/Linux gdbserver, I found that surprisingly, that FAILed. GDBserver always reported the breakpoint hit for the same thread. Turns out that I broke gdbserver's thread event randomization recently, with git commit 582511be ([gdbserver] linux-low.c: better starvation avoidance, handle non-stop mode too). In that commit I missed that the thread structure also has a status_pending_p field... The end result was that count_events_callback always returns 0, and then if no thread is stepping, select_event_lwp always returns the event thread. IOW, no randomization is happening at all. Quite curious how all the other changes in that patch were sufficient to fix non-stop-fair-events.exp anyway even with that broken. Tested on x86_64 Fedora 20, native and gdbserver. gdb/gdbserver/ChangeLog: 2015-03-19 Pedro Alves * linux-low.c (count_events_callback, select_event_lwp_callback): Use the lwp's status_pending_p field, not the thread's. gdb/testsuite/ChangeLog: 2015-03-19 Pedro Alves * gdb.threads/continue-pending-status.exp (saw_thread_2) (saw_thread_3): New globals. (top level): Increment them when an event for the corresponding thread is seen. (no thread starvation): New test. --- gdb/ChangeLog | 5 +++++ gdb/gdbserver/ChangeLog | 5 +++++ gdb/testsuite/ChangeLog | 8 ++++++++ gdb/gdbserver/linux-low.c | 7 +++++-- gdb/linux-nat.c | 1 + gdb/testsuite/gdb.threads/continue-pending-status.exp | 14 ++++++++++++++ 6 files changed, 38 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7ae3c58..cfbd576 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2015-03-19 Pedro Alves + + * linux-low.c (count_events_callback, select_event_lwp_callback): + Use the lwp's status_pending_p field, not the thread's. + 2015-03-19 Pedro Alves * linux-nat.c (status_callback): Return early if the LWP has no diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 6efab5b..df8f9ab 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2015-03-19 Pedro Alves + + * linux-low.c (count_events_callback, select_event_lwp_callback): + Use the lwp's status_pending_p field, not the thread's. + 2015-03-19 Pedro Alves * linux-low.c (select_event_lwp_callback): Update comments to diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 94dae82..6bf008a 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,13 @@ 2015-03-19 Pedro Alves + * gdb.threads/continue-pending-status.exp (saw_thread_2) + (saw_thread_3): New globals. + (top level): Increment them when an event for the corresponding + thread is seen. + (no thread starvation): New test. + +2015-03-19 Pedro Alves + * gdb.threads/continue-pending-status.c: New file. * gdb.threads/continue-pending-status.exp: New file. diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 9558f46..e53e0fc 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -2238,6 +2238,7 @@ static int count_events_callback (struct inferior_list_entry *entry, void *data) { struct thread_info *thread = (struct thread_info *) entry; + struct lwp_info *lp = get_thread_lwp (thread); int *count = data; gdb_assert (count != NULL); @@ -2245,7 +2246,7 @@ count_events_callback (struct inferior_list_entry *entry, void *data) /* Count only resumed LWPs that have an event pending. */ if (thread->last_status.kind == TARGET_WAITKIND_IGNORE && thread->last_resume_kind != resume_stop - && thread->status_pending_p) + && lp->status_pending_p) (*count)++; return 0; @@ -2273,6 +2274,7 @@ static int select_event_lwp_callback (struct inferior_list_entry *entry, void *data) { struct thread_info *thread = (struct thread_info *) entry; + struct lwp_info *lp = get_thread_lwp (thread); int *selector = data; gdb_assert (selector != NULL); @@ -2280,7 +2282,7 @@ select_event_lwp_callback (struct inferior_list_entry *entry, void *data) /* Select only resumed LWPs that have an event pending. */ if (thread->last_resume_kind != resume_stop && thread->last_status.kind == TARGET_WAITKIND_IGNORE - && thread->status_pending_p) + && lp->status_pending_p) if ((*selector)-- == 0) return 1; @@ -2324,6 +2326,7 @@ select_event_lwp (struct lwp_info **orig_lp) /* First see how many events we have. */ find_inferior (&all_threads, count_events_callback, &num_events); + gdb_assert (num_events > 0); /* Now randomly pick a LWP out of those that have had events. */ diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index f5f92d9..40f1e1f 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2841,6 +2841,7 @@ select_event_lwp (ptid_t filter, struct lwp_info **orig_lp, int *status) /* First see how many events we have. */ iterate_over_lwps (filter, count_events_callback, &num_events); + gdb_assert (num_events > 0); /* Now randomly pick a LWP out of those that have had events. */ diff --git a/gdb/testsuite/gdb.threads/continue-pending-status.exp b/gdb/testsuite/gdb.threads/continue-pending-status.exp index ff73ce4..1f170f7 100644 --- a/gdb/testsuite/gdb.threads/continue-pending-status.exp +++ b/gdb/testsuite/gdb.threads/continue-pending-status.exp @@ -56,6 +56,13 @@ proc get_current_thread {} { set attempts 20 +# These track whether we saw events for both threads 2 and 3. If the +# backend always returns the breakpoint hit for the same thread, then +# it fails to make sure threads aren't starved, and we'll fail the +# assert after the loop. +set saw_thread_2 0 +set saw_thread_3 0 + for {set i 0} {$i < $attempts} {incr i} { with_test_prefix "attempt $i" { gdb_test "b $srcfile:$break_line" \ @@ -71,8 +78,10 @@ for {set i 0} {$i < $attempts} {incr i} { # the resume and go straight to consuming the pending event. set thread [get_current_thread] if {$thread == 2} { + incr saw_thread_2 set thread 3 } else { + incr saw_thread_3 set thread 2 } gdb_test "thread $thread" \ @@ -108,3 +117,8 @@ for {set i 0} {$i < $attempts} {incr i} { } } } + +verbose -log "saw_thread_2=$saw_thread_2" +verbose -log "saw_thread_3=$saw_thread_3" + +gdb_assert {$saw_thread_2 > 0 && $saw_thread_3 > 0} "no thread starvation"