Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Fix gdb.threads/process-dies-while-detaching.exp
@ 2017-11-17 18:38 Pedro Alves
  2017-11-23 19:51 ` Sergio Durigan Junior
  0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2017-11-17 18:38 UTC (permalink / raw)
  To: gdb-patches

I noticed [1] a test bug in gdb.threads/process-dies-while-detaching.exp.
Simplified, the test code in question looks somewhat like this:

~~~
  # Detach from a process, and ensure that it exits after detaching.
  # This relies on inferior I/O.

  proc detach_and_expect_exit {test} {

      gdb_test_multiple "detach" $test ....

      set saw_prompt 0
      set saw_inf_exit 0
      while { !$saw_prompt && !$saw_inf_exit } {
          gdb_test_multiple "" $test {
              -re "exited, status=0" {
                  set saw_inf_exit 1
              }
              -re "$gdb_prompt " {
                  set saw_prompt 1
              }
          }
      }

      pass $test
  }
~~~

The bug is in the while loop's condition.  We want to make sure we see
both the inferior output and the prompt, so the loop's test should be:

   -    while { !$saw_prompt && !$saw_inf_exit } {
   +    while { !$saw_prompt || !$saw_inf_exit } {

If we just fix that, the test starts failing though, because that
exposes a latent problem -- when called from
test_detach_killed_outside, the parent doesn't print "exited,
status=0", because in that case the child dies with a signal, and so
detach_and_expect_exit times out.  Fix it by making the parent print
"signaled, sig=9" in that case, and have the .exp expect it.

[1] I changed GDB in a way that should have made the test fail, but it
    didn't.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.threads/process-dies-while-detaching.c: Include <errno.h>
	and <string.h>.
	(parent_function): Print distinct messages when waitpid fails, or
	the child exits with a signal, or the child exits for an unhandled
	reason.
	* gdb.threads/process-dies-while-detaching.exp
	(detach_and_expect_exit): New 'inf_output_re' parameter and use
	it.  Wait for both inferior output and GDB's prompt.
	(do_detach): New parameter 'child_exit'.  Use it to compute
	expected inferior output.
	(test_detach, test_detach_watch, test_detach_killed_outside):
	Adjust to pass down the expected child exit kind.
---
 .../gdb.threads/process-dies-while-detaching.c     | 22 +++++++++--
 .../gdb.threads/process-dies-while-detaching.exp   | 43 +++++++++++++---------
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
index eecaaed..4ba50d4 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
@@ -22,6 +22,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <assert.h>
+#include <errno.h>
+#include <string.h>
 
 /* This barrier ensures we only reach the initial breakpoint after all
    threads have started.  */
@@ -78,15 +80,27 @@ parent_function (pid_t child)
   alarm (300);
 
   ret = waitpid (child, &status, 0);
+
   if (ret == -1)
-    exit (1);
-  else if (!WIFEXITED (status))
-    exit (2);
-  else
+    {
+      printf ("waitpid, errno=%d (%s)\n", errno, strerror (errno));
+      exit (1);
+    }
+  else if (WIFEXITED (status))
     {
       printf ("exited, status=%d\n", WEXITSTATUS (status));
       exit (0);
     }
+  else if (WIFSIGNALED (status))
+    {
+      printf ("signaled, sig=%d\n", WTERMSIG (status));
+      exit (2);
+    }
+  else
+    {
+      printf ("unexpected, status=%x\n", status);
+      exit (3);
+    }
 }
 
 #endif
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index 4cba80c..ea8f6e9 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -72,9 +72,10 @@ proc return_if_fail { result } {
 }
 
 # Detach from a process, and ensure that it exits after detaching.
-# This relies on inferior I/O.
+# This relies on inferior I/O.  INF_OUTPUT_RE is the pattern that
+# matches the expected inferior output.
 
-proc detach_and_expect_exit {test} {
+proc detach_and_expect_exit {inf_output_re test} {
     global decimal
     global gdb_spawn_id
     global inferior_spawn_id
@@ -87,7 +88,7 @@ proc detach_and_expect_exit {test} {
 
     set saw_prompt 0
     set saw_inf_exit 0
-    while { !$saw_prompt && ! $saw_inf_exit } {
+    while { !$saw_prompt || ! $saw_inf_exit } {
 	# We don't know what order the interesting things will arrive in.
 	# Using a pattern of the form 'x|y|z' instead of -re x ... -re y
 	# ... -re z ensures that expect always chooses the match that
@@ -96,7 +97,7 @@ proc detach_and_expect_exit {test} {
 	# we don't skip anything.
 	return_if_fail [gdb_test_multiple "" $test {
 	    -i "$inferior_spawn_id $gdb_spawn_id"
-	    -re "(exited, status=0)|($gdb_prompt )" {
+	    -re "($inf_output_re)|($gdb_prompt )" {
 		if {[info exists expect_out(1,string)]} {
 		    verbose -log "saw inferior exit"
 		    set saw_inf_exit 1
@@ -130,15 +131,28 @@ proc continue_to_exit_bp {} {
 #
 # CMD indicates what to do with the parent after detaching the child.
 # Can be either "detach" to detach, or "continue", to continue to
-# exit.  If "continue", then CONTINUE_RE is the regexp to expect.
-# Defaults to normal exit output.
+# exit.
 #
-proc do_detach {multi_process cmd {continue_re ""}} {
+# CHILD_EXIT indicates how is the child expected to exit.  Can be
+# either "normal" for normal exit, or "signal" for killed with signal
+# SIGKILL.
+#
+proc do_detach {multi_process cmd child_exit} {
     global decimal
     global server_spawn_id
 
-    if {$continue_re == ""} {
+    if {$child_exit == "normal"} {
 	set continue_re "exited normally.*"
+	set inf_output_re "exited, status=0"
+    } elseif {$child_exit == "signal"} {
+	if {$multi_process} {
+	    set continue_re "exited with code 02.*"
+	} else {
+	    set continue_re "terminated with signal SIGKILL.*"
+	}
+	set inf_output_re "signaled, sig=9"
+    } else {
+	error "unhandled \$child_exit: $child_exit"
     }
 
     set is_remote [expr {[target_info exists gdb_protocol]
@@ -154,7 +168,7 @@ proc do_detach {multi_process cmd {continue_re ""}} {
 	if {$cmd == "detach"} {
 	    # Make sure that detach works and that the parent process
 	    # exits cleanly.
-	    detach_and_expect_exit "detach parent"
+	    detach_and_expect_exit $inf_output_re "detach parent"
 	} elseif {$cmd == "continue"} {
 	    # Make sure that continuing works and that the parent process
 	    # exits cleanly.
@@ -205,7 +219,7 @@ proc test_detach {multi_process cmd} {
 	# Run to _exit in the child.
 	continue_to_exit_bp
 
-	do_detach $multi_process $cmd
+	do_detach $multi_process $cmd "normal"
     }
 }
 
@@ -240,7 +254,7 @@ proc test_detach_watch {multi_process cmd} {
 	# thread individually).
 	continue_to_exit_bp
 
-	do_detach $multi_process $cmd
+	do_detach $multi_process $cmd "normal"
     }
 }
 
@@ -279,12 +293,7 @@ proc test_detach_killed_outside {multi_process cmd} {
 	# Give it some time to die.
 	sleep 2
 
-	if {$multi_process} {
-	    set continue_re "exited with code 02.*"
-	} else {
-	    set continue_re "terminated with signal SIGKILL.*"
-	}
-	do_detach $multi_process $cmd $continue_re
+	do_detach $multi_process $cmd "signal"
     }
 }
 
-- 
2.5.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gdb.threads/process-dies-while-detaching.exp
  2017-11-17 18:38 [PATCH] Fix gdb.threads/process-dies-while-detaching.exp Pedro Alves
@ 2017-11-23 19:51 ` Sergio Durigan Junior
  2017-12-03 15:24   ` Pedro Alves
  0 siblings, 1 reply; 4+ messages in thread
From: Sergio Durigan Junior @ 2017-11-23 19:51 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Friday, November 17 2017, Pedro Alves wrote:

> I noticed [1] a test bug in gdb.threads/process-dies-while-detaching.exp.
> Simplified, the test code in question looks somewhat like this:
>
> ~~~
>   # Detach from a process, and ensure that it exits after detaching.
>   # This relies on inferior I/O.
>
>   proc detach_and_expect_exit {test} {
>
>       gdb_test_multiple "detach" $test ....
>
>       set saw_prompt 0
>       set saw_inf_exit 0
>       while { !$saw_prompt && !$saw_inf_exit } {
>           gdb_test_multiple "" $test {
>               -re "exited, status=0" {
>                   set saw_inf_exit 1
>               }
>               -re "$gdb_prompt " {
>                   set saw_prompt 1
>               }
>           }
>       }
>
>       pass $test
>   }
> ~~~
>
> The bug is in the while loop's condition.  We want to make sure we see
> both the inferior output and the prompt, so the loop's test should be:
>
>    -    while { !$saw_prompt && !$saw_inf_exit } {
>    +    while { !$saw_prompt || !$saw_inf_exit } {
>
> If we just fix that, the test starts failing though, because that
> exposes a latent problem -- when called from
> test_detach_killed_outside, the parent doesn't print "exited,
> status=0", because in that case the child dies with a signal, and so
> detach_and_expect_exit times out.  Fix it by making the parent print
> "signaled, sig=9" in that case, and have the .exp expect it.
>
> [1] I changed GDB in a way that should have made the test fail, but it
>     didn't.

I skimmed over the patch and it looks good to me, but you're the expert
in this area here so I trust your judgement more than mine ;-).

> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.threads/process-dies-while-detaching.c: Include <errno.h>
> 	and <string.h>.
> 	(parent_function): Print distinct messages when waitpid fails, or
> 	the child exits with a signal, or the child exits for an unhandled
> 	reason.
> 	* gdb.threads/process-dies-while-detaching.exp
> 	(detach_and_expect_exit): New 'inf_output_re' parameter and use
> 	it.  Wait for both inferior output and GDB's prompt.
> 	(do_detach): New parameter 'child_exit'.  Use it to compute
> 	expected inferior output.
> 	(test_detach, test_detach_watch, test_detach_killed_outside):
> 	Adjust to pass down the expected child exit kind.
> ---
>  .../gdb.threads/process-dies-while-detaching.c     | 22 +++++++++--
>  .../gdb.threads/process-dies-while-detaching.exp   | 43 +++++++++++++---------
>  2 files changed, 44 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
> index eecaaed..4ba50d4 100644
> --- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
> +++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
> @@ -22,6 +22,8 @@
>  #include <sys/types.h>
>  #include <sys/wait.h>
>  #include <assert.h>
> +#include <errno.h>
> +#include <string.h>
>  
>  /* This barrier ensures we only reach the initial breakpoint after all
>     threads have started.  */
> @@ -78,15 +80,27 @@ parent_function (pid_t child)
>    alarm (300);
>  
>    ret = waitpid (child, &status, 0);
> +
>    if (ret == -1)
> -    exit (1);
> -  else if (!WIFEXITED (status))
> -    exit (2);
> -  else
> +    {
> +      printf ("waitpid, errno=%d (%s)\n", errno, strerror (errno));
> +      exit (1);
> +    }
> +  else if (WIFEXITED (status))
>      {
>        printf ("exited, status=%d\n", WEXITSTATUS (status));
>        exit (0);
>      }
> +  else if (WIFSIGNALED (status))
> +    {
> +      printf ("signaled, sig=%d\n", WTERMSIG (status));
> +      exit (2);
> +    }
> +  else
> +    {
> +      printf ("unexpected, status=%x\n", status);
> +      exit (3);
> +    }
>  }
>  
>  #endif
> diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> index 4cba80c..ea8f6e9 100644
> --- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> +++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> @@ -72,9 +72,10 @@ proc return_if_fail { result } {
>  }
>  
>  # Detach from a process, and ensure that it exits after detaching.
> -# This relies on inferior I/O.
> +# This relies on inferior I/O.  INF_OUTPUT_RE is the pattern that
> +# matches the expected inferior output.
>  
> -proc detach_and_expect_exit {test} {
> +proc detach_and_expect_exit {inf_output_re test} {
>      global decimal
>      global gdb_spawn_id
>      global inferior_spawn_id
> @@ -87,7 +88,7 @@ proc detach_and_expect_exit {test} {
>  
>      set saw_prompt 0
>      set saw_inf_exit 0
> -    while { !$saw_prompt && ! $saw_inf_exit } {
> +    while { !$saw_prompt || ! $saw_inf_exit } {
>  	# We don't know what order the interesting things will arrive in.
>  	# Using a pattern of the form 'x|y|z' instead of -re x ... -re y
>  	# ... -re z ensures that expect always chooses the match that
> @@ -96,7 +97,7 @@ proc detach_and_expect_exit {test} {
>  	# we don't skip anything.
>  	return_if_fail [gdb_test_multiple "" $test {
>  	    -i "$inferior_spawn_id $gdb_spawn_id"
> -	    -re "(exited, status=0)|($gdb_prompt )" {
> +	    -re "($inf_output_re)|($gdb_prompt )" {
>  		if {[info exists expect_out(1,string)]} {
>  		    verbose -log "saw inferior exit"
>  		    set saw_inf_exit 1
> @@ -130,15 +131,28 @@ proc continue_to_exit_bp {} {
>  #
>  # CMD indicates what to do with the parent after detaching the child.
>  # Can be either "detach" to detach, or "continue", to continue to
> -# exit.  If "continue", then CONTINUE_RE is the regexp to expect.
> -# Defaults to normal exit output.
> +# exit.
>  #
> -proc do_detach {multi_process cmd {continue_re ""}} {
> +# CHILD_EXIT indicates how is the child expected to exit.  Can be
> +# either "normal" for normal exit, or "signal" for killed with signal
> +# SIGKILL.
> +#
> +proc do_detach {multi_process cmd child_exit} {
>      global decimal
>      global server_spawn_id
>  
> -    if {$continue_re == ""} {
> +    if {$child_exit == "normal"} {
>  	set continue_re "exited normally.*"
> +	set inf_output_re "exited, status=0"
> +    } elseif {$child_exit == "signal"} {
> +	if {$multi_process} {
> +	    set continue_re "exited with code 02.*"
> +	} else {
> +	    set continue_re "terminated with signal SIGKILL.*"
> +	}
> +	set inf_output_re "signaled, sig=9"
> +    } else {
> +	error "unhandled \$child_exit: $child_exit"
>      }
>  
>      set is_remote [expr {[target_info exists gdb_protocol]
> @@ -154,7 +168,7 @@ proc do_detach {multi_process cmd {continue_re ""}} {
>  	if {$cmd == "detach"} {
>  	    # Make sure that detach works and that the parent process
>  	    # exits cleanly.
> -	    detach_and_expect_exit "detach parent"
> +	    detach_and_expect_exit $inf_output_re "detach parent"
>  	} elseif {$cmd == "continue"} {
>  	    # Make sure that continuing works and that the parent process
>  	    # exits cleanly.
> @@ -205,7 +219,7 @@ proc test_detach {multi_process cmd} {
>  	# Run to _exit in the child.
>  	continue_to_exit_bp
>  
> -	do_detach $multi_process $cmd
> +	do_detach $multi_process $cmd "normal"
>      }
>  }
>  
> @@ -240,7 +254,7 @@ proc test_detach_watch {multi_process cmd} {
>  	# thread individually).
>  	continue_to_exit_bp
>  
> -	do_detach $multi_process $cmd
> +	do_detach $multi_process $cmd "normal"
>      }
>  }
>  
> @@ -279,12 +293,7 @@ proc test_detach_killed_outside {multi_process cmd} {
>  	# Give it some time to die.
>  	sleep 2
>  
> -	if {$multi_process} {
> -	    set continue_re "exited with code 02.*"
> -	} else {
> -	    set continue_re "terminated with signal SIGKILL.*"
> -	}
> -	do_detach $multi_process $cmd $continue_re
> +	do_detach $multi_process $cmd "signal"
>      }
>  }
>  
> -- 
> 2.5.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gdb.threads/process-dies-while-detaching.exp
  2017-11-23 19:51 ` Sergio Durigan Junior
