From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id tGIJAeFvHGRpjxcAWB0awg (envelope-from ) for ; Thu, 23 Mar 2023 11:27:29 -0400 Received: by simark.ca (Postfix, from userid 112) id E7DDC1E223; Thu, 23 Mar 2023 11:27:28 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=WMSfZl5u; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 770D51E110 for ; Thu, 23 Mar 2023 11:27:28 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DA8CB385040B for ; Thu, 23 Mar 2023 15:27:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DA8CB385040B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1679585247; bh=W9JSIlnCB1leIJ5rndWAo8TTLEmg9AzcEEpbhHGY5YA=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=WMSfZl5u75yXnfaA4e/e8gee1BpaR/2gcmWR270Go4mIX5V9g+6Cyrvq4YmO8haRf 24tnqFqjLhjwn1iWw1Qra0BNkQLUXcmntMqXgYUc4fABsPARnxEojAKEEzXlVXsxzF z0loK19cLIfjjTOEyHh9/jGUh+Gpdnur4TEENA2M= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 2529B3858CDB for ; Thu, 23 Mar 2023 15:27:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2529B3858CDB 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 32NFQuae018553 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Mar 2023 11:27:01 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 32NFQuae018553 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 956511E110; Thu, 23 Mar 2023 11:26:56 -0400 (EDT) Message-ID: <9bf423f6-bea3-6b0e-3052-596de36d4a5e@polymtl.ca> Date: Thu, 23 Mar 2023 11:26:56 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCH] PR gdb/30219: Clear sync_quit_force_run in quit_force Content-Language: en-US To: Kevin Buettner , gdb-patches@sourceware.org Cc: pedro@palves.net References: <20230310231851.141981-1-kevinb@redhat.com> In-Reply-To: <20230310231851.141981-1-kevinb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 23 Mar 2023 15:26:56 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 3/10/23 18:18, Kevin Buettner wrote: > PR 30219 shows an internal error due to a "Bad switch" in > print_exception() in gdb/exceptions.c. The switch in question > contains cases for RETURN_QUIT and RETURN_ERROR, but is missing a case > for the recently added RETURN_FORCED_QUIT. This commit adds that case > in addition to introducing annotate_forced_quit() which will print a > suitable annotation when annotations are turned on. > > Making the above change allows the errant test case to pass, but does > not fix the underlying problem, which I'll describe shortly. Even > though the addition of a case for RETURN_FORCED_QUIT isn't the actual > fix, I still think it's important to add this case so that other > situations which lead to print_exeption() being called won't generate > that "Bad switch" internal error. > > In order to understand the underlying problem, please examine > this portion of the backtrace from the bug report: > > 0x5576e4ff5780 print_exception > /home/smarchi/src/binutils-gdb/gdb/exceptions.c:100 > 0x5576e4ff5930 exception_print(ui_file*, gdb_exception const&) > /home/smarchi/src/binutils-gdb/gdb/exceptions.c:110 > 0x5576e6a896dd quit_force(int*, int) > /home/smarchi/src/binutils-gdb/gdb/top.c:1849 > > The real problem is in quit_force; here's the try/catch which > eventually leads to the internal error: > > /* Get out of tfind mode, and kill or detach all inferiors. */ > try > { > disconnect_tracing (); > for (inferior *inf : all_inferiors ()) > kill_or_detach (inf, from_tty); > } > catch (const gdb_exception &ex) > { > exception_print (gdb_stderr, ex); > } > > While running the calls in the try-block, a QUIT check is being > performed. This check finds that sync_quit_force_run is (still) set, > causing a gdb_exception_forced_quit to be thrown. The exception > gdb_exception_forced_quit is derived from gdb_exception, causing > exception_print to be called. As shown by the backtrace, > print_exception is then called, leading to the internal error. > > The actual fix, also implemented by this commit, is to clear > sync_quit_force_run along with the quit flag. This will allow the > various cleanup code, called by quit_force, to run without triggering > a gdb_exception_forced_quit. (Though, if another SIGTERM is sent to > the gdb process, these flags will be set again and a QUIT check in the > cleanup code will detect it and throw the exception.) Clearing the quit flag sounds right. However, I wonder if we should go further and disable any "quitting" while GDB is in quit_force. Meaning that any SIGTERM or SIGINT received while in quit_force would have no effect. I don't know what good could be achieved by responding to a SIGTERM or SIGINT while GDB is already in the process of cleaning up and exiting. What you have is fine with me for now though, it's a clear improvement over the current situation. > diff --git a/gdb/annotate.c b/gdb/annotate.c > index 60fe6ccd5c2..36efce1af7c 100644 > --- a/gdb/annotate.c > +++ b/gdb/annotate.c > @@ -282,6 +282,13 @@ annotate_quit (void) > printf_unfiltered (("\n\032\032quit\n")); > } > > +void > +annotate_forced_quit (void) > +{ > + if (annotation_level > 1) > + printf_unfiltered (("\n\032\032forced-quit\n")); Hmm, this in a new annotation? In theory, this would need to be documented, to have a NEWS entry, etc. And then frontends using annotations would need to be updated to use that. We don't really want that, if possible, given annotations are obsolete. Would it work to display the "quit" annotation (the one printed by annotate_quit)? The doc [1] says, for that annotation: This annotation occurs right before GDB responds to an interrupt. and: Quit and error annotations indicate that any annotations which GDB was in the middle of may end abruptly. For example, if a value-history-begin annotation is followed by a error, one cannot expect to receive the matching value-history-end. I think it also works in the forced quit case. The execution of whatever thing was going on is interrupted, just like in the regular quit case, it just happens that GDB also happens to exit after that. [1] https://sourceware.org/gdb/onlinedocs/annotate.html#Errors Simon