From patchwork Mon Feb 1 20:38:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 10691 Received: (qmail 48388 invoked by alias); 1 Feb 2016 20:38:07 -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 48372 invoked by uid 89); 1 Feb 2016 20:38:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=(unknown), overwrites, 1006, detective 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; Mon, 01 Feb 2016 20:38:05 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 234FF5A43; Mon, 1 Feb 2016 20:38:03 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u11Kc2nS000335; Mon, 1 Feb 2016 15:38:03 -0500 Message-ID: <56AFC22A.8000007@redhat.com> Date: Mon, 01 Feb 2016 20:38:02 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Don Breazeal , gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] PR remote/19496, internal err forking-threads-plus-bkpt References: <1453942111-1215-1-git-send-email-donb@codesourcery.com> <1453942111-1215-2-git-send-email-donb@codesourcery.com> In-Reply-To: <1453942111-1215-2-git-send-email-donb@codesourcery.com> On 01/28/2016 12:48 AM, Don Breazeal wrote: > This patch fixes an internal error that occurs in > gdb.threads/forking-threads-plus-breakpoint.exp: > > /blah/binutils-gdb/gdb/target.c:2723: internal-error: Can't determine the > current address space of thread Thread 3170.3170 > > In default_thread_address_space, find_inferior_ptid couldn't find 3170.3170 > because it had been overwritten in inferior_appeared, called as follows: > > inferior_appeared > remote_add_inferior > remote_notice_new_inferior > remote_update_thread_list > > The cause of the problem was the following sequence of events: > > * GDB knows only about the main thread > > * the first fork event is reported to GDB, saved as pending_event > > * qXfer:threads_read gets the threads from the remote. > remove_new_fork_children id's the fork child from the pending event > and removes it from the list reported to GDB. All the rest of the > threads, including the fork parent, are added to the GDB thread list. > > * GDB stops all the threads. All the stop events are pushed onto the > stop reply queue behind the pending fork event. > > * remote_wait_ns calls queued_stop_reply and process_stop_reply to > remove the fork event from the front of the stop reply queue and save > event information in the thread_info structure for the fork parent > thread. Unfortunately, none of the information saved in this way is > the fork-specific information, so the actual fork event info is lost. > > * A subsequent qXfer:threads:read packet gets the thread list including > the fork parent and fork child. remove_new_fork_children checks the > thread list to see if there is a fork parent, doesn't find one, checks > the stop reply queue for a pending fork event, doesn't find one, and > allows the fork child thread to be reported to GDB before the fork > event has been handled. remote_update_thread_list calls > remote_notice_new_thread and overwrites the current (main) thread in > inferior_appeared. GDB has now lost all knowledge of the main thread, > and an internal error results. > > The fix was to make sure that when the stop reply was removed from the > stop reply queuei, all of the necessary fork event information was stored > in the parent thread structure. In process_stop_reply we call a new > function, update_thread_if_fork_parent, to store the pending_follow > information from the fork stop reply in the fork parent thread. > > Tested on x86_64 and Nios II Linux. No regressions, but more failures, > which are addressed in subsequent patches in this patchset. > Many thanks for the detective work! > +static void > +update_thread_if_fork_parent (struct stop_reply *stop_reply) > +{ > + ptid_t ptid; > + > + ptid = stop_reply->ptid; > + if (stop_reply->ws.kind == TARGET_WAITKIND_FORKED > + || stop_reply->ws.kind == TARGET_WAITKIND_VFORKED) > + { > + struct thread_info *tp = find_thread_ptid (ptid); > + > + tp->pending_follow = stop_reply->ws; > + } > +} So the fork event has been reported out of target_wait but it was left pending on the infrun side (infrun.c:save_waitstatus) IOW, the fork event hasn't been processed by handle_inferior_event yet, so it hasn't made it to tp->pending_follow yet. The information is not lost, we're just looking for it in the wrong place. I think this would be the right fix: From 211e4553a500f7a81d11860f9707db97b0a53c45 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 1 Feb 2016 20:25:00 +0000 Subject: [PATCH] Fix PR remote/19496, internal error --- gdb/remote.c | 7 ++++++- gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp | 6 ------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 8831b50..5dffb98 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6180,7 +6180,12 @@ remove_new_fork_children (struct threads_listing_context *context) fork child threads from the CONTEXT list. */ ALL_NON_EXITED_THREADS (thread) { - struct target_waitstatus *ws = &thread->pending_follow; + struct target_waitstatus *ws; + + if (thread->suspend.waitstatus_pending_p) + ws = &thread->suspend.waitstatus; + else + ws = &thread->pending_follow; if (is_pending_fork_parent (ws, pid, thread->ptid)) { diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp index 6c72061..ff3ca9a 100644 --- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp +++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp @@ -100,12 +100,6 @@ proc do_test { cond_bp_target detach_on_fork displaced } { set fork_count 0 set ok 0 - if {$displaced == "off" - && [target_info exists gdb_protocol] - && ([target_info gdb_protocol] == "remote" - || [target_info gdb_protocol] == "extended-remote")} { - setup_kfail "remote/19496" *-*-* - } set test "inferior 1 exited" gdb_test_multiple "" $test { -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {