From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24906 invoked by alias); 13 Nov 2014 13:00:25 -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 24890 invoked by uid 89); 13 Nov 2014 13:00:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,UNSUBSCRIBE_BODY autolearn=no 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, 13 Nov 2014 13:00:22 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sADD0JEp009182 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 13 Nov 2014 08:00:19 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sADD0Ioc008913; Thu, 13 Nov 2014 08:00:19 -0500 Message-ID: <5464AB62.5040100@redhat.com> Date: Thu, 13 Nov 2014 13:00:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Don Breazeal , gdb-patches@sourceware.org Subject: Re: [PATCH 06/16 v3] Extended-remote Linux follow fork References: <1408580964-27916-1-git-send-email-donb@codesourcery.com> <1414798134-11536-4-git-send-email-donb@codesourcery.com> In-Reply-To: <1414798134-11536-4-git-send-email-donb@codesourcery.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-11/txt/msg00257.txt.bz2 On 10/31/2014 11:28 PM, Don Breazeal wrote: > This patch implements basic support for follow-fork and detach-on-fork on > extended-remote Linux targets. Only 'fork' is supported in this patch; > 'vfork' support is added n a subsequent patch. Sufficient extended-remote > functionality has been implemented here to pass gdb.base/foll-fork.exp with > the catchpoint tests commented out. > Thanks. Let's try to shake out the higher-level concepts first before focusing on specific details. > > * implementing a new RSP 'T' Stop Reply Packet stop reason: "fork", in > gdbserver/remote-utils.c and remote.c. "f" is an hexadecimal digit. Last time we added such a packet, "core", we broke backwards compatibility with older GDBs... :-/ We need to careful with that. > > * implementing new target and RSP support for target_follow_fork with > target extended-remote. (The RSP components were actually defined in > patch 4, but they see their first use here). > > - extended_remote target routine extended_remote_follow_fork > > - RSP packet vFollowFork Reading through this, I don't think this is the model we should be exposing at the RSP level, and requiring servers to support. The hiding of the child fork until the users resumes is a current detail that we may want to change in the future. It seems better to me to _not_ hide the child from GDB, and then implement the hide-child-until-resume detail in GDB. That is, I think we should model fork events at the RSP level similarly to how ptrace and ttrace expose them. So, e.g., I think switching to the child to write to its memory should be done with the regular Hg packet. Handling detach_fork would be done by GDB calling the regular detach packet (D;PID), etc. I'm not even seeing a fundamental need to keep this for "extended-remote" alone, given gdb nowadays supports the multiprocess extensions with "target remote" too. Also, I don't see how this packet could work correctly with non-stop mode. You're assuming only one thread/process has stopped for a fork. > else > { > /* Silently skip unknown optional info. */ > @@ -9418,8 +9447,11 @@ remote_pid_to_str (struct target_ops *ops, ptid_t ptid) > if (ptid_equal (magic_null_ptid, ptid)) > xsnprintf (buf, sizeof buf, "Thread
"); > else if (rs->extended && remote_multi_process_p (rs)) > - xsnprintf (buf, sizeof buf, "Thread %d.%ld", > - ptid_get_pid (ptid), ptid_get_lwp (ptid)); > + if (ptid_get_lwp (ptid) == 0) > + return normal_pid_to_str (ptid); Can you explain this bit? Thanks, Pedro Alves