From patchwork Fri Nov 4 17:50:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 59952 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DE79E3858280 for ; Fri, 4 Nov 2022 17:50:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DE79E3858280 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1667584234; bh=VcPE/fji9b2WoEOsA5PEiFXGSVw3AC+pV2d+A92uf+k=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=r9HHLfYoE+8jRpn7aJkU7P7j2nZJuUdtEgVq75q3nMU+vlzDy+ZRIwXc1ha0CaFR/ I4afCoJd7pYbDdT/8++iHhJ7pkclji9fVsGoe0uRp25xZFYQHVwR9RnTPqRXur/ibH lRJlHYJqixZgZmpA48viFw+Hzl9o0maA7hZRH/eo= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id CDA343858C60 for ; Fri, 4 Nov 2022 17:50:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CDA343858C60 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id C5CAB2188B for ; Fri, 4 Nov 2022 17:50:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id A42E913216 for ; Fri, 4 Nov 2022 17:50:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id iqydJs9QZWOSHwAAMHmgww (envelope-from ) for ; Fri, 04 Nov 2022 17:50:07 +0000 To: gdb-patches@sourceware.org Subject: [PATCH] [gdb/cli] Make quit really quit after remote connection closed Date: Fri, 4 Nov 2022 18:50:07 +0100 Message-Id: <20221104175007.27221-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, 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-Patchwork-Original-From: Tom de Vries via Gdb-patches From: Tom de Vries Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Consider a hello world a.out, started using gdbserver: ... $ gdbserver --once 127.0.0.1:2345 ./a.out Process ./a.out created; pid = 15743 Listening on port 2345 ... that we can connect to using gdb: ... $ gdb -ex "target remote 127.0.0.1:2345" Remote debugging using 127.0.0.1:2345 Reading /home/vries/a.out from remote target... ... 0x00007ffff7dd4550 in _start () from target:/lib64/ld-linux-x86-64.so.2 (gdb) ... After that, we can for instance quit with confirmation: ... (gdb) quit A debugging session is active. Inferior 1 [process 16691] will be killed. Quit anyway? (y or n) y $ ... Or, kill with confirmation and quit: ... (gdb) kill Kill the program being debugged? (y or n) y [Inferior 1 (process 16829) killed] (gdb) quit $ ... Or, monitor exit, kill with confirmation, and quit: ... (gdb) monitor exit (gdb) kill Kill the program being debugged? (y or n) y Remote connection closed (gdb) quit $ ... But when doing monitor exit followed by quit with confirmation, we get the gdb prompt back, requiring us to do quit once more: ... (gdb) monitor exit (gdb) quit A debugging session is active. Inferior 1 [process 16944] will be killed. Quit anyway? (y or n) y Remote connection closed (gdb) quit $ ... So, the first quit didn't quit. This happens as follows: - quit_command calls query_if_trace_running - a TARGET_CLOSE_ERROR is thrown - it's caught in remote_target::get_trace_status, but then rethrown because it's TARGET_CLOSE_ERROR - catch_command_errors catches the error, at which point the quit command has been aborted. The TARGET_CLOSE_ERROR is defined as: ... /* Target throwing an error has been closed. Current command should be aborted as the inferior state is no longer valid. */ TARGET_CLOSE_ERROR, ... so in a way this is expected behaviour. But aborting quit because the inferior state (which we've already confirmed we're not interested in) is no longer valid, and having to type quit again seems pointless. Furthermore, the purpose of not catching errors thrown by query_if_trace_running as per commit 2f9d54cfcef ("make -gdb-exit call disconnect_tracing too, and don't lose history if the target errors on "quit""), was to make sure that error (_("Not confirmed.") had effect. Fix this in quit_command by catching only the TARGET_CLOSE_ERROR exception during query_if_trace_running and reporting it: ... (gdb) monitor exit (gdb) quit A debugging session is active. Inferior 1 [process 19219] will be killed. Quit anyway? (y or n) y Remote connection closed $ ... Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15746 --- gdb/cli/cli-cmds.c | 15 +++- .../gdb.server/monitor-exit-quit.exp | 81 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.server/monitor-exit-quit.exp base-commit: ac87b20a96adec653af45253fc3b2daf1a0710eb diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index fe4041662ef..bcfd3641ef5 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -488,7 +488,20 @@ quit_command (const char *args, int from_tty) if (!quit_confirm ()) error (_("Not confirmed.")); - query_if_trace_running (from_tty); + try + { + query_if_trace_running (from_tty); + } + catch (const gdb_exception_error &ex) + { + if (ex.error == TARGET_CLOSE_ERROR) + /* We don't care about this since we're quitting anyway, so keep + quitting. */ + exception_print (gdb_stderr, ex); + else + /* Rethrow, to properly handle error (_("Not confirmed.")). */ + throw; + } quit_force (args ? &exit_code : NULL, from_tty); } diff --git a/gdb/testsuite/gdb.server/monitor-exit-quit.exp b/gdb/testsuite/gdb.server/monitor-exit-quit.exp new file mode 100644 index 00000000000..fdf24520989 --- /dev/null +++ b/gdb/testsuite/gdb.server/monitor-exit-quit.exp @@ -0,0 +1,81 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test quitting after monitor exit. + +load_lib gdbserver-support.exp + +standard_testfile server.c + +if { [skip_gdbserver_tests] } { + return 0 +} + +if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} { + return -1 +} + +save_vars { GDBFLAGS } { + # If GDB and GDBserver are both running locally, set the sysroot to avoid + # reading files via the remote protocol. + if { ![is_remote host] && ![is_remote target] } { + set GDBFLAGS "$GDBFLAGS -ex \"set sysroot\"" + } + + clean_restart $binfile +} + +# Make sure we're disconnected, in case we're testing with an +# extended-remote board, therefore already connected. +gdb_test "disconnect" ".*" + +set target_exec [gdbserver_download_current_prog] + +set res [gdbserver_start "" $target_exec] + +set gdbserver_protocol [lindex $res 0] +set gdbserver_gdbport [lindex $res 1] +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport + +gdb_test_no_output "monitor exit" +gdb_test_no_output "set confirm off" + +set do_cleanup 1 + +gdb_test_multiple "quit" "" { + -re -wrap "" { + fail "$gdb_test_name (prompt)" + # Let default_gdb_exit do the cleanup. + set do_cleanup 0 + } + -early -re "DOSEXIT code" { + pass "$gdb_test_name" + } + -early eof { + pass "$gdb_test_name" + } +} + +# Cleanup, as in default_gdb_exit. +if { $do_cleanup } { + if ![is_remote host] { + remote_close host + } + unset gdb_spawn_id + unset ::gdb_tty_name + unset inferior_spawn_id +}