@ 2017-12-03 15:24   ` Pedro Alves
  2017-12-03 15:34     ` Pedro Alves
  0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2017-12-03 15:24 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

On 11/23/2017 07:51 PM, Sergio Durigan Junior wrote:
> On Friday, November 17 2017, Pedro Alves wrote:
> 
>> I noticed [1] a test bug in gdb.threads/process-dies-while-detaching.exp.
>> Simplified, the test code in question looks somewhat like this:
>>
>> ~~~
>>   # Detach from a process, and ensure that it exits after detaching.
>>   # This relies on inferior I/O.
>>
>>   proc detach_and_expect_exit {test} {
>>
>>       gdb_test_multiple "detach" $test ....
>>
>>       set saw_prompt 0
>>       set saw_inf_exit 0
>>       while { !$saw_prompt && !$saw_inf_exit } {
>>           gdb_test_multiple "" $test {
>>               -re "exited, status=0" {
>>                   set saw_inf_exit 1
>>               }
>>               -re "$gdb_prompt " {
>>                   set saw_prompt 1
>>               }
>>           }
>>       }
>>
>>       pass $test
>>   }
>> ~~~
>>
>> The bug is in the while loop's condition.  We want to make sure we see
>> both the inferior output and the prompt, so the loop's test should be:
>>
>>    -    while { !$saw_prompt && !$saw_inf_exit } {
>>    +    while { !$saw_prompt || !$saw_inf_exit } {
>>
>> If we just fix that, the test starts failing though, because that
>> exposes a latent problem -- when called from
>> test_detach_killed_outside, the parent doesn't print "exited,
>> status=0", because in that case the child dies with a signal, and so
>> detach_and_expect_exit times out.  Fix it by making the parent print
>> "signaled, sig=9" in that case, and have the .exp expect it.
>>
>> [1] I changed GDB in a way that should have made the test fail, but it
>>     didn't.
> 
> I skimmed over the patch and it looks good to me, but you're the expert
> in this area here so I trust your judgement more than mine ;-)

:-)

