From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114039 invoked by alias); 19 Jan 2018 03:17:43 -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 114025 invoked by uid 89); 19 Jan 2018 03:17:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Jan 2018 03:17:40 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w0J3HYS9005309 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 18 Jan 2018 22:17:38 -0500 Received: by simark.ca (Postfix, from userid 112) id 2E85A1E5B7; Thu, 18 Jan 2018 22:17:34 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id BC9C71E074; Thu, 18 Jan 2018 22:17:32 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 19 Jan 2018 03:17:00 -0000 From: Simon Marchi To: Pedro Alves Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] Pass inferior down to target_detach and to_detach In-Reply-To: <71c999db-f10c-ca2b-911e-4d6968121a3f@redhat.com> References: <1514699454-18587-1-git-send-email-simon.marchi@ericsson.com> <1514699454-18587-2-git-send-email-simon.marchi@ericsson.com> <71c999db-f10c-ca2b-911e-4d6968121a3f@redhat.com> Message-ID: <9c3d2551c577a90566d9ac83f0409988@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.2 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 19 Jan 2018 03:17:34 +0000 X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00385.txt.bz2 On 2018-01-18 11:41, Pedro Alves wrote: > On 12/31/2017 05:50 AM, Simon Marchi wrote: >> The to_detach target_ops method implementations are currently expected >> to work on current_inferior/inferior_ptid. In order to make things >> more >> explicit, and remove some "shadow" parameter passing through globals, >> this patch adds an "inferior" parameter to to_detach. Implementations >> will be expected to use this instead of relying on the global. >> However, >> to keep things simple, this patch only does the minimum that is >> necessary to add the parameter. The following patch gives an example >> of >> how one such implementation would be adapted. If the approach is >> deemed >> good, we can then look into adapting more implementations. Until >> then, >> they'll continue to work as they do currently. > > On the multi-target work/branch, I ended up hanging the current > target stack to the current inferior. Which means that in practice > we have to switch the current inferior/thread before calling any target > method. I can't see any other practical way. I've pondered making > every target method take an inferior or some kind of "execution > context" object as parameter, but that's a massive undertaking. > The result is still better for relying more on thread pointers > instead of inferior_ptid / ptid_t, though, IMO. Ok. > Note that in practice, even with your patch (in master) we still have > to switch the current inferior before calling target_detach anyway, > for making sure things like the current program space is set correctly, > target_gdbarch(), target description, removing breakpoints, accessing > memory, > registers, etc., like here: > >> /* Note that the detach above makes PARENT_INF dangling. */ >> @@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec) >> } >> } >> >> - target_detach (0); >> + target_detach (inf->vfork_parent, 0); > > ... this still relies on the switch_to_thread call a bit above. > > Or look at the prepare_to_detach call in target_detach. > > All that said, I'm totally fine with incremental progress. Actually, > it's probably the only way to go! Yes this is the idea. The ramifications go very deep, so it's hard to know what still relies on the current inferior being set. This is just one step in the right direction, and it's easier to take smaller steps. > So I'm fine with the patch, though, I think we should add a > temporary assertion in target_detach, like: > > gdb_assert (inf == current_inferior ()); > > until everything beneath either uses an explicit inferior, > or switches the current inferior temporarily. That's a good idea, this way it will also check if I passed the wrong thing to target_detach. Simon