From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40654 invoked by alias); 30 Aug 2018 20:26:29 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 40641 invoked by uid 89); 30 Aug 2018 20:26:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Mention X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Aug 2018 20:26:26 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 892E240241C3; Thu, 30 Aug 2018 20:26:24 +0000 (UTC) Received: from localhost (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 62BEC2027056; Thu, 30 Aug 2018 20:26:24 +0000 (UTC) From: Sergio Durigan Junior To: Gary Benson Cc: gdb-patches@sourceware.org, tom@tromey.com Subject: Re: [PATCH v2] Indicate batch mode failures by exiting with nonzero status References: <1534934813-10188-1-git-send-email-gbenson@redhat.com> Date: Thu, 30 Aug 2018 20:26:00 -0000 In-Reply-To: <1534934813-10188-1-git-send-email-gbenson@redhat.com> (Gary Benson's message of "Wed, 22 Aug 2018 11:46:53 +0100") Message-ID: <8736uviosv.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00843.txt.bz2 On Wednesday, August 22 2018, Gary Benson wrote: > Hi all, > > This patch causes GDB in batch mode to exit with nonzero status > if the last command to be executed fails. > > Changes since version 1: > - I've reworked the patch to not use a global variable. > - Behaviour has not changed from version 1. > > Built and regtested on RHEL 7.5 x86_64. Hey, just to make sure this is reported here: I'm seeing failures when running this test on my Fedora 28 x86_64: FAIL: gdb.base/batch-exit-status.exp: -batch -jslkflsdjlkfjlksdjf: $actual_status == $expect_status FAIL: gdb.base/batch-exit-status.exp: -batch -ex "set not-a-thing 4": $actual_status == $expect_status FAIL: gdb.base/batch-exit-status.exp: -batch -x /home/sergio/work/src/git/binutils-gdb/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/batch-exit-status.bad-commands: $actua$ _status == $expect_status FAIL: gdb.base/batch-exit-status.exp: -batch -x /home/sergio/work/src/git/binutils-gdb/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/batch-exit-status.good-commands -x /ho$ e/sergio/work/src/git/binutils-gdb/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/batch-exit-status.bad-commands: $actual_status == $expect_status FAIL: gdb.base/batch-exit-status.exp: -batch -x /home/sergio/work/src/git/binutils-gdb/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.base/batch-exit-status.good-commands -ex "s$ t not-a-thing 4": $actual_status == $expect_status This has also been caught by the BuildBot: https://sourceware.org/ml/gdb-testers/2018-q3/msg05129.html https://sourceware.org/ml/gdb-testers/2018-q3/msg05223.html Let me know if you need more details. Thanks, > Ok to commit? > > Thanks, > Gary > > -- > gdb/ChangeLog: > > PR gdb/13000: > * gdb/main.c (captured_main_1): Exit with nonzero status > in batch mode if the last command to be executed failed. > * NEWS: Mention the above. > > gdb/testsuite/ChangeLog: > > PR gdb/13000: > * gdb.base/batch-exit-status.exp: New file. > * gdb.base/batch-exit-status.good-commands: Likewise. > * gdb.base/batch-exit-status.bad-commands: Likewise. > --- > gdb/ChangeLog | 7 ++ > gdb/NEWS | 3 + > gdb/main.c | 78 +++++++++++++--------- > gdb/testsuite/ChangeLog | 7 ++ > .../gdb.base/batch-exit-status.bad-commands | 1 + > gdb/testsuite/gdb.base/batch-exit-status.exp | 63 +++++++++++++++++ > .../gdb.base/batch-exit-status.good-commands | 1 + > 7 files changed, 129 insertions(+), 31 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/batch-exit-status.bad-commands > create mode 100644 gdb/testsuite/gdb.base/batch-exit-status.exp > create mode 100644 gdb/testsuite/gdb.base/batch-exit-status.good-commands > > diff --git a/gdb/NEWS b/gdb/NEWS > index 16d3d72..6af712a 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -10,6 +10,9 @@ > * DWARF index cache: GDB can now automatically save indices of DWARF > symbols on disk to speed up further loading of the same binaries. > > +* GDB in batch mode now exits with status 1 if the last command to be > + executed failed. > + > * New commands > > frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND > diff --git a/gdb/main.c b/gdb/main.c > index e925128..61644cd 100644 > --- a/gdb/main.c > +++ b/gdb/main.c > @@ -486,6 +486,7 @@ captured_main_1 (struct captured_main_args *context) > int i; > int save_auto_load; > struct objfile *objfile; > + int ret = 1; > > #ifdef HAVE_USEFUL_SBRK > /* Set this before constructing scoped_command_stats. */ > @@ -986,7 +987,7 @@ captured_main_1 (struct captured_main_args *context) > processed; it sets global parameters, which are independent of > what file you are debugging or what directory you are in. */ > if (system_gdbinit && !inhibit_gdbinit) > - catch_command_errors (source_script, system_gdbinit, 0); > + ret = catch_command_errors (source_script, system_gdbinit, 0); > > /* Read and execute $HOME/.gdbinit file, if it exists. This is done > *before* all the command line arguments are processed; it sets > @@ -994,7 +995,7 @@ captured_main_1 (struct captured_main_args *context) > debugging or what directory you are in. */ > > if (home_gdbinit && !inhibit_gdbinit && !inhibit_home_gdbinit) > - catch_command_errors (source_script, home_gdbinit, 0); > + ret = catch_command_errors (source_script, home_gdbinit, 0); > > /* Process '-ix' and '-iex' options early. */ > for (i = 0; i < cmdarg_vec.size (); i++) > @@ -1004,12 +1005,12 @@ captured_main_1 (struct captured_main_args *context) > switch (cmdarg_p.type) > { > case CMDARG_INIT_FILE: > - catch_command_errors (source_script, cmdarg_p.string, > - !batch_flag); > + ret = catch_command_errors (source_script, cmdarg_p.string, > + !batch_flag); > break; > case CMDARG_INIT_COMMAND: > - catch_command_errors (execute_command, cmdarg_p.string, > - !batch_flag); > + ret = catch_command_errors (execute_command, cmdarg_p.string, > + !batch_flag); > break; > } > } > @@ -1017,11 +1018,11 @@ captured_main_1 (struct captured_main_args *context) > /* Now perform all the actions indicated by the arguments. */ > if (cdarg != NULL) > { > - catch_command_errors (cd_command, cdarg, 0); > + ret = catch_command_errors (cd_command, cdarg, 0); > } > > for (i = 0; i < dirarg.size (); i++) > - catch_command_errors (directory_switch, dirarg[i], 0); > + ret = catch_command_errors (directory_switch, dirarg[i], 0); > > /* Skip auto-loading section-specified scripts until we've sourced > local_gdbinit (which is often used to augment the source search > @@ -1036,19 +1037,20 @@ captured_main_1 (struct captured_main_args *context) > /* The exec file and the symbol-file are the same. If we can't > open it, better only print one error message. > catch_command_errors returns non-zero on success! */ > - if (catch_command_errors (exec_file_attach, execarg, > - !batch_flag)) > - catch_command_errors (symbol_file_add_main_adapter, symarg, > - !batch_flag); > + ret = catch_command_errors (exec_file_attach, execarg, > + !batch_flag); > + if (ret != 0) > + ret = catch_command_errors (symbol_file_add_main_adapter, > + symarg, !batch_flag); > } > else > { > if (execarg != NULL) > - catch_command_errors (exec_file_attach, execarg, > - !batch_flag); > + ret = catch_command_errors (exec_file_attach, execarg, > + !batch_flag); > if (symarg != NULL) > - catch_command_errors (symbol_file_add_main_adapter, symarg, > - !batch_flag); > + ret = catch_command_errors (symbol_file_add_main_adapter, > + symarg, !batch_flag); > } > > if (corearg && pidarg) > @@ -1056,9 +1058,14 @@ captured_main_1 (struct captured_main_args *context) > "a core file at the same time.")); > > if (corearg != NULL) > - catch_command_errors (core_file_command, corearg, !batch_flag); > + { > + ret = catch_command_errors (core_file_command, corearg, > + !batch_flag); > + } > else if (pidarg != NULL) > - catch_command_errors (attach_command, pidarg, !batch_flag); > + { > + ret = catch_command_errors (attach_command, pidarg, !batch_flag); > + } > else if (pid_or_core_arg) > { > /* The user specified 'gdb program pid' or gdb program core'. > @@ -1067,14 +1074,20 @@ captured_main_1 (struct captured_main_args *context) > > if (isdigit (pid_or_core_arg[0])) > { > - if (catch_command_errors (attach_command, pid_or_core_arg, > - !batch_flag) == 0) > - catch_command_errors (core_file_command, pid_or_core_arg, > - !batch_flag); > + ret = catch_command_errors (attach_command, pid_or_core_arg, > + !batch_flag); > + if (ret == 0) > + ret = catch_command_errors (core_file_command, > + pid_or_core_arg, > + !batch_flag); > + } > + else > + { > + /* Can't be a pid, better be a corefile. */ > + ret = catch_command_errors (core_file_command, > + pid_or_core_arg, > + !batch_flag); > } > - else /* Can't be a pid, better be a corefile. */ > - catch_command_errors (core_file_command, pid_or_core_arg, > - !batch_flag); > } > > if (ttyarg != NULL) > @@ -1098,7 +1111,7 @@ captured_main_1 (struct captured_main_args *context) > { > auto_load_local_gdbinit_loaded = 1; > > - catch_command_errors (source_script, local_gdbinit, 0); > + ret = catch_command_errors (source_script, local_gdbinit, 0); > } > } > > @@ -1118,12 +1131,12 @@ captured_main_1 (struct captured_main_args *context) > switch (cmdarg_p.type) > { > case CMDARG_FILE: > - catch_command_errors (source_script, cmdarg_p.string, > - !batch_flag); > + ret = catch_command_errors (source_script, cmdarg_p.string, > + !batch_flag); > break; > case CMDARG_COMMAND: > - catch_command_errors (execute_command, cmdarg_p.string, > - !batch_flag); > + ret = catch_command_errors (execute_command, cmdarg_p.string, > + !batch_flag); > break; > } > } > @@ -1134,8 +1147,11 @@ captured_main_1 (struct captured_main_args *context) > > if (batch_flag) > { > + int error_status = EXIT_FAILURE; > + int *exit_arg = ret == 0 ? &error_status : NULL; > + > /* We have hit the end of the batch file. */ > - quit_force (NULL, 0); > + quit_force (exit_arg, 0); > } > } > > diff --git a/gdb/testsuite/gdb.base/batch-exit-status.bad-commands b/gdb/testsuite/gdb.base/batch-exit-status.bad-commands > new file mode 100644 > index 0000000..4793acb > --- /dev/null > +++ b/gdb/testsuite/gdb.base/batch-exit-status.bad-commands > @@ -0,0 +1 @@ > +bork > diff --git a/gdb/testsuite/gdb.base/batch-exit-status.exp b/gdb/testsuite/gdb.base/batch-exit-status.exp > new file mode 100644 > index 0000000..bee4d72 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/batch-exit-status.exp > @@ -0,0 +1,63 @@ > +# Copyright (C) 2018 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 . > + > +# Check that "gdb -batch" exits with appropriate status. > + > +standard_testfile > + > +set good_commands "$srcdir/$subdir/batch-exit-status.good-commands" > +set bad_commands "$srcdir/$subdir/batch-exit-status.bad-commands" > + > +proc _test_exit_status {expect_status cmdline_opts} { > + global gdb_spawn_id > + > + gdb_exit > + if {[gdb_spawn_with_cmdline_opts $cmdline_opts] != 0} { > + fail "spawn" > + return > + } > + > + set result [wait -i $gdb_spawn_id] > + verbose $result > + gdb_assert { [lindex $result 2] == 0 } > + set actual_status [lindex $result 3] > + gdb_assert { $actual_status == $expect_status } > +} > + > +proc test_exit_status {expect_status cmdline_opts} { > + with_test_prefix $cmdline_opts { > + _test_exit_status $expect_status $cmdline_opts > + } > +} > + > +# gdb -batch with nothing to do should exit 0. > +test_exit_status 0 "-batch" > + > +# Bad command-line options should cause exit 1. > +test_exit_status 1 "-batch -jslkflsdjlkfjlksdjf" > + > +# gdb -batch with good commands should exit 0. > +test_exit_status 0 "-batch -ex \"info source\"" > +test_exit_status 0 "-batch -x $good_commands" > + > +# gdb -batch with bad commands should exit 1. > +test_exit_status 1 "-batch -ex \"set not-a-thing 4\"" > +test_exit_status 1 "-batch -x $bad_commands" > + > +# Success or failure of the last thing determines the exit code. > +test_exit_status 0 "-batch -ex \"set not-a-thing 4\" -x $good_commands" > +test_exit_status 0 "-batch -x $bad_commands -ex \"info source\"" > +test_exit_status 1 "-batch -x $good_commands -x $bad_commands" > +test_exit_status 1 "-batch -x $good_commands -ex \"set not-a-thing 4\"" > diff --git a/gdb/testsuite/gdb.base/batch-exit-status.good-commands b/gdb/testsuite/gdb.base/batch-exit-status.good-commands > new file mode 100644 > index 0000000..80708a9 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/batch-exit-status.good-commands > @@ -0,0 +1 @@ > +info mem > -- > 1.8.3.1 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/