From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7703 invoked by alias); 23 Nov 2017 19:51:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 7147 invoked by uid 89); 23 Nov 2017 19:51:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Nov 2017 19:51:23 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E3DBC6146F for ; Thu, 23 Nov 2017 19:51:21 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AB3BF61984; Thu, 23 Nov 2017 19:51:21 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix gdb.threads/process-dies-while-detaching.exp References: <1510943896-1301-1-git-send-email-palves@redhat.com> Date: Thu, 23 Nov 2017 19:51:00 -0000 In-Reply-To: <1510943896-1301-1-git-send-email-palves@redhat.com> (Pedro Alves's message of "Fri, 17 Nov 2017 18:38:16 +0000") Message-ID: <87a7zcbwp2.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00577.txt.bz2 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 > > * gdb.threads/process-dies-while-detaching.c: Include > and . > (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 > #include > #include > +#include > +#include > > /* 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/