From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 935A23858CDA for ; Mon, 26 Sep 2022 14:16:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 935A23858CDA Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 060701E0D5; Mon, 26 Sep 2022 10:16:19 -0400 (EDT) Message-ID: <1ff3e5de-f218-4657-0798-525158ba4cbe@simark.ca> Date: Mon, 26 Sep 2022 10:16:19 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH] gdb: fix target_ops reference count for some cases Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org References: <20220921131200.3983844-1-aburgess@redhat.com> <9a31d60a-01a8-e527-71d9-0f5908be2961@simark.ca> <87a66re30o.fsf@redhat.com> From: Simon Marchi In-Reply-To: <87a66re30o.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Sep 2022 14:16:22 -0000 >> Just wondering, why do we need to restore explicitly the current >> pspace, instead of using just scoped_restore_current_thread? >> >> scoped_restore_current_pspace_and_thread's doc says: >> >> /* Save/restore the current program space, thread, inferior and frame. >> Use this when you need to call >> switch_to_program_space_and_thread. */ >> >> ... but you are not using switch_to_program_space_and_thread here. >> Maybe it's ok and I just don't understand. Same in >> ~scoped_mock_context. > > I suspect the comment you quote is just out of date. > > switch_to_program_space_and_thread can end up calling > switch_to_inferior_no_thread if there are no running threads in the > program space being switched too. But, even if switch_to_thread does > end up being called we: > > - set the program space, > - set the inferior, > - set the current thread, > - reinit the frame cache, > > By comparison, switch_to_inferior_no_thread does: > > - sets the program space, > - sets the inferior, > - sets the current thread (to nullptr this time though), > - reinits the frame cache, > > As you can see they do the same set of things, all of which I think > should be reverted once we leave the scope, hence > scoped_restore_current_pspace_and_thread seems like the way to go. Hmm okay, but won't scoped_restore_current_thread always restore the pspace one way or another? scoped_restore_current_thread::restore will either call switch_to_thread or switch_to_inferior_no_thread, which both end up setting the pspace. I just don't understand why scoped_restore_current_pspace_and_thread exists, it seems like scoped_restore_current_thread would always work where scoped_restore_current_pspace_and_thread is used. I must be missing something, scoped_restore_current_pspace_and_thread must have been added for a reason. Simon