[PATCH 0/4] Fix a set of terminal/readline handling bugs/crashes

[PATCH 0/4] Fix a set of terminal/readline handling bugs/crashes

[Pedro Alves]

This series fixes a couple terminal/readline handling bugs that result
in GDB crashing with:

 readline: readline_callback_read_char() called with no handler!
 Aborted (core dumped)
 $

... and also a bug where we lose the "&" when repeating background
commands.

Because the these won't trigger without target-async, the first two
are user-visible regressions in 7.8, which has async by default (7.7
didn't).  As such, I'd like to include these in 7.8.1.

For last one (gdb/17471), I'm a bit borderline (re. 7.8.1).
Technically, it isn't really a regression, but still annoying (see
gdb/17300).  If you'd prefer not having that in 7.8.1, please do speak
up.

Tested on x86_64 Fedora 20, native and gdbserver.

Pedro Alves (4):
  Make common code handle target_terminal_* idempotency
  PR gdb/17472: With annotations, input while executing in the
    foreground crashes readline/GDB
  PR gdb/17300: Input after "c -a" crashes readline/GDB
  PR gdb/17471: Repeating a background command makes it foreground

 gdb/annotate.c                                     |  22 ++-
 gdb/infcmd.c                                       | 160 ++++++++++++++-------
 gdb/target.c                                       |  64 +++++++++
 gdb/target.h                                       |  25 ++--
 .../gdb.base/annota-input-while-running.c          |  25 ++++
 .../gdb.base/annota-input-while-running.exp        | 130 +++++++++++++++++
 gdb/testsuite/gdb.base/bg-execution-repeat.c       |  33 +++++
 gdb/testsuite/gdb.base/bg-execution-repeat.exp     |  86 +++++++++++
 .../gdb.base/continue-all-already-running.c        |  25 ++++
 .../gdb.base/continue-all-already-running.exp      |  79 ++++++++++
 gdb/windows-nat.c                                  |   2 +-
 11 files changed, 582 insertions(+), 69 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/annota-input-while-running.c
 create mode 100644 gdb/testsuite/gdb.base/annota-input-while-running.exp
 create mode 100644 gdb/testsuite/gdb.base/bg-execution-repeat.c
 create mode 100644 gdb/testsuite/gdb.base/bg-execution-repeat.exp
 create mode 100644 gdb/testsuite/gdb.base/continue-all-already-running.c
 create mode 100644 gdb/testsuite/gdb.base/continue-all-already-running.exp

-- 
1.9.3

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

[Pedro Alves]

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

[PATCH 2/4] PR gdb/17472: With annotations, input while executing in the foreground crashes readline/GDB

[Pedro Alves]

Jan caught an intermittent GDB crash with the annota1.exp test:

 Starting program: .../gdb/testsuite/gdb.base/annota1 ^M
 [...]
 FAIL: gdb.base/annota1.exp: run until main breakpoint (timeout)
 [...]
 readline: readline_callback_read_char() called with no handler!^M
 ERROR: Process no longer exists

All we need to is to continue the inferior in the foreground, and type
a command while the inferior is running.  E.g.:

 (gdb) set annotate 2

 â–’â–’pre-prompt
 (gdb)
 â–’â–’prompt
 c

 â–’â–’post-prompt
 Continuing.

 â–’â–’starting

 â–’â–’frames-invalid

 *inferior is running now*

 p 1<ret>

 readline: readline_callback_read_char() called with no handler!
 Aborted (core dumped)
 $


When we run a foreground execution command we call
target_terminal_inferior to stop GDB from processing input, and to put
the inferior's terminal settings in effect.  Then we tell readline to
hide the prompt with display_gdb_prompt, which clears readline's input
callback too.  When the target stops, we call target_terminal_ours,
which re-installs stdin in the event loop, and then we redisplay the
prompt, reinstalling the readline callbacks.

However, when annotations are in effect, the "frames-invalid"
annotation code calls target_terminal_ours after 'resume' had already
called target_terminal_inferior:

 (top-gdb) bt
 #0  0x000000000056b82f in annotate_frames_invalid () at gdb/annotate.c:219
 #1  0x000000000072e6cc in reinit_frame_cache () at gdb/frame.c:1705
 #2  0x0000000000594bb9 in registers_changed_ptid (ptid=...) at gdb/regcache.c:612
 #3  0x000000000064cca1 in target_resume (ptid=..., step=1, signal=GDB_SIGNAL_0) at gdb/target.c:2136
 #4  0x00000000005f57af in resume (step=1, sig=GDB_SIGNAL_0) at gdb/infrun.c:2263
 #5  0x00000000005f6051 in proceed (addr=18446744073709551615, siggnal=GDB_SIGNAL_DEFAULT, step=1) at gdb/infrun.c:2613

