From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123493 invoked by alias); 2 Mar 2020 19:07:42 -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 123480 invoked by uid 89); 2 Mar 2020 19:07:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=poll, theyll, they'll, arises X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Mar 2020 19:07:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583176058; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UDti7cEfP6X4LZkA5mDlPebuXkh7GRBOzEvBOuXXTA8=; b=QaEUv0kPVUxXyjMGQ0sXvTnJ0M9AJDgIoYmbunsqVgdqLTqgiudyS+bK361a+t3+BHWJ7+ mlMuWR56yq0HMAFZaP+YQVzisXVXh51x7ACpJC3avYjfY42YY6zEhjGV5mCh1sgWw99WM0 bFh7zfAiujrIsHenmBVbDup2DyT6wzo= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-407-x3-zyeVgNyOPtAcCOFy3Eg-1; Mon, 02 Mar 2020 14:07:36 -0500 Received: by mail-ed1-f70.google.com with SMTP id x13so294683eds.19 for ; Mon, 02 Mar 2020 11:07:36 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f909:7b00:21e8:4cd6:8ec1:f387? ([2001:8a0:f909:7b00:21e8:4cd6:8ec1:f387]) by smtp.gmail.com with ESMTPSA id g13sm110990ejx.23.2020.03.02.11.07.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Mar 2020 11:07:34 -0800 (PST) Subject: Re: [PATCHv3 2/2] gdb/remote: Restore support for 'S' stop reply packet To: Andrew Burgess References: <20200302151915.GQ3317@embecosm.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <9af55184-812e-ecb6-a3ba-bb89106b82fb@redhat.com> Date: Mon, 02 Mar 2020 19:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200302151915.GQ3317@embecosm.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2020-03/txt/msg00039.txt On 3/2/20 3:19 PM, Andrew Burgess wrote: > * Pedro Alves [2020-03-02 12:25:12 +0000]: >> "inferior" -> "target". >> >> The "inferior" -> "target" distinction I'm making in these >> small remarks above matters, because say the remote server >> is debugging two single-threaded inferiors. We still want to >> (and do) warn. >=20 > I hadn't really considered this case, however, this raises a > follow on question: >=20 > Before entering the target wait code we call > switch_to_inferior_no_thread, partly, as I understand it because > having inferior_ptid pointing at a thread leads to invalid code > that relies on this thread being _the_ event thread, when really we > need to extract the inferior and thread-id from the stop event. The main reason we switch the inferior is that the target stack is a property of the inferior now. Each inferior has its own target stack (see inferior::m_target_stack). Note, target stack, not target. A single target may be used by different inferiors. E.g., when remote debugging, if you're debugging two inferiors under control of the same gdbserver, each inferior will have a pointer to the same remote target instance in its target stack. To call any target method, we must be sure to make the relevant inferior current, because target methods consult the inferior's target stack to know which target to call. When target methods defer to the target beneath, they'll again consult the target stack stored in the current inferior (the "this->beneath ()->foo()" calls in target-delegates.c). So to call the target_wait method to poll events out of some target, you need to switch to some representative inferior that uses the process_stratum target you want to poll. However, a reason we need to switch to all inferiors in turn and not just a subset sufficient to cover all instantiated process_stratum targets, is the strata above process_stratum, like record_stratum. Those targets aren't shared between inferiors, and they generate target events as well. I suspect it may be possible to come up with a cleaner design here, but I haven't been able to come up with one that's as simple as the current one, given all the constraints of the rest of current gdb's design. I didn't try very hard though. Since we're switching the inferior around, the question=20 of -- which thread in the inferior should we pick? -- immediately arises. And the best answer is "none". Switching to no-thread also has the property that it "catches" target backends relying on inferior_ptid to infer things about the next stopping event, when they should not. (Another point that helps with seeing how that's wrong is to consider that inferior_ptid includes no field that would indicate which target instance the ptid came from. So e.g., if you're debugging against two remote stubs, it could well be the case that when you get to target_wait, inferior_ptid points at a thread of the other target, even though it seemingly looks like a valid thread in the current target.) >=20 > So, why do we allow the current_inferior to remain valid while we > perform the wait? Instead, why don't we clear both current_inferior > and inferior_ptid, and then enter the wait code? It's impossible to clear current_inferior with the current design. There must always be a current inferior. Thanks, Pedro Alves