http://sourceware.org/ml/gdb-patches/2014-09/msg00102.html

Subject: Re: Regression: GDB stopped on run with attached process (PR 17347)  [Re: [pushed+7.8] Re: [PATCH] Fix "attach" command vs user input race

On 09/03/2014 08:58 AM, Jan Kratochvil wrote:

> https://sourceware.org/bugzilla/show_bug.cgi?id=17347

Thanks Jan.

Here's a fix, test included.  Comments?

Thanks,

Pedro Alves

--------------------------

[PATCH] gdb/17347 - Regression: GDB stopped on run with attached process

Doing:

  gdb --pid=PID -ex run

Results in GDB getting a SIGTTIN, and thus ending stopped.  That's

usually indicative of a missing target_terminal_ours call.

E.g., from the PR:

 $ sleep 1h & p=$!; sleep 0.1; gdb -batch sleep $p -ex run

 [1] 28263

 [1]   Killed                  sleep 1h

 [2]+  Stopped                 gdb -batch sleep $p -ex run

The workaround is doing:

 gdb -ex "attach $PID" -ex "run"

instead of

 gdb [-p] $PID -ex "run"

With the former, gdb waits for the attach command to complete before

moving on to the "run" command, because the interpreter is in sync

mode at this point, within execute_command.  But for the latter,

attach_command is called directly from captured_main, and thus misses

that waiting.  IOW, "run" is running before the attach continuation

has run, before the program stops and attach completes.  The broken

terminal settings are just one symptom of that.  Any command that

queries or requires input results in the same.

The fix is to wait in catch_command_errors (which is specific to

main.c nowadays), just like we wait in execute_command.

gdb/ChangeLog:

2014-09-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17347

	* main.c: Include "infrun.h".

	(catch_command_errors, catch_command_errors_const): Wait for the

	foreground command to complete.

	* top.c (maybe_wait_sync_command_done): New function, factored out

	from ...

	(maybe_wait_sync_command_done): ... here.

	* top.h (maybe_wait_sync_command_done): New declaration.

gdb/testsuite/ChangeLog:

2014-09-03  Pedro Alves  <palves@redhat.com>

	PR gdb/17347

	* gdb.base/attach.exp (spawn_test_prog): New, factored out from

	...

	(do_attach_tests, do_call_attach_tests, do_command_attach_tests):

	... here.

	(gdb_spawn_with_cmdline_opts): New procedure.

	(test_command_line_attach_run): New procedure.

	(top level): Call it.

---

 gdb/main.c                        |   9 +++

 gdb/testsuite/gdb.base/attach.exp | 118 ++++++++++++++++++++++++++------------

 gdb/top.c                         |  26 +++++----

 gdb/top.h                         |   8 +++

 4 files changed, 114 insertions(+), 47 deletions(-)

Index: gdb-7.8/gdb/main.c

===================================================================

--- gdb-7.8.orig/gdb/main.c	2014-09-07 19:12:45.066981588 +0200

+++ gdb-7.8/gdb/main.c	2014-09-07 19:14:22.613095201 +0200

@@ -47,6 +47,7 @@

 #include "filenames.h"

 #include "filestuff.h"

 #include "event-top.h"

+#include "infrun.h"

 /* The selected interpreter.  This will be used as a set command

    variable, so it should always be malloc'ed - since

@@ -350,7 +351,11 @@ catch_command_errors (catch_command_erro

   TRY_CATCH (e, mask)

     {

+      int was_sync = sync_execution;

+

       command (arg, from_tty);

+

+      maybe_wait_sync_command_done (was_sync);

     }

   return handle_command_errors (e);

 }

@@ -369,7 +374,11 @@ catch_command_errors_const (catch_comman

   TRY_CATCH (e, mask)

     {

+      int was_sync = sync_execution;

+

       command (arg, from_tty);

+

+      maybe_wait_sync_command_done (was_sync);

     }

   return handle_command_errors (e);

 }

Index: gdb-7.8/gdb/testsuite/gdb.base/attach.exp

===================================================================

--- gdb-7.8.orig/gdb/testsuite/gdb.base/attach.exp	2014-09-07 19:12:45.067981589 +0200

+++ gdb-7.8/gdb/testsuite/gdb.base/attach.exp	2014-09-07 19:12:48.601985706 +0200

@@ -58,6 +58,37 @@ if [get_compiler_info] {

     return -1

 }

+# Start the program running and then wait for a bit, to be sure that

+# it can be attached to.  Return the process's PID.

+

+proc spawn_test_prog { executable } {

+    set testpid [eval exec $executable &]

+    exec sleep 2

+    if { [istarget "*-*-cygwin*"] } {

+	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be

+	# different due to the way fork/exec works.

+	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]

+    }

+

+    return $testpid

+}

+

+# Spawn GDB with CMDLINE_FLAGS appended to the GDBFLAGS global.

+

+proc gdb_spawn_with_cmdline_opts { cmdline_flags } {

+    global GDBFLAGS

+

+    set saved_gdbflags $GDBFLAGS

+

+    append GDBFLAGS $cmdline_flags

+

+    set res [gdb_spawn]

+

+    set GDBFLAGS $saved_gdbflags

+

+    return $res

+}

+

 proc do_attach_tests {} {

     global gdb_prompt

     global binfile

@@ -70,13 +101,7 @@ proc do_attach_tests {} {

     # Start the program running and then wait for a bit, to be sure

     # that it can be attached to.

-    set testpid [eval exec $binfile &]

-    exec sleep 2

-    if { [istarget "*-*-cygwin*"] } {

-	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be

-	# different due to the way fork/exec works.

-	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]

-    }

+    set testpid [spawn_test_prog $binfile]

     # Verify that we cannot attach to nonsense.

@@ -279,16 +304,7 @@ proc do_attach_tests {} {

     remote_exec build "kill -9 ${testpid}"

-    # Start the program running and then wait for a bit, to be sure

-    # that it can be attached to.

-   

-    set testpid [eval exec $binfile &]

-    exec sleep 2

-    if { [istarget "*-*-cygwin*"] } {

-	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be

-	# different due to the way fork/exec works.

-	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]

-    }

+    set testpid [spawn_test_prog $binfile]

     # Verify that we can attach to the process, and find its a.out

     # when we're cd'd to some directory that doesn't contain the

@@ -335,16 +351,7 @@ proc do_call_attach_tests {} {

     global gdb_prompt

     global binfile2

-    # Start the program running and then wait for a bit, to be sure

-    # that it can be attached to.

-   

-    set testpid [eval exec $binfile2 &]

-    exec sleep 2

-    if { [istarget "*-*-cygwin*"] } {

-	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be

-	# different due to the way fork/exec works.

-	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]

-    }

+    set testpid [spawn_test_prog $binfile2]

     # Attach

@@ -397,16 +404,7 @@ proc do_command_attach_tests {} {

 	return 0

     }

-    # Start the program running and then wait for a bit, to be sure

-    # that it can be attached to.

-

-    set testpid [eval exec $binfile &]

-    exec sleep 2

-    if { [istarget "*-*-cygwin*"] } {

-	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be

-	# different due to the way fork/exec works.

-	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]

-    }

+    set testpid [spawn_test_prog $binfile]

     gdb_exit

     if $verbose>1 then {

@@ -429,6 +427,50 @@ proc do_command_attach_tests {} {

     remote_exec build "kill -9 ${testpid}"

 }

+# Test ' gdb --pid PID -ex "run" '.  GDB used to have a bug where

+# "run" would run before the attach finished - PR17347.

+

+proc test_command_line_attach_run {} {

+    global gdb_prompt

+    global binfile

+    global verbose

+    global GDB

+    global INTERNAL_GDBFLAGS

+

+    if ![isnative] then {

+	unsupported "commandline attach run test"

+	return 0

+    }

+

+    with_test_prefix "cmdline attach run" {

+	set testpid [spawn_test_prog $binfile]

+

+	set test "run to prompt"

+	gdb_exit

+	set res [gdb_spawn_with_cmdline_opts "--pid=$testpid -ex \"start\""]

+	if { $res != 0} {

+	    fail $test

+	    return $res

+	}

+	gdb_test_multiple "" $test {

+	    -re {Attaching to.*Start it from the beginning\? \(y or n\) } {

+		pass $test

+	    }

+	}

+

+	send_gdb "y\n"

+

+	set test "run to main"

+	gdb_test_multiple "" $test {

+	    -re "Temporary breakpoint .* main .*$gdb_prompt $" {

+		pass $test

+	    }

+	}

+

+	# Get rid of the process

+	remote_exec build "kill -9 ${testpid}"

+    }

+}

 # Start with a fresh gdb

@@ -453,4 +495,6 @@ do_call_attach_tests

 do_command_attach_tests

+test_command_line_attach_run

+

 return 0

Index: gdb-7.8/gdb/top.c

===================================================================

--- gdb-7.8.orig/gdb/top.c	2014-09-07 19:12:45.067981589 +0200

+++ gdb-7.8/gdb/top.c	2014-09-07 19:12:48.601985706 +0200

@@ -375,6 +375,21 @@ check_frame_language_change (void)

     }

 }

+void

+maybe_wait_sync_command_done (int was_sync)

+{

+  /* If the interpreter is in sync mode (we're running a user

+     command's list, running command hooks or similars), and we

+     just ran a synchronous command that started the target, wait

+     for that command to end.  */

+  if (!interpreter_async && !was_sync && sync_execution)

+    {

+      while (gdb_do_one_event () >= 0)

+	if (!sync_execution)

+	  break;

+    }

+}

