[gdb/testsuite] Rewrite catch-follow-exec.exp

Message ID cb843f7b-dea8-cdf1-7c7b-bead3f948bc1@suse.de
State New, archived
Headers

Commit Message

Tom de Vries Oct. 23, 2018, 10:38 p.m. UTC
  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?

Thanks,
- Tom
  

Comments

Simon Marchi Oct. 23, 2018, 11:37 p.m. UTC | #1
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.  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".  Instead of [0-9][0-9]*, I am
pretty sure you can use [0-9]+, 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.

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"

(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.  If you decide to keep the gdb_test_multiple, I
think you don't need to specify -i "$gdb_spawn_id", it's the default.  Also, it's
common practice to factor out the test name, to make sure it's constant.  And
because the test name is the same as the command, you could do

set test "info prog"
gdb_test_multiple $test $test {
  eof {
    fail $test
  }
  -re "No selected thread\." {
    pass $test
  }
}

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.

Thanks!

Simon
  

Patch

[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  <tdevries@suse.de>

	* gdb.base/catch-follow-exec.exp: Rewrite using gdb_test.

---
 gdb/testsuite/gdb.base/catch-follow-exec.exp | 64 +++++++++-------------------
 1 file changed, 19 insertions(+), 45 deletions(-)

diff --git a/gdb/testsuite/gdb.base/catch-follow-exec.exp b/gdb/testsuite/gdb.base/catch-follow-exec.exp
index c3c7c7ecdd..25d14b8465 100644
--- a/gdb/testsuite/gdb.base/catch-follow-exec.exp
+++ b/gdb/testsuite/gdb.base/catch-follow-exec.exp
@@ -18,66 +18,40 @@ 
 
 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
+    global gdb_prompt gdb_spawn_id
 
-    set test "catch-follow-exec"
+    gdb_test "catch exec" \
+	{Catchpoint [0-9][0-9]* \(exec\)} \
+	"catch 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_test "set follow-exec-mode new" \
+	"" \
+	"set follow-exec-mode new"
 
-    gdb_exit
-    if {[gdb_spawn_with_cmdline_opts "$FLAGS"] != 0} {
-	fail "spawn"
-	return
+    gdb_run_cmd
+    gdb_expect {
+        -re ".*hit Catchpoint.*${gdb_prompt} $" {
+	    pass "run"
+        }
     }
 
-    gdb_test_multiple "" "run til exit" {
-	"runtime error:" {
-	    # Error in case of --enable-ubsan
-	    fail "no runtime error"
+    gdb_test_multiple "info prog" "info prog" {
+	-i "$gdb_spawn_id" eof {
+	    fail "info prog"
 	}
-	eof {
-	    set result [wait -i $gdb_spawn_id]
-	    verbose $result
-
-	    gdb_assert { [lindex $result 2] == 0 }
-
-	    # 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 }
-
-	    # Error in case of --disable-ubsan, we get
-	    # "CHILDKILLED SIGSEGV {segmentation violation}" as extra
-	    # argument(s).
-	    gdb_assert { [llength $result] == 4 }
+	-i "$gdb_spawn_id" "No selected thread\."  {
+	    pass "info prog"
 	}
-
-	remote_close host
-	clear_gdb_spawn_id
     }
 }