And then once we hide the prompt and remove readline's input handler
callback, we're in a bad state.  We end up with the target running
supposedly in the foreground, but with stdin still installed on the
event loop.  Any input then calls into readline, which aborts because
no rl_linefunc callback handler is installed:

 Program received signal SIGABRT, Aborted.
 0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 56        return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);

 (top-gdb) bt
 #0  0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  0x0000003b36a36f68 in __GI_abort () at abort.c:89
 During symbol reading, debug info gives source 9 included from file at zero line 0.
 During symbol reading, debug info gives command-line macro definition with non-zero line 19: _STDC_PREDEF_H 1.
 #2  0x0000000000784a25 in rl_callback_read_char () at src/readline/callback.c:116
 #3  0x0000000000619111 in rl_callback_read_char_wrapper (client_data=0x0) at src/gdb/event-top.c:167
 #4  0x00000000006194e7 in stdin_event_handler (error=0, client_data=0x0) at src/gdb/event-top.c:373
 #5  0x00000000006180da in handle_file_event (data=...) at src/gdb/event-loop.c:763
 #6  0x00000000006175c1 in process_event () at src/gdb/event-loop.c:340
 #7  0x0000000000617688 in gdb_do_one_event () at src/gdb/event-loop.c:404
 #8  0x00000000006176d8 in start_event_loop () at src/gdb/event-loop.c:429
 #9  0x0000000000619143 in cli_command_loop (data=0x0) at src/gdb/event-top.c:182
 #10 0x000000000060f4c8 in current_interp_command_loop () at src/gdb/interps.c:318
 #11 0x0000000000610691 in captured_command_loop (data=0x0) at src/gdb/main.c:323
 #12 0x000000000060c385 in catch_errors (func=0x610676 <captured_command_loop>, func_args=0x0, errstring=0x900241 "", mask=RETURN_MASK_ALL)
     at src/gdb/exceptions.c:237
 #13 0x0000000000611b8f in captured_main (data=0x7fffffffd7b0) at src/gdb/main.c:1151
 #14 0x000000000060c385 in catch_errors (func=0x610a8e <captured_main>, func_args=0x7fffffffd7b0, errstring=0x900241 "", mask=RETURN_MASK_ALL)
     at src/gdb/exceptions.c:237
 #15 0x0000000000611bb8 in gdb_main (args=0x7fffffffd7b0) at src/gdb/main.c:1159
 #16 0x000000000045ef57 in main (argc=3, argv=0x7fffffffd8b8) at src/gdb/gdb.c:32

The fix is to make the annotation code call target_terminal_inferior
again after printing, if the inferior's settings were in effect.

While at it, when we're doing output only, instead of
target_terminal_ours, we should call target_terminal_ours_for_output.
The latter doesn't actually remove stdin from the event loop, and also
leaves SIGINT forwarded to the target.

New test included.

Tested on x86_64 Fedora 20, native and gdbserver.

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

	PR gdb/17472
	* annotate.c (annotate_breakpoints_invalid): Use
	target_terminal_our_for_output instead of target_terminal_ours.
	Give back the terminal to the target.
	(annotate_frames_invalid): Likewise.

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

	PR gdb/17472
	* gdb.base/annota-input-while-running.c: New file.
	* gdb.base/annota-input-while-running.exp: New file.
---
 gdb/annotate.c                                     |  22 +++-
 gdb/target.c                                       |   8 ++
 gdb/target.h                                       |   5 +
 .../gdb.base/annota-input-while-running.c          |  25 ++++
 .../gdb.base/annota-input-while-running.exp        | 130 +++++++++++++++++++++
 5 files changed, 188 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/annota-input-while-running.c
 create mode 100644 gdb/testsuite/gdb.base/annota-input-while-running.exp