+

 /* Execute the line P as a command, in the current user context.

    Pass FROM_TTY as second argument to the defining function.  */

@@ -461,16 +476,7 @@ execute_command (char *p, int from_tty)

       else

 	cmd_func (c, arg, from_tty);

-      /* If the interpreter is in sync mode (we're running a user

-	 command's list, running command hooks or similars), and we

-	 just ran a synchronous command that started the target, wait

-	 for that command to end.  */

-      if (!interpreter_async && !was_sync && sync_execution)

-	{

-	  while (gdb_do_one_event () >= 0)

-	    if (!sync_execution)

-	      break;

-	}

+      maybe_wait_sync_command_done (was_sync);

       /* If this command has been post-hooked, run the hook last.  */

       execute_cmd_post_hook (c);

Index: gdb-7.8/gdb/top.h

===================================================================

--- gdb-7.8.orig/gdb/top.h	2014-09-07 19:12:45.068981590 +0200

+++ gdb-7.8/gdb/top.h	2014-09-07 19:12:48.601985706 +0200

@@ -42,6 +42,14 @@ extern void quit_command (char *, int);

 extern void quit_cover (void);

 extern void execute_command (char *, int);

+/* If the interpreter is in sync mode (we're running a user command's

+   list, running command hooks or similars), and we just ran a

+   synchronous command that started the target, wait for that command

+   to end.  WAS_SYNC indicates whether sync_execution was set before

+   the command was run.  */

+

+extern void maybe_wait_sync_command_done (int was_sync);

+

 extern void check_frame_language_change (void);

 /* Prepare for execution of a command.