From patchwork Wed Oct 24 11:47:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 29868 Received: (qmail 88584 invoked by alias); 24 Oct 2018 11:47:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 88558 invoked by uid 89); 24 Oct 2018 11:47:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.2 spammy=Except X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 Oct 2018 11:47:04 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id A03E5AF3F; Wed, 24 Oct 2018 11:47:01 +0000 (UTC) Subject: Re: [PATCH][gdb/testsuite] Rewrite catch-follow-exec.exp To: Simon Marchi , Gary Benson Cc: gdb-patches@sourceware.org, Pedro Alves References: <20181005101122.GA23867@delia> <20181009135155.GB12668@blade.nx> <8f8ffb94-5a0c-8b2b-d541-eaacd7d1f42c@suse.de> <20181010092735.GA29557@blade.nx> <20181010134423.GA23926@blade.nx> <20181011074744.GA7677@delia> <20181011083318.GA13751@blade.nx> <06b38d2e3f0a9394280553e70b9dfaf8@polymtl.ca> <480ea2be-eac0-8d19-cfe7-93c56b33a7ac@suse.de> <9d0dc535-b053-5063-eb0c-d7bf3e80fe49@suse.de> From: Tom de Vries Message-ID: <8b734739-a3c5-ad0a-9d4e-e92ccdfd72f8@suse.de> Date: Wed, 24 Oct 2018 13:47:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes On 10/24/18 1:37 AM, Simon Marchi wrote: > On 2018-10-23 6:38 p.m., Tom de Vries wrote: >> On 10/23/18 11:05 PM, Tom de Vries wrote: >>> On 10/23/18 11:04 PM, Simon Marchi wrote: >>>> On 2018-10-15 3:54 p.m., Tom de Vries wrote: >>>>>> Just wondering.  Would it make life easier if we fixed PR 23368, which >>>>>> is the reason we have to do the test in an unnatural way? >>>>> >>>>> Yes. >>>> >>>> Hi Tom, >>>> >>>> PR 23368 should be fixed now. Do you plan on updating catch-follow-exec.exp >>>> to be written in a more standard way? >>> >>> Sure, will do. >> >> How does this look? > > Hi Tom, > > Thanks for looking into this so quickly. And thanks for the quick review. > I have some superficial suggestions that > can help shorten the test a bit and make it more readable (some of them can be personal > preference though...). > > When the test name is omitted, it defaults to the command. So instead of > > gdb_test "catch exec" \ > {Catchpoint [0-9][0-9]* \(exec\)} \ > "catch exec" > > You can write > > gdb_test "catch exec" {Catchpoint [0-9][0-9]* \(exec\)} > > and the test name will be "catch exec". Done. > Instead of [0-9][0-9]*, I am > pretty sure you can use [0-9]+, Done. > or $decimal, which is provided by DejaGnu > (/usr/share/dejagnu/runtest.exp): > > 101: set decimal "\[0-9\]+" > > Except in the {} string, $decimal won't work, because it won't get > substituted. Indeed. I prefer the {} quoting over "" quoting if that means less escaping, so I went with {} here. > > For this: > > gdb_test "set follow-exec-mode new" \ > "" \ > "set follow-exec-mode new" > > You can use > > gdb_test_no_output "set follow-exec-mode new" > Done. > (again, omitting the test name makes it default to the command) > > I'd suggest replacing > > gdb_test_multiple "info prog" "info prog" { > -i "$gdb_spawn_id" eof { > fail "info prog" > } > -i "$gdb_spawn_id" "No selected thread\." { > pass "info prog" > } > } > > with the simpler > > gdb_test "info prog" "No selected thread." > > If GDB crashes as it did before your fix, the test will be unresolved, which is > treated the same as a FAIL. Done. > While at it, could you update the comment at the top of the file, which currently > says: > > # Check whether finish respects the print pretty user setting when printing the > # function result. > Done. Also, I realized that by using runto_main at the start, I could replace gdb_run_cmd/gdb_expect with a regular gdb_test continue. Committed as attached. Thanks, - Tom [gdb/testsuite] Rewrite catch-follow-exec.exp using gdb_test The testcase catch-follow-exec.exp is written use gdb -batch in order to avoid a GDB SIGTTOU. After the commit of "Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368)", that no longer is necessary. Rewrite the test using regular gdb_test commands. Tested with x86_64-linux. 2018-10-24 Tom de Vries * gdb.base/catch-follow-exec.exp: Rewrite using gdb_test. --- gdb/testsuite/gdb.base/catch-follow-exec.exp | 63 ++++++---------------------- 1 file changed, 13 insertions(+), 50 deletions(-) diff --git a/gdb/testsuite/gdb.base/catch-follow-exec.exp b/gdb/testsuite/gdb.base/catch-follow-exec.exp index c3c7c7ecdd..5f7db25265 100644 --- a/gdb/testsuite/gdb.base/catch-follow-exec.exp +++ b/gdb/testsuite/gdb.base/catch-follow-exec.exp @@ -13,72 +13,35 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -# Check whether finish respects the print pretty user setting when printing the -# function result. +# Test whether info prog crashes gdb at a catch point in follow-exec-mode new. standard_testfile -if { [target_info gdb_protocol] != "" } { - # Even though the feature under features being tested are supported by - # gdbserver, the way this test is written doesn't make it easy with a - # remote target. - unsupported "not native" - return -} - if { ![remote_file target exists /bin/ls] } { unsupported "no ls" return } -if { [build_executable "failed to prepare" $testfile $srcfile debug] == -1 } { - return -1 +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } { + return } proc catch_follow_exec { } { - global binfile - global gdb_spawn_id - - set test "catch-follow-exec" - - append FLAGS " \"$binfile\"" - append FLAGS " -batch" - append FLAGS " -ex \"catch exec\"" - append FLAGS " -ex \"set follow-exec-mode new\"" - append FLAGS " -ex \"run\"" - append FLAGS " -ex \"info prog\"" - - gdb_exit - if {[gdb_spawn_with_cmdline_opts "$FLAGS"] != 0} { - fail "spawn" - return + if { ![runto_main] } { + untested "could not run to main" + return -1 } - gdb_test_multiple "" "run til exit" { - "runtime error:" { - # Error in case of --enable-ubsan - fail "no runtime error" - } - eof { - set result [wait -i $gdb_spawn_id] - verbose $result + gdb_test "catch exec" \ + {Catchpoint [0-9]+ \(exec\)} - gdb_assert { [lindex $result 2] == 0 } + gdb_test_no_output "set follow-exec-mode new" - # We suspect this will be zero instead of one after fixing PR23368 - # - "gdb goes to into background when hitting exec catchpoint with - # follow-exec-mode new" - gdb_assert { [lindex $result 3] != 0 } + gdb_test "continue" \ + ".*hit Catchpoint.*" - # Error in case of --disable-ubsan, we get - # "CHILDKILLED SIGSEGV {segmentation violation}" as extra - # argument(s). - gdb_assert { [llength $result] == 4 } - } - - remote_close host - clear_gdb_spawn_id - } + gdb_test "info prog" \ + "No selected thread." } catch_follow_exec