diff --git a/gdb/annotate.c b/gdb/annotate.c
index 6cce693..97e2b2b 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -72,8 +72,17 @@ annotate_breakpoints_invalid (void)
       && (!breakpoints_invalid_emitted
 	  || async_background_execution_p ()))
     {
-      target_terminal_ours ();
+      /* If the inferior owns the terminal (e.g., we're resuming),
+	 make sure to leave with the inferior still owning it.  */
+      int was_inferior = target_terminal_is_inferior ();
+
+      target_terminal_ours_for_output ();
+
       printf_unfiltered (("\n\032\032breakpoints-invalid\n"));
+
+      if (was_inferior)
+	target_terminal_inferior ();
+
       breakpoints_invalid_emitted = 1;
     }
 }
@@ -210,8 +219,17 @@ annotate_frames_invalid (void)
       && (!frames_invalid_emitted
 	  || async_background_execution_p ()))
     {
-      target_terminal_ours ();
+      /* If the inferior owns the terminal (e.g., we're resuming),
+	 make sure to leave with the inferior still owning it.  */
+      int was_inferior = target_terminal_is_inferior ();
+
+      target_terminal_ours_for_output ();
+
       printf_unfiltered (("\n\032\032frames-invalid\n"));
+
+      if (was_inferior)
+	target_terminal_inferior ();
+
       frames_invalid_emitted = 1;
     }
 }
diff --git a/gdb/target.c b/gdb/target.c
index bcc736b..c1eb869 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -461,6 +461,14 @@ target_terminal_init (void)
 
 /* See target.h.  */
 