I was going to push this as is, but I noticed that when
testing against --target_board=native-gdbserver, sometimes
I'd see this:

ERROR: Process no longer exists
ERROR: : spawn id exp9 not open
    while executing
"expect {
-i exp8 -timeout 220 
        -i $server_spawn_id
        eof {
            pass $test
            wait -i $server_spawn_id
            unset server_spawn_id
        }
        timeout {
           ..."
    ("uplevel" body line 1)
    invoked from within
"uplevel $body" NONE : spawn id exp9 not open


The problem is that:

 - inferior_spawn_id and server_spawn_id are the same when testing
   with gdbserver.
 - gdbserver exits after "detach", so we get an eof for
   $inferior_spawn_id in the loop in detach_and_expect_exit.
   That's the first "ERROR: Process no longer exists".
 - and then when we reach test_server_exit, server_spawn_id
   is already closed (because server_spawn_id==inferior_spawn_id).

To handle this, I'm making the loop in detach_and_expect_exit
use an indirect spawn id list and remove $inferior_spawn_id from
the list as soon as we got the inferior output we're expecting,
so that the "eof" is left unprocessed until we reach test_server_exit.

I'm squashing in the patch below.

From 75f48083daa998603438e71436e7268fc53aa978 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 3 Dec 2017 00:14:15 +0000
Subject: [PATCH] fix gdbserver target remote

---
 gdb/testsuite/gdb.threads/process-dies-while-detaching.exp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index ea8f6e9..910e786 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -86,6 +86,14 @@ proc detach_and_expect_exit {inf_output_re test} {
 	}
     }]
 
