[1/7] New test case gdb.trace/signal.exp

Message ID 86a8l0leqy.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi April 11, 2016, 8:40 a.m. UTC
  Pedro Alves <palves@redhat.com> writes:

Hi Pedro,

> It'd be good to have an intro comment like this in the signal.exp
> file itself.  Actually, it'd be good to add some general comments in
> the .exp file explaining what the test is doing: both an overview,
> and on the individual test parts.  From the quick skim I did, I
> couldn't really figure out what is the test doing.

Fixed.

>> +
>> +set syscall_insn ""
>> +
>> +# Define the syscall instruction for each target.
>> +
>> +if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
>> +    set syscall_insn "\[ \t\](int|syscall|sysenter)\[ \t\]"
>> +} elseif { [istarget "aarch64*-*-linux*"] || [istarget "arm*-*-linux*"] } {
>> +    set syscall_insn "\[ \t\](swi|svc)\[ \t\]"
>> +} else {
>> +    return -1
>
> Call "unsupported" before returning, perhaps?
>

Done, and I move this block after gdb_target_supports_trace check.

>> +
>> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, (.* in
>> |__libc_|)kill \\(\\).*" \
>> +    "continue to kill (1st time)"
>
> Avoid using ()s at end of test message to differentiate tests:
>
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
>
> Suggest, e.g.:
>
>     "continue to kill, 1st time"
>     "continue to kill, 2nd time"
>

I didn't know this rule.  Thanks for letting me know.  Fixed.


>> +
>> +for { set i 0 } { $i < [expr 20] } { incr i } {
>
> Why "20" here?
>

It is a leftover.  Fixed.

>> +    if {[info exists tracepoint_hits($i)]} {
>> +	gdb_assert { $tracepoint_hits($i) == $iterations } \
>> +	    "tracepoint $i hit $iterations times"
>> +    }
>> +}
>> 
>
> I noticed several places could use $decimal.

Done.
  

Comments

Pedro Alves April 11, 2016, 2:03 p.m. UTC | #1
On 04/11/2016 09:40 AM, Yao Qi wrote:



> +# Record the hit times of each tracepoint in this array.
> +array set tracepoint_hits { }
> +
> +set test "tfind 0"
> +gdb_test_multiple $test $test {

Why do we need this separate "tfind 0" step?  I'd think the
"tfind" loop below would be sufficient?

> +    -re "Found trace frame 0, tracepoint ($decimal).*\r\n$gdb_prompt $" {
> +
> +	set idx [expr $expect_out(1,string)]
> +
> +	if {[info exists tracepoint_hits($idx)]} {
> +	    incr tracepoint_hits($idx)
> +	} else {
> +	    set tracepoint_hits($idx) 1
> +	}
> +    }
> +}
> +
> +set loop 1
> +while { $loop } {
> +    set test "tfind"
> +    gdb_test_multiple $test $test {
> +	-re "Found trace frame $decimal, tracepoint ($decimal).*\r\n$gdb_prompt $" {
> +	    set idx [expr $expect_out(1,string)]
> +
> +	    if {[info exists tracepoint_hits($idx)]} {
> +		incr tracepoint_hits($idx)
> +	    } else {
> +		set tracepoint_hits($idx) 1
> +	    }
> +	}
> +	-re "Target failed to find requested trace frame\..*\r\n$gdb_prompt $" {
> +	    set loop 0
> +	}
> +    }

If this gdb_test_multiple FAILs or times out, the loop
will continue, over and over, forever.  So we need to reverse
the logic -- assume no looping, unless the "Found trace ..." regex matched.

I'd also suggest to preinitialize the array elements to 0, avoiding
the "info exists" calls in "Step 3", here:

> +# Step 3, check the number of collections on each tracepoint.
> +
> +for { set i $tpnum } { $i < [expr $tpnum + 2] } { incr i } {
> +    if {[info exists tracepoint_hits($i)]} {
> +	gdb_assert { $tracepoint_hits($i) == $iterations } \
> +	    "tracepoint $i hit $iterations times"
> +    } else {
> +	fail "can't find tracepoint $i hit"

Also, here I think it's nicer if PASS/FAIL messages are the same,
for test result diffing.

Thus, something like:

for { set i $tpnum } { $i < [expr $tpnum + 2] } { incr i } {
  set tracepoint_hits($idx) 0
}

while { 1 } {
   set test "tfind"
   set idx 0
   gdb_test_multiple $test $test {
	-re "Found trace frame $decimal, tracepoint ($decimal).*\r\n$gdb_prompt $" {
	    set idx [expr $expect_out(1,string)]
	    incr tracepoint_hits($idx)
	}
   }
   if {$idx == 0} {
	break
   }
}

# Step 3, check the number of collections on each tracepoint.
for { set i $tpnum } { $i < [expr $tpnum + 2] } { incr i } {
   gdb_assert { $tracepoint_hits($i) == $iterations } \
      "tracepoint $i hit $iterations times"
}


Otherwise looks good to me.

Thanks,
Pedro Alves
  
Pedro Alves April 11, 2016, 2:07 p.m. UTC | #2
BTW,

On 04/11/2016 09:40 AM, Yao Qi wrote:
> +#include <signal.h>
> +#include <string.h>
> +#include <stdio.h>
> +

> +      kill (getpid (), SIGABRT);

getpid needs unistd.h, and stdio.h doesn't look like needed.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/testsuite/gdb.trace/signal.c b/gdb/testsuite/gdb.trace/signal.c
new file mode 100644
index 0000000..b2b976d
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/signal.c
@@ -0,0 +1,68 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <signal.h>
+#include <string.h>
+#include <stdio.h>
+
+static int counter = 0;
+
+static void
+handler (int sig)
+{
+  counter++;
+}
+
+static int iterations = 3;
+
+static void
+start (int pid)
+{
+  int i;
+
+  for (i = 0; i < iterations; i++)
+    {
+      kill (pid, SIGABRT);
+    }
+}
+
+static void
+end (void)
+{}
+
+int
+main (void)
+{
+  struct sigaction act;
+  int i;
+
+  memset (&act, 0, sizeof act);
+  act.sa_handler = handler;
+  act.sa_flags = SA_NODEFER;
+  sigaction (SIGABRT, &act, NULL);
+
+  for (i = 0; i < 3; i++)
+    {
+      kill (getpid (), SIGABRT);
+    }
+
+  counter = 0;
+  start (getpid ());
+
+  end ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/signal.exp b/gdb/testsuite/gdb.trace/signal.exp
new file mode 100644
index 0000000..8883bfc
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/signal.exp
@@ -0,0 +1,196 @@ 
+#   Copyright 2016 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This is to test whether GDBserver or other remote stubs deliver signal
+# to the inferior while step over thread.  The program signal.c sends
+# signal SIGABRT to itself via kill syscall.  The test sets tracepoints
+# syscall instruction and the next one, and it is quite likely that
+# GDBserver gets the signal when it steps over thread and does the
+# collection.  If GDBserver doesn't deliver signal in thread step over,
+# one collection is got for one tracepoint hit.  Otherwise, there may
+# be two collections for one tracepoint hit, because tracepoint is
+# collected once before step over, the program goes into signal handler
+# (because signal is delivered in step over), and program goes back
+# to the tracepoint again (one more collection) after returns from
+# signal handler.
+
+load_lib "trace-support.exp"
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    untested $testfile.exp
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1
+}
+
+# Step 1, find the syscall instruction address.
+
+set syscall_insn ""
+
+# Define the syscall instruction for each target.
+
+if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
+    set syscall_insn "\[ \t\](int|syscall|sysenter)\[ \t\]"
+} elseif { [istarget "aarch64*-*-linux*"] || [istarget "arm*-*-linux*"] } {
+    set syscall_insn "\[ \t\](swi|svc)\[ \t\]"
+} else {
+    unsupported "unknown syscall instruction"
+    return -1
+}
+
+# Start with a fresh gdb.
+clean_restart ${testfile}
+if ![runto_main] {
+    fail "Can't run to main"
+    return -1
+}
+
+gdb_test "break kill" "Breakpoint $decimal at .*"
+gdb_test "handle SIGABRT nostop noprint pass" ".*" "pass SIGABRT"
+
+# Hit the breakpoint on $syscall for the first time.  In this time,
+# we will let PLT resolution done, and the number single steps we will
+# do later will be reduced.
+gdb_test "continue" "Continuing\\..*Breakpoint $decimal, (.* in |__libc_|)kill \\(\\).*" \
+    "continue to kill, 1st time"
+
+# Hit the breakpoint on $syscall for the second time.  In this time,
+# the address of syscall insn and next insn of syscall are recorded.
+gdb_test "continue" "Continuing\\..*Breakpoint $decimal, (.* in |__libc_|)kill \\(\\).*" \
+    "continue to kill, 2nd time"
+
+gdb_test "display/i \$pc" ".*"
+
+# Single step until we see a syscall insn or we reach the
+# upper bound of loop iterations.
+set msg "find syscall insn in kill"
+set steps 0
+set max_steps 1000
+gdb_test_multiple "stepi" $msg {
+    -re ".*$syscall_insn.*$gdb_prompt $" {
+	pass $msg
+    }
+    -re "x/i .*=>.*\r\n$gdb_prompt $" {
+	incr steps
+	if {$steps == $max_steps} {
+	    fail $msg
+	} else {
+	    send_gdb "stepi\n"
+	    exp_continue
+	}
+    }
+}
+
+if {$steps == $max_steps} {
+    return
+}
+
+# Remove the display
+gdb_test_no_output "delete display 1"
+
+set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0"]
+set syscall_insn_next 0
+set test "x/2i \$pc"
+gdb_test_multiple $test $test {
+    -re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	set syscall_insn_next $expect_out(1,string)
+    }
+}
+
+delete_breakpoints
+gdb_test "break start" "Breakpoint $decimal at .*"
+gdb_continue_to_breakpoint "continue to start"
+
+gdb_assert { 0 == [get_integer_valueof "counter" "1"] } "counter is zero"
+
+delete_breakpoints
+
+# Step 2, set tracepoints on syscall instruction and the next one.
+# It is more likely to get signal on these two places when GDBserver
+# is doing step-over.
+gdb_test "trace *$syscall_insn_addr" "Tracepoint $decimal at .*" \
+    "tracepoint on syscall instruction"
+set tpnum [get_integer_valueof "\$bpnum" 0]
+gdb_test "trace *$syscall_insn_next" "Tracepoint $decimal at .*" \
+    "tracepoint on instruction following syscall instruction"
+
+gdb_test "break end" "Breakpoint $decimal at .*"
+
+gdb_test_no_output "tstart"
+gdb_test "continue" ".*Breakpoint.* end .*at.*$srcfile.*" \
+    "continue to end"
+gdb_test_no_output "tstop"
+
+set iterations [get_integer_valueof "iterations" "0"]
+
+gdb_assert { $iterations == [get_integer_valueof "counter" "0"] } \
+    "iterations equals to counter"
+
+# Record the hit times of each tracepoint in this array.
+array set tracepoint_hits { }
+
+set test "tfind 0"
+gdb_test_multiple $test $test {
+    -re "Found trace frame 0, tracepoint ($decimal).*\r\n$gdb_prompt $" {
+
+	set idx [expr $expect_out(1,string)]
+
+	if {[info exists tracepoint_hits($idx)]} {
+	    incr tracepoint_hits($idx)
+	} else {
+	    set tracepoint_hits($idx) 1
+	}
+    }
+}
+
+set loop 1
+while { $loop } {
+    set test "tfind"
+    gdb_test_multiple $test $test {
+	-re "Found trace frame $decimal, tracepoint ($decimal).*\r\n$gdb_prompt $" {
+	    set idx [expr $expect_out(1,string)]
+
+	    if {[info exists tracepoint_hits($idx)]} {
+		incr tracepoint_hits($idx)
+	    } else {
+		set tracepoint_hits($idx) 1
+	    }
+	}
+	-re "Target failed to find requested trace frame\..*\r\n$gdb_prompt $" {
+	    set loop 0
+	}
+    }
+}
+
+# Step 3, check the number of collections on each tracepoint.
+
+for { set i $tpnum } { $i < [expr $tpnum + 2] } { incr i } {
+    if {[info exists tracepoint_hits($i)]} {
+	gdb_assert { $tracepoint_hits($i) == $iterations } \
+	    "tracepoint $i hit $iterations times"
+    } else {
+	fail "can't find tracepoint $i hit"
+    }
+}