Re: [PATCH v2] Indicate batch mode failures by exiting with nonzero status

Sergio Durigan Junior
2018-08-30T20:26:29Z

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 <http://www.gnu.org/licenses/>.
> +
> +# 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