+    # Use an indirect spawn id list, and remove inferior spawn id from
+    # the expected output as soon as it matches, so that if
+    # $inf_inferior_spawn_id is $server_spawn_id and we're testing in
+    # "target remote" mode, the eof caused by gdbserver exiting is
+    # left for the caller to handle.
+    global daee_spawn_id_list
+    set daee_spawn_id_list "$inferior_spawn_id $gdb_spawn_id"
+
     set saw_prompt 0
     set saw_inf_exit 0
     while { !$saw_prompt || ! $saw_inf_exit } {
@@ -96,14 +104,16 @@ proc detach_and_expect_exit {inf_output_re test} {
 	# first in the script that occurs anywhere in the input, so that
 	# we don't skip anything.
 	return_if_fail [gdb_test_multiple "" $test {
-	    -i "$inferior_spawn_id $gdb_spawn_id"
+	    -i daee_spawn_id_list
 	    -re "($inf_output_re)|($gdb_prompt )" {
 		if {[info exists expect_out(1,string)]} {
 		    verbose -log "saw inferior exit"
 		    set saw_inf_exit 1
+		    set daee_spawn_id_list "$gdb_spawn_id"
 		} elseif {[info exists expect_out(2,string)]} {
 		    verbose -log "saw prompt"
 		    set saw_prompt 1
+		    set daee_spawn_id_list "$inferior_spawn_id"
 		}
 		array unset expect_out
 	    }
-- 
2.5.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix gdb.threads/process-dies-while-detaching.exp
  2017-12-03 15:24   ` Pedro Alves
@ 2017-12-03 15:34     ` Pedro Alves
  0 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2017-12-03 15:34 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

On 12/03/2017 03:24 PM, Pedro Alves wrote:

> I'm squashing in the patch below.
> 

And here's the resulting patch that I pushed in.

From f0fb2488c93c00fa1436a4813a375faa00a94de5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 3 Dec 2017 15:32:08 +0000
Subject: [PATCH] Fix gdb.threads/process-dies-while-detaching.exp

I noticed [1] a test bug in gdb.threads/process-dies-while-detaching.exp.
Simplified, the test code in question looks somewhat like this:

~~~
  # Detach from a process, and ensure that it exits after detaching.
  # This relies on inferior I/O.

  proc detach_and_expect_exit {test} {

      gdb_test_multiple "detach" $test ....

      set saw_prompt 0
      set saw_inf_exit 0
      while { !$saw_prompt && !$saw_inf_exit } {
          gdb_test_multiple "" $test {
              -re "exited, status=0" {
                  set saw_inf_exit 1
              }
              -re "$gdb_prompt " {
                  set saw_prompt 1
              }
          }
      }

      pass $test
  }
~~~

The bug is in the while loop's condition.  We want to make sure we see
both the inferior output and the prompt, so the loop's test should be:

   -    while { !$saw_prompt && !$saw_inf_exit } {
   +    while { !$saw_prompt || !$saw_inf_exit } {

If we just fix that, the test starts failing though, because it
exposes a couple latent problems:

- When called from test_detach_killed_outside, the parent doesn't
  print "exited, status=0", because in that case the child dies with a
  signal, and so detach_and_expect_exit times out.

  Fix it by making the parent print "signaled, sig=9" in that case,
  and have the .exp expect it.

- When testing against --target_board=native-gdbserver, sometimes we'd
  get this:

    ERROR: Process no longer exists
    ERROR: : spawn id exp9 not open
	while executing
    "expect {
    -i exp8 -timeout 220
	    -i $server_spawn_id
	    eof {
		pass $test
		wait -i $server_spawn_id
		unset server_spawn_id
	    }
	    timeout {
	       ..."
	("uplevel" body line 1)
	invoked from within
    "uplevel $body" NONE : spawn id exp9 not open

  The problem is that:

   - inferior_spawn_id and server_spawn_id are the same when testing
     with gdbserver.
   - gdbserver exits after "detach", so we get an eof for
     $inferior_spawn_id in the loop in detach_and_expect_exit.
     That's the first "ERROR: Process no longer exists".
   - and then when we reach test_server_exit, server_spawn_id
     is already closed (because server_spawn_id==inferior_spawn_id).

  To handle this, make the loop in detach_and_expect_exit use an
  indirect spawn id list and remove $inferior_spawn_id from the list
  as soon as we got the inferior output we're expecting, so that the
  "eof" is left unprocessed until we reach test_server_exit.

[1] I changed GDB in a way that should have made the test fail, but it
    didn't.

gdb/testsuite/ChangeLog:
2017-12-03  Pedro Alves  <palves@redhat.com>

	* gdb.threads/process-dies-while-detaching.c: Include <errno.h>
	and <string.h>.
	(parent_function): Print distinct messages when waitpid fails, or
	the child exits with a signal, or the child exits for an unhandled
	reason.
	* gdb.threads/process-dies-while-detaching.exp
	(detach_and_expect_exit): New 'inf_output_re' parameter and use
	it.  Wait for both inferior output and GDB's prompt.  Use an
	indirect spawn id list.
	(do_detach): New parameter 'child_exit'.  Use it to compute
	expected inferior output.
	(test_detach, test_detach_watch, test_detach_killed_outside):
	Adjust to pass down the expected child exit kind.
---
 gdb/testsuite/ChangeLog                            | 16 +++++++
 .../gdb.threads/process-dies-while-detaching.c     | 22 +++++++--
 .../gdb.threads/process-dies-while-detaching.exp   | 55
+++++++++++++++-------
 3 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 779fd0a..fa096b3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,19 @@
+2017-12-03  Pedro Alves  <palves@redhat.com>
+
+	* gdb.threads/process-dies-while-detaching.c: Include <errno.h>
+	and <string.h>.
+	(parent_function): Print distinct messages when waitpid fails, or
+	the child exits with a signal, or the child exits for an unhandled
+	reason.
+	* gdb.threads/process-dies-while-detaching.exp
+	(detach_and_expect_exit): New 'inf_output_re' parameter and use
+	it.  Wait for both inferior output and GDB's prompt.  Use an
+	indirect spawn id list.
+	(do_detach): New parameter 'child_exit'.  Use it to compute
+	expected inferior output.
+	(test_detach, test_detach_watch, test_detach_killed_outside):
+	Adjust to pass down the expected child exit kind.
+
 2017-12-01  Joel Brobecker  <brobecker@adacore.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 	    Pedro Alves  <palves@redhat.com>
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
index eecaaed..4ba50d4 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.c
@@ -22,6 +22,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <assert.h>
+#include <errno.h>
+#include <string.h>

 /* This barrier ensures we only reach the initial breakpoint after all
    threads have started.  */
@@ -78,15 +80,27 @@ parent_function (pid_t child)
   alarm (300);

   ret = waitpid (child, &status, 0);
+
   if (ret == -1)
-    exit (1);
-  else if (!WIFEXITED (status))
-    exit (2);
-  else
+    {
+      printf ("waitpid, errno=%d (%s)\n", errno, strerror (errno));
+      exit (1);
+    }
+  else if (WIFEXITED (status))
     {
       printf ("exited, status=%d\n", WEXITSTATUS (status));
       exit (0);
     }
+  else if (WIFSIGNALED (status))
+    {
+      printf ("signaled, sig=%d\n", WTERMSIG (status));
+      exit (2);
+    }
+  else
+    {
+      printf ("unexpected, status=%x\n", status);
+      exit (3);
+    }
 }

 #endif
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index 4cba80c..910e786 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -72,9 +72,10 @@ proc return_if_fail { result } {
 }

 # Detach from a process, and ensure that it exits after detaching.
-# This relies on inferior I/O.
+# This relies on inferior I/O.  INF_OUTPUT_RE is the pattern that
+# matches the expected inferior output.

-proc detach_and_expect_exit {test} {
+proc detach_and_expect_exit {inf_output_re test} {
     global decimal
     global gdb_spawn_id
     global inferior_spawn_id
@@ -85,9 +86,17 @@ proc detach_and_expect_exit {test} {
 	}
     }]

+    # Use an indirect spawn id list, and remove inferior spawn id from
+    # the expected output as soon as it matches, so that if
+    # $inf_inferior_spawn_id is $server_spawn_id and we're testing in
+    # "target remote" mode, the eof caused by gdbserver exiting is
+    # left for the caller to handle.
+    global daee_spawn_id_list
+    set daee_spawn_id_list "$inferior_spawn_id $gdb_spawn_id"
+
     set saw_prompt 0
     set saw_inf_exit 0
-    while { !$saw_prompt && ! $saw_inf_exit } {
+    while { !$saw_prompt || ! $saw_inf_exit } {
 	# We don't know what order the interesting things will arrive in.
 	# Using a pattern of the form 'x|y|z' instead of -re x ... -re y
 	# ... -re z ensures that expect always chooses the match that
@@ -95,14 +104,16 @@ proc detach_and_expect_exit {test} {
 	# first in the script that occurs anywhere in the input, so that
 	# we don't skip anything.
 	return_if_fail [gdb_test_multiple "" $test {
-	    -i "$inferior_spawn_id $gdb_spawn_id"
-	    -re "(exited, status=0)|($gdb_prompt )" {
+	    -i daee_spawn_id_list
+	    -re "($inf_output_re)|($gdb_prompt )" {
 		if {[info exists expect_out(1,string)]} {
 		    verbose -log "saw inferior exit"
 		    set saw_inf_exit 1
+		    set daee_spawn_id_list "$gdb_spawn_id"
 		} elseif {[info exists expect_out(2,string)]} {
 		    verbose -log "saw prompt"
 		    set saw_prompt 1
+		    set daee_spawn_id_list "$inferior_spawn_id"
 		}
 		array unset expect_out
 	    }
@@ -130,15 +141,28 @@ proc continue_to_exit_bp {} {
 #
 # CMD indicates what to do with the parent after detaching the child.
 # Can be either "detach" to detach, or "continue", to continue to
-# exit.  If "continue", then CONTINUE_RE is the regexp to expect.
-# Defaults to normal exit output.
+# exit.
+#
+# CHILD_EXIT indicates how is the child expected to exit.  Can be
+# either "normal" for normal exit, or "signal" for killed with signal
+# SIGKILL.
 #
-proc do_detach {multi_process cmd {continue_re ""}} {
+proc do_detach {multi_process cmd child_exit} {
     global decimal
     global server_spawn_id

-    if {$continue_re == ""} {
+    if {$child_exit == "normal"} {
 	set continue_re "exited normally.*"
+	set inf_output_re "exited, status=0"
+    } elseif {$child_exit == "signal"} {
+	if {$multi_process} {
+	    set continue_re "exited with code 02.*"
+	} else {
+	    set continue_re "terminated with signal SIGKILL.*"
+	}
+	set inf_output_re "signaled, sig=9"
+    } else {
+	error "unhandled \$child_exit: $child_exit"
     }

     set is_remote [expr {[target_info exists gdb_protocol]
@@ -154,7 +178,7 @@ proc do_detach {multi_process cmd {continue_re ""}} {
 	if {$cmd == "detach"} {
 	    # Make sure that detach works and that the parent process
 	    # exits cleanly.
-	    detach_and_expect_exit "detach parent"
+	    detach_and_expect_exit $inf_output_re "detach parent"
 	} elseif {$cmd == "continue"} {
 	    # Make sure that continuing works and that the parent process
 	    # exits cleanly.
@@ -205,7 +229,7 @@ proc test_detach {multi_process cmd} {
 	# Run to _exit in the child.
 	continue_to_exit_bp

-	do_detach $multi_process $cmd
+	do_detach $multi_process $cmd "normal"
     }
 }

@@ -240,7 +264,7 @@ proc test_detach_watch {multi_process cmd} {
 	# thread individually).
 	continue_to_exit_bp

-	do_detach $multi_process $cmd
+	do_detach $multi_process $cmd "normal"
     }
 }

@@ -279,12 +303,7 @@ proc test_detach_killed_outside {multi_process cmd} {
 	# Give it some time to die.
 	sleep 2

-	if {$multi_process} {
-	    set continue_re "exited with code 02.*"
-	} else {
-	    set continue_re "terminated with signal SIGKILL.*"
-	}
-	do_detach $multi_process $cmd $continue_re
+	do_detach $multi_process $cmd "signal"
     }
 }

-- 
2.5.5



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-12-03 15:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 18:38 [PATCH] Fix gdb.threads/process-dies-while-detaching.exp Pedro Alves
2017-11-23 19:51 ` Sergio Durigan Junior
2017-12-03 15:24   ` Pedro Alves
2017-12-03 15:34     ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox