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 590FB3858C83 for ; Mon, 27 Mar 2023 15:24:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 590FB3858C83 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 32RFOIL8026345 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 27 Mar 2023 11:24:23 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 32RFOIL8026345 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1679930663; bh=/3eKMKtJmgr07lJNNns6TP8UODuYLaeLGAoZ0+s0o4Q=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=LAep+KqnKf8eD5hhzbSalB8e9c7KTRaTDfEYSzX3t2Nzmh3vlmYtOv5gcu8dt0PV1 S6HvhFlJ6ZbE9FuJAbge4KDxB9d6Z0dfv6mJz/fJTYdUfOWngZHa1rqBcEIGZTCJZb byuN0Xm2bXvuo7ru95KP+0kMxGRpQDBhEC8d2wCI= Received: from [192.168.0.101] (modemcable194.137-20-96.mc.videotron.ca [96.20.137.194]) (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 DCD291E110; Mon, 27 Mar 2023 11:24:17 -0400 (EDT) Message-ID: Date: Mon, 27 Mar 2023 11:24:16 -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 v2] PR gdb/30219: Clear sync_quit_force_run in quit_force Content-Language: fr To: Kevin Buettner , gdb-patches@sourceware.org Cc: pedro@palves.net References: <20230310231851.141981-1-kevinb@redhat.com> <20230324150721.7afee776@f37-zws-nv> From: Simon Marchi In-Reply-To: <20230324150721.7afee776@f37-zws-nv> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 27 Mar 2023 15:24:18 +0000 X-Spam-Status: No, score=-3037.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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 3/24/23 18:07, 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. > > 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.) > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30219 > Reviewed-by: Simon Marchi > --- > gdb/exceptions.c | 1 + > gdb/top.c | 8 ++++++++ > 2 files changed, 9 insertions(+) > > diff --git a/gdb/exceptions.c b/gdb/exceptions.c > index 4ab8dce58de..8b7858578a9 100644 > --- a/gdb/exceptions.c > +++ b/gdb/exceptions.c > @@ -90,6 +90,7 @@ print_exception (struct ui_file *file, const struct gdb_exception &e) > switch (e.reason) > { > case RETURN_QUIT: > + case RETURN_FORCED_QUIT: > annotate_quit (); > break; > case RETURN_ERROR: > diff --git a/gdb/top.c b/gdb/top.c > index aa2a6409d98..81f74f72f61 100644 > --- a/gdb/top.c > +++ b/gdb/top.c > @@ -1824,6 +1824,14 @@ quit_force (int *exit_arg, int from_tty) > { > int exit_code = 0; > > + /* Clear the quit flag and sync_quit_force_run so that a > + gdb_exception_forced_quit isn't inadvertently triggered by a QUIT > + check while running the various cleanup/exit code below. Note > + that the call to 'check_quit_flag' clears the quit flag as a side > + effect. */ > + check_quit_flag (); > + sync_quit_force_run = false; > + > /* An optional expression may be used to cause gdb to terminate with the > value of that expression. */ > if (exit_arg) > -- > 2.40.0 > LGTM: Approved-By: Simon Marchi Simon