[PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground

Pedro Alves
2014-10-09T18:00:41Z

When we repeat a command, by just pressing <ret>, the input from the
previous command is reused for the new command invocation.

When an execution command strips the "&" out of its incoming argument
string, to detect background execution, we poke a '\0' directly to the
incoming argument string.

Combine both, and a repeat of a background command loses the "&".

This is actually only visible if args other than "&" are specified
(e.g., "c 1&" or "next 2&" or "c -a&"), as in the special case of "&"
alone (e.g. "c&") doesn't actually clobber the incoming string.

Fix this by making strip_bg_char return a new string instead of poking
a hole in the input string.

New test included.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-09 Pedro Alves <palves@redhat.com>

PR gdb/17471
* infcmd.c (strip_bg_char): Change prototype and rewrite. Now
returns a copy of the input.
(run_command_1, continue_command, step_1, jump_command)
(signal_command, until_command, advance_command, finish_command)
(attach_command): Adjust and install a cleanup to free the
stripped args.

gdb/testsuite/
2014-10-09 Pedro Alves <palves@redhat.com>

PR gdb/17471
* gdb.base/bg-execution-repeat.c: New file.
* gdb.base/bg-execution-repeat.exp: New file.
---
gdb/infcmd.c | 142 ++++++++++++++++---------
gdb/testsuite/gdb.base/bg-execution-repeat.c | 33 ++++++
gdb/testsuite/gdb.base/bg-execution-repeat.exp | 86 +++++++++++++++
3 files changed, 208 insertions(+), 53 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/bg-execution-repeat.c
create mode 100644 gdb/testsuite/gdb.base/bg-execution-repeat.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index d270664..4415b31 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -104,8 +104,6 @@ static void run_no_args_command (char *args, int from_tty);

static void go_command (char *line_no, int from_tty);

-static int strip_bg_char (char **);
-
void _initialize_infcmd (void);

#define ERROR_NO_INFERIOR \
@@ -370,35 +368,40 @@ construct_inferior_arguments (int argc, char **argv)
}
\f

-/* This function detects whether or not a '&' character (indicating
- background execution) has been added as *the last* of the arguments ARGS
- of a command. If it has, it removes it and returns 1. Otherwise it
- does nothing and returns 0. */
+/* This function strips the '&' character (indicating background
+ execution) that is added as *the last* of the arguments ARGS of a
+ command. A copy of the incoming ARGS without the '&' is returned,
+ unless the resulting string after stripping is empty, in which case
+ NULL is returned. *BG_CHAR_P is an output boolean that indicates
+ whether the '&' character was found. */

-static int
-strip_bg_char (char **args)
+static char *
+strip_bg_char (const char *args, int *bg_char_p)
{
- char *p = NULL;
+ const char *p;

- p = strchr (*args, '&');
+ if (args == NULL || *args == '\0')
+ {
+ *bg_char_p = 0;
+ return NULL;
+ }

- if (p)
+ p = args + strlen (args);
+ if (p[-1] == '&')
{
- if (p == (*args + strlen (*args) - 1))
- {
- if (strlen (*args) > 1)
- {
- do
- p--;
- while (*p == ' ' || *p == '\t');
- *(p + 1) = '\0';
- }
- else
- *args = 0;
- return 1;
- }
+ p--;
+ while (p > args && isspace (p[-1]))
+ p--;
+
+ *bg_char_p = 1;
+ if (p != args)
+ return savestring (args, p - args);
+ else
+ return NULL;
}
- return 0;
+
+ *bg_char_p = 0;
+ return xstrdup (args);
}

/* Common actions to take after creating any sort of inferior, by any
@@ -527,7 +530,8 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
ptid_t ptid;
struct ui_out *uiout = current_uiout;
struct target_ops *run_target;
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

dont_repeat ();

@@ -550,8 +554,8 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
reopen_exec_file ();
reread_symbols ();

- if (args != NULL)
- async_exec = strip_bg_char (&args);
+ args = strip_bg_char (args, &async_exec);
+ args_chain = make_cleanup (xfree, args);

/* Do validation and preparation before possibly changing anything
in the inferior. */
@@ -597,6 +601,9 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
ui_out_flush (uiout);
}

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
/* We call get_inferior_args() because we might need to compute
the value now. */
run_target->to_create_inferior (run_target, exec_file, get_inferior_args (),
@@ -770,13 +777,15 @@ continue_1 (int all_threads)
static void
continue_command (char *args, int from_tty)
{
- int async_exec = 0;
+ int async_exec;
int all_threads = 0;
+ struct cleanup *args_chain;
+
ERROR_NO_INFERIOR;

/* Find out whether we must run in the background. */
- if (args != NULL)
- async_exec = strip_bg_char (&args);
+ args = strip_bg_char (args, &async_exec);
+ args_chain = make_cleanup (xfree, args);

prepare_execution_command (&current_target, async_exec);

@@ -840,6 +849,9 @@ continue_command (char *args, int from_tty)
}
}

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
if (from_tty)
printf_filtered (_("Continuing.\n"));

@@ -899,21 +911,25 @@ step_1 (int skip_subroutines, int single_inst, char *count_string)
{
int count = 1;
struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
- int async_exec = 0;
+ int async_exec;
int thread = -1;
+ struct cleanup *args_chain;

ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
ensure_valid_thread ();
ensure_not_running ();

- if (count_string)
- async_exec = strip_bg_char (&count_string);
+ count_string = strip_bg_char (count_string, &async_exec);
+ args_chain = make_cleanup (xfree, count_string);

prepare_execution_command (&current_target, async_exec);

count = count_string ? parse_and_eval_long (count_string) : 1;

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
if (!single_inst || skip_subroutines) /* Leave si command alone. */
{
struct thread_info *tp = inferior_thread ();
@@ -1134,7 +1150,8 @@ jump_command (char *arg, int from_tty)
struct symtab_and_line sal;
struct symbol *fn;
struct symbol *sfn;
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1142,8 +1159,8 @@ jump_command (char *arg, int from_tty)
ensure_not_running ();

/* Find out whether we must run in the background. */
- if (arg != NULL)
- async_exec = strip_bg_char (&arg);
+ arg = strip_bg_char (arg, &async_exec);
+ args_chain = make_cleanup (xfree, arg);

prepare_execution_command (&current_target, async_exec);

@@ -1159,6 +1176,9 @@ jump_command (char *arg, int from_tty)
sal = sals.sals[0];
xfree (sals.sals);

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
if (sal.symtab == 0 && sal.pc == 0)
error (_("No source file has been specified."));

@@ -1227,7 +1247,8 @@ static void
signal_command (char *signum_exp, int from_tty)
{
enum gdb_signal oursig;
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

dont_repeat (); /* Too dangerous. */
ERROR_NO_INFERIOR;
@@ -1236,8 +1257,8 @@ signal_command (char *signum_exp, int from_tty)
ensure_not_running ();

/* Find out whether we must run in the background. */
- if (signum_exp != NULL)
- async_exec = strip_bg_char (&signum_exp);
+ signum_exp = strip_bg_char (signum_exp, &async_exec);
+ args_chain = make_cleanup (xfree, signum_exp);

prepare_execution_command (&current_target, async_exec);

@@ -1453,7 +1474,8 @@ until_next_command (int from_tty)
static void
until_command (char *arg, int from_tty)
{
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1461,8 +1483,8 @@ until_command (char *arg, int from_tty)
ensure_not_running ();

/* Find out whether we must run in the background. */
- if (arg != NULL)
- async_exec = strip_bg_char (&arg);
+ arg = strip_bg_char (arg, &async_exec);
+ args_chain = make_cleanup (xfree, arg);

prepare_execution_command (&current_target, async_exec);

@@ -1470,12 +1492,16 @@ until_command (char *arg, int from_tty)
until_break_command (arg, from_tty, 0);
else
until_next_command (from_tty);
+
+ /* Done with ARGS. */
+ do_cleanups (args_chain);
}

static void
advance_command (char *arg, int from_tty)
{
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1486,12 +1512,15 @@ advance_command (char *arg, int from_tty)
error_no_arg (_("a location"));

/* Find out whether we must run in the background. */
- if (arg != NULL)
- async_exec = strip_bg_char (&arg);
+ arg = strip_bg_char (arg, &async_exec);
+ args_chain = make_cleanup (xfree, arg);

prepare_execution_command (&current_target, async_exec);

until_break_command (arg, from_tty, 1);
+
+ /* Done with ARGS. */
+ do_cleanups (args_chain);
}
\f
/* Return the value of the result of a function at the end of a 'finish'
@@ -1766,8 +1795,8 @@ finish_command (char *arg, int from_tty)
{
struct frame_info *frame;
struct symbol *function;
-
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;

ERROR_NO_INFERIOR;
ensure_not_tfind_mode ();
@@ -1775,14 +1804,17 @@ finish_command (char *arg, int from_tty)
ensure_not_running ();

/* Find out whether we must run in the background. */
- if (arg != NULL)
- async_exec = strip_bg_char (&arg);
+ arg = strip_bg_char (arg, &async_exec);
+ args_chain = make_cleanup (xfree, arg);

prepare_execution_command (&current_target, async_exec);

if (arg)
error (_("The \"finish\" command does not take any arguments."));

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
frame = get_prev_frame (get_selected_frame (_("No selected frame.")));
if (frame == 0)
error (_("\"finish\" not meaningful in the outermost frame."));
@@ -2546,7 +2578,8 @@ attach_command_continuation_free_args (void *args)
void
attach_command (char *args, int from_tty)
{
- int async_exec = 0;
+ int async_exec;
+ struct cleanup *args_chain;
struct target_ops *attach_target;

dont_repeat (); /* Not for the faint of heart */
@@ -2567,8 +2600,8 @@ attach_command (char *args, int from_tty)
this function should probably be moved into target_pre_inferior. */
target_pre_inferior (from_tty);

- if (args != NULL)
- async_exec = strip_bg_char (&args);
+ args = strip_bg_char (args, &async_exec);
+ args_chain = make_cleanup (xfree, args);

attach_target = find_attach_target ();

@@ -2582,6 +2615,9 @@ attach_command (char *args, int from_tty)
shouldn't refer to attach_target again. */
attach_target = NULL;

+ /* Done with ARGS. */
+ do_cleanups (args_chain);
+
/* Set up the "saved terminal modes" of the inferior
based on what modes we are starting it with. */
target_terminal_init ();
diff --git a/gdb/testsuite/gdb.base/bg-execution-repeat.c b/gdb/testsuite/gdb.base/bg-execution-repeat.c
new file mode 100644
index 0000000..26ab997
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bg-execution-repeat.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 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/>. */
+
+#include <unistd.h>
+
+int
+foo (void)
+{
+ return 0; /* set break here */
+}
+
+int
+main (void)
+{
+ foo ();
+ sleep (5);
+ foo ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/bg-execution-repeat.exp b/gdb/testsuite/gdb.base/bg-execution-repeat.exp
new file mode 100644
index 0000000..d4eb360
--- /dev/null
+++ b/gdb/testsuite/gdb.base/bg-execution-repeat.exp
@@ -0,0 +1,86 @@
+# Copyright (C) 2014 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/>.
+
+# Test that repeating a background command doesn't lose the "&" in the
+# repeat, turning a background command into a foreground command. See
+# PR gdb/17471.
+
+standard_testfile
+
+if { [build_executable "failed to prepare" ${testfile} $srcfile] } {
+ return -1
+}
+
+set linenum [gdb_get_line_number "set break here"]
+
+# Run the test proper. CONTINUE_CMD is the background continue
+# command to issue.
+
+proc test {continue_cmd} {
+ global gdb_prompt
+ global binfile
+ global linenum
+
+ clean_restart $binfile
+
+ if ![runto_main] {
+ return
+ }
+
+ gdb_breakpoint "$linenum"
+
+ set test $continue_cmd
+ gdb_test_multiple $test $test {
+ -re "Continuing\\.\r\n$gdb_prompt " {
+ # Note no end anchor. If the breakpoint triggers soon enough
+ # enough we see further output after the prompt.
+ pass $test
+ }
+ }
+
+ # Wait for the stop. Don't expect a prompt, as we had resumed the
+ # inferior in the background.
+ set test "breakpoint hit 1"
+ gdb_test_multiple "" $test {
+ -re "set break here" {
+ pass $test
+ }
+ }
+
+ # Trigger a repeat. Buggy GDB used to lose the "&", making this a
+ # foreground command...
+ send_gdb "\n"
+ gdb_test "" "Continuing\\." "repeat bg command"
+
+ # ... and thus further input wouldn't be processed until the target
+ # stopped.
+ gdb_test "print 1" " = 1" "input still accepted"
+
+ # Make sure we see a stop after the print, and not before. Don't
+ # expect a prompt, as we had resumed the inferior in the background.
+ set test "breakpoint hit 2"
+ gdb_test_multiple "" $test {
+ -re "set break here ..\r\n" {
+ pass $test
+ }
+ }
+}
+
+# Test with and without extra arguments.
+foreach cmd {"c&" "c 1&"} {
+ with_test_prefix $cmd {
+ test $cmd
+ }
+}
--
1.9.3