+int
+target_terminal_is_inferior (void)
+{
+  return (terminal_state == terminal_is_inferior);
+}
+
+/* See target.h.  */
+
 void
 target_terminal_inferior (void)
 {
diff --git a/gdb/target.h b/gdb/target.h
index c1ab124..dc5a1f3 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1380,6 +1380,11 @@ extern int target_insert_breakpoint (struct gdbarch *gdbarch,
 extern int target_remove_breakpoint (struct gdbarch *gdbarch,
 				     struct bp_target_info *bp_tgt);
 
+/* Returns true if the terminal settings of the inferior are in
+   effect.  */
+
+extern int target_terminal_is_inferior (void);
+
 /* Initialize the terminal settings we record for the inferior,
    before we actually run the inferior.  */
 
diff --git a/gdb/testsuite/gdb.base/annota-input-while-running.c b/gdb/testsuite/gdb.base/annota-input-while-running.c
new file mode 100644
index 0000000..3db1399
--- /dev/null
+++ b/gdb/testsuite/gdb.base/annota-input-while-running.c
@@ -0,0 +1,25 @@
+/* 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
+main (void)
+{
+  sleep (5);
+  return 0; /* set break here */
+}
diff --git a/gdb/testsuite/gdb.base/annota-input-while-running.exp b/gdb/testsuite/gdb.base/annota-input-while-running.exp
new file mode 100644
index 0000000..1375718
--- /dev/null
+++ b/gdb/testsuite/gdb.base/annota-input-while-running.exp
@@ -0,0 +1,130 @@
+# Copyright 1999-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 annotations support doesn't leave GDB's terminal settings
+# into effect when we run a foreground command.
+
+if [is_remote target] then {
+    # We cannot use runto_main because of the different prompt we get
+    # when using annotation level 2.
+    return 0
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+# Break at main
+
+gdb_test "break main" \
+    "Breakpoint.*at.* file .*$srcfile.*\\." \
+    "breakpoint main"
+
+# NOTE: this prompt is OK only when the annotation level is > 1
+# NOTE: When this prompt is in use the gdb_test procedure cannot be
+# used because it assumes that the last char after the gdb_prompt is a
+# white space.  This is not true with this annotated prompt.  So we
+# must use the gdb_annota_test replacement below, or
+# gdb_test_multiple.
+
+set old_gdb_prompt $gdb_prompt
+set gdb_prompt "\r\n\032\032pre-prompt\r\n$gdb_prompt \r\n\032\032prompt\r\n"
+
+# Like gdb_test, but cope with the annotation prompt.
+proc gdb_annota_test {command pattern message} {
+    global gdb_prompt
+
+    gdb_test_multiple $command $message {
+	-re "$pattern$gdb_prompt$" {
+	    pass "$message"
+	}
+	-re "$gdb_prompt$" {
+	    fail "$message"
+	}
+    }
+}
+
+# Set the annotation level to 2.
+
+set test "annotation set at level 2"
+gdb_annota_test "set annotate 2" ".*" "annotation set at level 2"
+
+# Run to main.
+
+gdb_annota_test "run" \
+    "\r\n\032\032post-prompt.*\r\n\r\n\032\032stopped.*" \
+    "run until main breakpoint"
+
+set test "delete breakpoints"
+gdb_test_multiple "delete" $test {
+    -re "Delete all breakpoints. .y or n." {
+	send_gdb "y\n"
+	exp_continue
+    }
+    -re "$gdb_prompt$" {
+	pass $test
+    }
+}
+
+# Set the target running, and then type something.  GDB used to have a
+# bug where it'd be accepting input even though the target was
+# supposedly resumed in the foreground.  This ultimately resulted in
+# readline aborting.
+
+set linenum [gdb_get_line_number "set break here"]
+
+gdb_annota_test "break $linenum" \
+    "Breakpoint .*$srcfile, line .*" \
+    "break after sleep"
+
+# Continue, and wait a bit to make sure the inferior really starts
+# running.  Wait less than much the program sleeps, which is 5
+# seconds, though.
+set saw_continuing 0
+set test "continue"
+gdb_test_multiple $test $test {
+    -timeout 2
+    -re "Continuing\\." {
+	set saw_continuing 1
+	exp_continue
+    }
+    timeout {
+	gdb_assert $saw_continuing $test
+    }
+}
+
+# Type something.
+send_gdb "print 1\n"
+
+# Poor buggy GDB would crash before the breakpoint was hit.
+set test "breakpoint hit"
+gdb_test_multiple "" $test {
+    -re "stopped\r\n$gdb_prompt" {
+	pass $test
+    }
+}
+
+set test "print command result"
+gdb_test_multiple "" $test {
+    -re "\r\n1\r\n\r\n\032\032value-history-end\r\n$gdb_prompt" {
+	pass $test
+    }
+}
+
+# Restore the original prompt for the rest of the testsuite.
+
+set gdb_prompt $old_gdb_prompt
-- 
1.9.3

[PATCH 1/4] Make common code handle target_terminal_* idempotency

[Pedro Alves]

I found a place that should be giving back the terminal to the target,
but only if the target was already owning it.  So I need to add a
getter for who owns the terminal.

The trouble is that several places/target have their own globals to
track this state:

 - inflow.c:terminal_is_ours
 - remote.c:remote_async_terminal_ours_p
 - linux-nat.c:async_terminal_is_ours
 - go32-nat.c:terminal_is_ours

While one might think of adding a new target_ops method to query this,
conceptually, this state isn't really part of a particular target_ops.
Considering multi-target, the core shouldn't have to ask all targets
to know whether it's GDB that owns the terminal.  There's only one GDB
(or rather, only one top level interpreter).

So what this comment does is add a new global that is tracked by the
core instead.  A subsequent pass may later remove the other globals.

Tested on x86_64 Fedora 20, native and gdbserver.

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

	* target.c (enum terminal_state): New enum.
	(terminal_state): New global.
	(target_terminal_init): New function.
	(target_terminal_inferior): Skip if inferior already owns the
	terminal.
	(target_terminal_ours, target_terminal_ours_for_output): New
	functions.
	* target.h (target_terminal_init): Convert to function prototype.
	(target_terminal_ours_for_output): Convert to function prototype
	and tweak comment.
	(target_terminal_ours): Convert to function prototype and tweak
	comment.
	* windows-nat.c (do_initial_windows_stuff): Call
	target_terminal_init instead of child_terminal_init_with_pgrp.
---
 gdb/target.c      | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.h      | 20 +++++++-------------
 gdb/windows-nat.c |  2 +-
 3 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/gdb/target.c b/gdb/target.c
index 34db652..bcc736b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -432,6 +432,35 @@ target_load (const char *arg, int from_tty)
   (*current_target.to_load) (&current_target, arg, from_tty);
 }
 
+/* Possible terminal states.  */
+
+enum terminal_state
+  {
+    /* The inferior's terminal settings are in effect.  */
+    terminal_is_inferior = 0,
+
+    /* Some of our terminal settings are in effect, enough to get
+       proper output.  */
+    terminal_is_ours_for_output = 1,
+
+    /* Our terminal settings are in effect, for output and input.  */
+    terminal_is_ours = 2
+  };
+
+static enum terminal_state terminal_state;
+
+/* See target.h.  */
+
+void
+target_terminal_init (void)
+{
+  (*current_target.to_terminal_init) (&current_target);
+
+  terminal_state = terminal_is_ours;
+}
+
+/* See target.h.  */
+
 void
 target_terminal_inferior (void)
 {
@@ -442,9 +471,36 @@ target_terminal_inferior (void)
   if (target_can_async_p () && !sync_execution)
     return;
 
+  if (terminal_state == terminal_is_inferior)
+    return;
+
   /* If GDB is resuming the inferior in the foreground, install
      inferior's terminal modes.  */
   (*current_target.to_terminal_inferior) (&current_target);
+  terminal_state = terminal_is_inferior;
+}
+
+/* See target.h.  */
+
+void
+target_terminal_ours (void)
+{
+  if (terminal_state == terminal_is_ours)
+    return;
+
+  (*current_target.to_terminal_ours) (&current_target);
+  terminal_state = terminal_is_ours;
+}
+
+/* See target.h.  */
+
+void
+target_terminal_ours_for_output (void)
+{
+  if (terminal_state != terminal_is_inferior)
+    return;
+  (*current_target.to_terminal_ours_for_output) (&current_target);
+  terminal_state = terminal_is_ours_for_output;
 }
 
 /* See target.h.  */
diff --git a/gdb/target.h b/gdb/target.h
index a679228..c1ab124 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1383,31 +1383,25 @@ extern int target_remove_breakpoint (struct gdbarch *gdbarch,
 /* Initialize the terminal settings we record for the inferior,
    before we actually run the inferior.  */
 
-#define target_terminal_init() \
-     (*current_target.to_terminal_init) (&current_target)
+extern void target_terminal_init (void);
 
 /* Put the inferior's terminal settings into effect.
    This is preparation for starting or resuming the inferior.  */
 
 extern void target_terminal_inferior (void);
 
-/* Put some of our terminal settings into effect,
-   enough to get proper results from our output,
-   but do not change into or out of RAW mode
-   so that no input is discarded.
+/* Put some of our terminal settings into effect, enough to get proper
+   results from our output, but do not change into or out of RAW mode
+   so that no input is discarded.  This is a no-op if terminal_ours
+   was most recently called.  */
 
-   After doing this, either terminal_ours or terminal_inferior
-   should be called to get back to a normal state of affairs.  */
-
-#define target_terminal_ours_for_output() \
-     (*current_target.to_terminal_ours_for_output) (&current_target)
+extern void target_terminal_ours_for_output (void);
 
 /* Put our terminal settings into effect.
    First record the inferior's terminal settings
    so they can be restored properly later.  */
 
-#define target_terminal_ours() \
-     (*current_target.to_terminal_ours) (&current_target)
+extern void target_terminal_ours (void);
 
 /* Return true if the target stack has a non-default
   "to_terminal_ours" method.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 754a2d1..0f98793 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1741,7 +1741,7 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
      current thread until we report an event out of windows_wait.  */
   inferior_ptid = pid_to_ptid (pid);
 
-  child_terminal_init_with_pgrp (pid);
+  target_terminal_init ();
   target_terminal_inferior ();
 
   windows_initialization_done = 0;
-- 
1.9.3

[PATCH 3/4] PR gdb/17300: Input after "c -a" crashes readline/GDB

[Pedro Alves]

If all threads in the target were already running when the user does
"c -a", nothing puts the inferior's terminal settings in effect and
removes stdin from the event loop, which we must when running a
foreground command.  The result is that user input afterwards crashes
readline/gdb:

 (gdb) start
 Temporary breakpoint 1 at 0x4005d4: file continue-all-already-running.c, line 23.
 Starting program: continue-all-already-running

 Temporary breakpoint 1, main () at continue-all-already-running.c:23
 23        sleep (10);
 (gdb) c -a&
 Continuing.
 (gdb) c -a
 Continuing.
 p 1
 readline: readline_callback_read_char() called with no handler!
 Aborted (core dumped)
 $

Backtrace:

 Program received signal SIGABRT, Aborted.
 0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 56        return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
 (top-gdb) p 1
 $1 = 1
 (top-gdb) bt
 #0  0x0000003b36a35877 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
 #1  0x0000003b36a36f68 in __GI_abort () at abort.c:89
 #2  0x0000000000784aa9 in rl_callback_read_char () at readline/callback.c:116
 #3  0x0000000000619181 in rl_callback_read_char_wrapper (client_data=0x0) at gdb/event-top.c:167
 #4  0x0000000000619557 in stdin_event_handler (error=0, client_data=0x0) at gdb/event-top.c:373
 #5  0x000000000061814a in handle_file_event (data=...) at gdb/event-loop.c:763
 #6  0x0000000000617631 in process_event () at gdb/event-loop.c:340
 #7  0x00000000006176f8 in gdb_do_one_event () at gdb/event-loop.c:404
 #8  0x0000000000617748 in start_event_loop () at gdb/event-loop.c:429
 #9  0x00000000006191b3 in cli_command_loop (data=0x0) at gdb/event-top.c:182
 #10 0x000000000060f538 in current_interp_command_loop () at gdb/interps.c:318
 #11 0x0000000000610701 in captured_command_loop (data=0x0) at gdb/main.c:323
 #12 0x000000000060c3f5 in catch_errors (func=0x6106e6 <captured_command_loop>, func_args=0x0, errstring=0x9002c1 "", mask=RETURN_MASK_ALL)
     at gdb/exceptions.c:237
 #13 0x0000000000611bff in captured_main (data=0x7fffffffd780) at gdb/main.c:1151
 #14 0x000000000060c3f5 in catch_errors (func=0x610afe <captured_main>, func_args=0x7fffffffd780, errstring=0x9002c1 "", mask=RETURN_MASK_ALL)
     at gdb/exceptions.c:237
 #15 0x0000000000611c28 in gdb_main (args=0x7fffffffd780) at gdb/main.c:1159
 #16 0x000000000045ef97 in main (argc=5, argv=0x7fffffffd888) at gdb/gdb.c:32
 (top-gdb)

Tested on x86_64 Fedora 20, native and gdbserver.

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

	PR gdb/17300
	* infcmd.c (continue_1): If continuing all threads in the
	foreground, make sure the inferior's terminal settings are put in
	effect.

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

	PR gdb/17300
	* gdb.base/continue-all-already-running.c: New file.
	* gdb.base/continue-all-already-running.exp: New file.
---
 gdb/infcmd.c                                       | 18 +++++
 .../gdb.base/continue-all-already-running.c        | 25 +++++++
 .../gdb.base/continue-all-already-running.exp      | 79 ++++++++++++++++++++++
 3 files changed, 122 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/continue-all-already-running.c
 create mode 100644 gdb/testsuite/gdb.base/continue-all-already-running.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 0374533..d270664 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -735,6 +735,24 @@ continue_1 (int all_threads)
 
       iterate_over_threads (proceed_thread_callback, NULL);
 
+      if (sync_execution)
+	{
+	  /* If all threads in the target were already running,
+	     proceed_thread_callback ends up never calling proceed,
+	     and so nothing calls this to put the inferior's terminal
+	     settings in effect and remove stdin from the event loop,
+	     which we must when running a foreground command.  E.g.:
+
+	      (gdb) c -a&
+	      Continuing.
+	      <all threads are running now>
+	      (gdb) c -a
+	      Continuing.
+	      <no thread was resumed, but the inferior now owns the terminal>
+	  */
+	  target_terminal_inferior ();
+	}
+
       /* Restore selected ptid.  */
       do_cleanups (old_chain);
     }
diff --git a/gdb/testsuite/gdb.base/continue-all-already-running.c b/gdb/testsuite/gdb.base/continue-all-already-running.c
new file mode 100644
index 0000000..a6516d5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/continue-all-already-running.c
@@ -0,0 +1,25 @@
+/* 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
+main (void)
+{
+  sleep (10);
+  return 0; /* set break here */
+}
diff --git a/gdb/testsuite/gdb.base/continue-all-already-running.exp b/gdb/testsuite/gdb.base/continue-all-already-running.exp
new file mode 100644
index 0000000..6dde730
--- /dev/null
+++ b/gdb/testsuite/gdb.base/continue-all-already-running.exp
@@ -0,0 +1,79 @@
+# 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 "c -a" doesn't leave GDB processing input, even if all
+# threads were already running.  PR gdb/17300.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } {
+    return -1
+}
+
+gdb_test_no_output "set non-stop on"
+
+if ![runto_main] {
+    return
+}
+
+set linenum [gdb_get_line_number "set break here"]
+gdb_breakpoint "$linenum"
+
+gdb_test "c -a&" "Continuing\\."
+
+set test "no stop"
+gdb_test_multiple "" $test {
+    -timeout 1
+    timeout {
+	pass $test
+    }
+}
+
+# Paranoia.  Check that input works after bg command.
+gdb_test "print 1" " = 1"
+
+# Continue in the foreground, and wait one second to make sure the
+# inferior really starts running.  If we get a prompt to soon (e.g.,
+# the program stops), this issues a fail.
+set saw_continuing 0
+set test "c -a"
+gdb_test_multiple "c -a" $test {
+    -timeout 1
+    -re "Continuing\\." {
+	set saw_continuing 1
+	exp_continue
+    }
+    timeout {
+	gdb_assert $saw_continuing $test
+    }
+}
+
+# Type something while the inferior is running in the foreground.
+send_gdb "print 2\n"
+
+# Poor buggy GDB would crash before the breakpoint was hit.
+set test "breakpoint hit"
+gdb_test_multiple "" $test {
+    -re "set break here ..\r\n$gdb_prompt " {
+	pass $test
+    }
+}
+
+set test "print command result"
+gdb_test_multiple "" $test {
+    -re " = 2\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
-- 
1.9.3

[pushed] Re: [PATCH 0/4] Fix a set of terminal/readline handling bugs/crashes

[Pedro Alves]

On 10/09/2014 07:00 PM, Pedro Alves wrote:
> This series fixes a couple terminal/readline handling bugs that result
> in GDB crashing with:
> 
>  readline: readline_callback_read_char() called with no handler!
>  Aborted (core dumped)
>  $
> 
> ... and also a bug where we lose the "&" when repeating background
> commands.
> 
> Because the these won't trigger without target-async, the first two
> are user-visible regressions in 7.8, which has async by default (7.7
> didn't).  As such, I'd like to include these in 7.8.1.
> 
> For last one (gdb/17471), I'm a bit borderline (re. 7.8.1).
> Technically, it isn't really a regression, but still annoying (see
> gdb/17300).  If you'd prefer not having that in 7.8.1, please do speak
> up.

I pushed the whole series to master and the 7.8 branch.

Thanks,
Pedro Alves

[patch] ASAN attach crash - 7.9 regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

On Thu, 09 Oct 2014 20:00:29 +0200, Pedro Alves wrote:
> Tested on x86_64 Fedora 20, native and gdbserver.

-fsanitize=address
gdb.base/attach-pie-noexec.exp

==32586==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200004ed90 at pc 0x48ad50 bp 0x7ffceb3aef50 sp 0x7ffceb3aef20
READ of size 2 at 0x60200004ed90 thread T0
    #0 0x48ad4f in __interceptor_strlen (/home/jkratoch/redhat/gdb-test-asan/gdb/gdb+0x48ad4f)
    #1 0xeafe5c in xstrdup xstrdup.c:33
    #2 0x85e024 in attach_command /home/jkratoch/redhat/gdb-test-asan/gdb/infcmd.c:2680

regressed by:

commit 6c4486e63f7583ed85a0c72841f6ccceebbf858e
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Oct 17 13:31:26 2014 +0100
    PR gdb/17471: Repeating a background command makes it foreground


OK for check-in and for 7.10?


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 1042 bytes --]

