From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id AE73B3858C3A for ; Mon, 28 Nov 2022 13:03:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AE73B3858C3A Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca 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 2ASD3B5m009352 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Nov 2022 08:03:16 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 2ASD3B5m009352 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1669640597; bh=mdo4dwNcaOvrbaA3oKHfAJlOwx1O4sJai34EJxS/N9U=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=LZnCb07RNL5Hf05RRnVDlneaStbUzuLQcrhFU+IZ5xIVAz8bgK9EAMvcrk+X+ILJV GjAXMA1ef1XCZ++lT28l0vDEkEBsKcZEb56kiqg9o+S81hC0JGkFhfyLl8INwwyNDo qjTee6UvmnDHMBh3mDA/b6c2kJNk8ieBsQ5UD6Pg= 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) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 798851E0CB; Mon, 28 Nov 2022 08:03:11 -0500 (EST) Message-ID: <8ee0b3a2-989e-c546-1c2f-aba900b0cef6@polymtl.ca> Date: Mon, 28 Nov 2022 08:03:11 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v2 3/5] gdb: fix assert when quitting GDB while a thread is stepping Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org Cc: Simon Marchi References: <20221121171213.1414366-1-simon.marchi@polymtl.ca> <20221121171213.1414366-4-simon.marchi@polymtl.ca> <87fse32sa0.fsf@redhat.com> From: Simon Marchi In-Reply-To: <87fse32sa0.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 28 Nov 2022 13:03:11 +0000 X-Spam-Status: No, score=-3032.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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 List-Id: On 11/28/22 07:11, Andrew Burgess wrote: > Simon Marchi writes: > >> From: Andrew Burgess >> >> This commit addresses one of the issues identified in PR gdb/28275. >> >> Bug gdb/28275 identifies a number of situations in which this assert: >> >> Assertion `!proc_target->commit_resumed_state' failed. >> >> could be triggered. There's actually a number of similar places where >> this assert is found in GDB, the two of interest in gdb/28275 are in >> target_wait and target_stop. >> >> In one of the comments: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=28275#c1 >> >> steps to trigger the assertion within target_stop were identified when >> using a modified version of the gdb.threads/detach-step-over.exp test >> script. >> >> In the gdb.threads/detach-step-over.exp test, we attach to a >> multi-threaded inferior, and continue the inferior in asynchronous >> (background) mode. Each thread is continuously hitting a conditional >> breakpoint where the condition is always false. While the inferior is >> running we detach. The goal is that we detach while GDB is performing a >> step-over for the conditional breakpoint in at least one thread. >> >> While detaching, if a step-over is in progress, then GDB has to >> complete the step over before we can detach. This involves calling >> target_stop and target_wait (see prepare_for_detach). >> >> As far as gdb/28275 is concerned, the interesting part here, is the >> the process_stratum_target::commit_resumed_state variable must be >> false when target_stop and target_wait are called. >> >> This is currently ensured because, in detach_command (infrun.c), we >> create an instance of scoped_disable_commit_resumed, this ensures that >> when target_detach is called, ::commit_resumed_state will be false. >> >> The modification to the test that I propose here, and which exposed >> the bug, is that, instead of using "detach" to detach from the >> inferior, we instead use "quit". Quitting GDB after attaching to an >> inferior will cause GDB to first detach, and then exit. >> >> When we quit GDB we end up calling target_detach via a different code >> path, the stack looks like: >> >> #0 target_detach >> #1 kill_or_detach >> #2 quit_force >> #3 quit_command >> >> Along this path there is no scoped_disable_commit_resumed created. >> ::commit_resumed_state can be true when we reach prepare_for_detach, >> which calls target_wait and target_stop, so the assertion will trigger. >> >> In this commit, I propose fixing this by adding the creation of a >> scoped_disable_commit_resumed into target_detach. This will ensure >> that ::commit_resumed_state is false when calling prepare_for_detach >> from within target_detach. >> >> I did consider placing the scoped_disable_commit_resumed in >> prepare_for_detach, however, placing it in target_detach ensures that >> the target's commit_resumed_state flag is left to false if the detached >> inferior was the last one for that target. It's the same rationale as >> for patch "gdb: disable commit resumed in target_kill" that comes later >> in this series, but for detach instead of kill. >> >> detach_command still includes a scoped_disable_commit_resumed too, but I >> think it is still relevant to cover the resumption at the end of the >> function. >> >> Co-Authored-By: Simon Marchi >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275 >> Change-Id: Ie128f7aba6ef0e018859275eca372e6ea738e96f > > LGTM. Thanks for taking this into your series. > > Thanks, > Andrew Thanks, pushed up to here. Simon