2015-08-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR gdb/18767
	* infcmd.c (attach_command): Move ARGS_CHAIN cleanup after last ARGS
	use.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 4948d27..5cd8dd7 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2617,9 +2617,6 @@ 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 ();
@@ -2684,12 +2681,19 @@ attach_command (char *args, int from_tty)
 	  a->async_exec = async_exec;
 	  add_inferior_continuation (attach_command_continuation, a,
 				     attach_command_continuation_free_args);
+
+	  /* Done with ARGS.  */
+	  do_cleanups (args_chain);
+
 	  return;
 	}
 
       wait_for_inferior ();
     }
 
+  /* Done with ARGS.  */
+  do_cleanups (args_chain);
+
   attach_command_post_wait (args, from_tty, async_exec);
 }
 

[patch] signal_command: Leftover cleanup chain regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

Hi,

mostly harmless but not correct.


Jan

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 604 bytes --]

gdb/ChangeLog
2015-08-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* infcmd.c (signal_command): Call do_cleanups for args_chain.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 4948d27..e4b2045 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1264,6 +1264,8 @@ signal_command (char *signum_exp, int from_tty)
 	oursig = gdb_signal_from_command (num);
     }
 
+  do_cleanups (args_chain);
+
   /* Look for threads other than the current that this command ends up
      resuming too (due to schedlock off), and warn if they'll get a
      signal delivered.  "signal 0" is used to suppress a previous

Re: [patch] ASAN attach crash - 7.9 regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Pedro Alves]

On 08/03/2015 10:02 PM, Jan Kratochvil wrote:
> On Thu, 09 Oct 2014 20:00:29 +0200, Pedro Alves wrote:
>> Tested on x86_64 Fedora 20, native and gdbserver.
> 
> -fsanitize=address
> gdb.base/attach-pie-noexec.exp
> 
> ==32586==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200004ed90 at pc 0x48ad50 bp 0x7ffceb3aef50 sp 0x7ffceb3aef20
> READ of size 2 at 0x60200004ed90 thread T0
>     #0 0x48ad4f in __interceptor_strlen (/home/jkratoch/redhat/gdb-test-asan/gdb/gdb+0x48ad4f)
>     #1 0xeafe5c in xstrdup xstrdup.c:33
>     #2 0x85e024 in attach_command /home/jkratoch/redhat/gdb-test-asan/gdb/infcmd.c:2680
> 
> regressed by:
> 
> commit 6c4486e63f7583ed85a0c72841f6ccceebbf858e
> Author: Pedro Alves <palves@redhat.com>
> Date:   Fri Oct 17 13:31:26 2014 +0100
>     PR gdb/17471: Repeating a background command makes it foreground
> 
> 
> OK for check-in and for 7.10?

OK.

Thanks,
Pedro Alves

Re: [patch] signal_command: Leftover cleanup chain regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Pedro Alves]

On 08/04/2015 09:28 AM, Jan Kratochvil wrote:
> Hi,
> 
> mostly harmless but not correct.

OK.

Thanks,
Pedro Alves

[commit+7.10] ASAN attach crash - 7.9 regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

On Tue, 04 Aug 2015 10:35:43 +0200, Pedro Alves wrote:
> OK.

master
	978b9495b78054b76052a09064cae8c94a58b93e
7.10
	be18bd2bb2f68cd62b94f5a176e55440c73d25b8


Jan

[commit+7.10] signal_command: Leftover cleanup chain regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

On Tue, 04 Aug 2015 10:37:10 +0200, Pedro Alves wrote:
> OK.

master
	c6343a91d94e9516afe56dfe85e435922bd9ea04
7.10
	9b811c1bcd75c471d799a05122e3ae140b2a34ae


Jan

Re: [commit+7.10] ASAN attach crash - 7.9 regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

On Tue, 04 Aug 2015 13:48:26 +0200, Jan Kratochvil wrote:
> On Tue, 04 Aug 2015 10:35:43 +0200, Pedro Alves wrote:
> > OK.
> 
> master
> 	978b9495b78054b76052a09064cae8c94a58b93e
> 7.10
> 	be18bd2bb2f68cd62b94f5a176e55440c73d25b8

That 7.10 was not pushed, so it is pushed to 7.10 now as:
	fa68327bb429223d98887fa43db67fbb49629eb1


Jan

Re: [commit+7.10] signal_command: Leftover cleanup chain regression [Re: [PATCH 4/4] PR gdb/17471: Repeating a background command makes it foreground]

[Jan Kratochvil]

On Tue, 04 Aug 2015 13:48:59 +0200, Jan Kratochvil wrote:
> On Tue, 04 Aug 2015 10:37:10 +0200, Pedro Alves wrote:
> > OK.
> 
> master
> 	c6343a91d94e9516afe56dfe85e435922bd9ea04
> 7.10
> 	9b811c1bcd75c471d799a05122e3ae140b2a34ae

That 7.10 was not pushed, so it is pushed to 7.10 now as:
	3ba0344e56ef739808615be5ca319f82c2a83855


Jan