diff mbox

[1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp

Message ID 1463504594-4419-1-git-send-email-antoine.tremblay@ericsson.com
State New
Headers show

Commit Message

Antoine Tremblay May 17, 2016, 5:03 p.m. UTC
This patch moves conditional tests that were done in ftrace.exp to
trace-conditon.exp.

Note that emit_ref is now tested by the anarg local variable there is no
need to test the register directly.

The test coverage remains the same.

gdb/testsuite/ChangeLog:

	* gdb.trace/ftrace.exp (test_ftrace_condition): Remove.
	Move condition tests...
	* gdb.trace/trace-condition.exp: Here.
---
 gdb/testsuite/gdb.trace/ftrace.exp          | 74 -----------------------------
 gdb/testsuite/gdb.trace/trace-condition.exp |  2 +
 2 files changed, 2 insertions(+), 74 deletions(-)

Comments

Pedro Alves May 27, 2016, noon UTC | #1
On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
> This patch moves conditional tests that were done in ftrace.exp to
> trace-conditon.exp.

Typo conditon.exp.

> 
> Note that emit_ref is now tested by the anarg local variable there is no
> need to test the register directly.
> 
> The test coverage remains the same.

How did you determine it that it's the same?

E.g., seems like the <, >, etc., conditions in ftrace.exp use a 
global variable, while the trace-condition.exp ones use constants.

I also notice that all tests in trace-condition.exp expect 10 frames
to be collected.  If a condition's handling is broken and the
tracepoints ends up unconditional, will we ever notice?

Seems like we have counter-cases using the same operators
but that eval false, like (relative to your patch):

    test_tracepoints $trace_command "21 == 21" 10
+    test_tracepoints $trace_command "21 == 22" 0
    test_tracepoints $trace_command "21 != 42" 10
+    test_tracepoints $trace_command "21 != 21" 0

Thanks,
Pedro Alves
Antoine Tremblay May 27, 2016, 12:35 p.m. UTC | #2
Pedro Alves writes:

> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
>> This patch moves conditional tests that were done in ftrace.exp to
>> trace-conditon.exp.
>
> Typo conditon.exp.
>

Fixed.

>> 
>> Note that emit_ref is now tested by the anarg local variable there is no
>> need to test the register directly.
>> 
>> The test coverage remains the same.
>
> How did you determine it that it's the same?

I've put an assert in each emit function and validated that they are all
called before / after the move.

>
> E.g., seems like the <, >, etc., conditions in ftrace.exp use a 
> global variable, while the trace-condition.exp ones use constants.
>

That is not a problem since testing with a constant tests the < >
on it's own.

The reading of global variable used in ftrace is tested separately in
trace-condition.exp.

> I also notice that all tests in trace-condition.exp expect 10 frames
> to be collected.  If a condition's handling is broken and the
> tracepoints ends up unconditional, will we ever notice?
>

That's a good point, that was pre-existing in trace-condition.exp I
think it's mitigated partially by counter-cases like:

test_tracepoints $trace_command "21 == 21" 10
test_tracepoints $trace_command "21 != 42" 10

But could be a problem with conditions like:
test_tracepoints $trace_command "(0xabababab & 0x0000ffff) == 0xabab" 10

It could add counter cases to all true/false tests like :

test_tracepoints $trace_command "(0xabababab & 0x0000ffff) == 0xffff" 0

And do that for the others.

> Seems like we have counter-cases using the same operators
> but that eval false, like (relative to your patch):
>

Yes indeed, as above. I'll do that in a patch preceding this one that
will change the existing trace-condition.exp first.

Thanks,
Antoine
Pedro Alves May 27, 2016, 12:47 p.m. UTC | #3
On 05/27/2016 01:35 PM, Antoine Tremblay wrote:
> 
> Pedro Alves writes:
> 
>> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:

>>> The test coverage remains the same.
>>
>> How did you determine it that it's the same?
> 
> I've put an assert in each emit function and validated that they are all
> called before / after the move.

That's excellent, and it's the sort of info that should be
explicitly  mentioned in the commit log.

Thanks,
Pedro Alves
Antoine Tremblay May 27, 2016, 12:51 p.m. UTC | #4
Pedro Alves writes:

> On 05/27/2016 01:35 PM, Antoine Tremblay wrote:
>> 
>> Pedro Alves writes:
>> 
>>> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
>
>>>> The test coverage remains the same.
>>>
>>> How did you determine it that it's the same?
>> 
>> I've put an assert in each emit function and validated that they are all
>> called before / after the move.
>
> That's excellent, and it's the sort of info that should be
> explicitly  mentioned in the commit log.

OK.
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index f770bf7..e90485c 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -174,43 +174,6 @@  proc test_fast_tracepoints {} {
     }
 }
 
-# Test compiled-condition
-# CONDEXP is the condition expression to be compiled.
-# VAR is the variable to be collected for testing.
-# LIST is a list of expected values of VAR should be collected
-# based on the CONDEXP.
-proc test_ftrace_condition { condexp var list } \
-{ with_test_prefix "cond $condexp" \
-{
-    global executable
-    global hex
-
-    clean_restart ${executable}
-    if ![runto_main] {
-	fail "Can't run to main to check for trace support"
-	return -1
-    }
-
-    gdb_test "break end" ".*" ""
-    gdb_test "tvariable \$tsv = 0"
-    gdb_test "ftrace set_point if $condexp" "Fast tracepoint .*"
-    gdb_trace_setactions "set action for tracepoint .*" "" \
-	"collect $var" "^$"
-
-    gdb_test_no_output "tstart" ""
-    gdb_test "continue" \
-	"Continuing\\.\[ \r\n\]+(Thread .* hit )?Breakpoint.*" ""
-    gdb_test_no_output "tstop" ""
-
-    set i 0
-    foreach expval $list {
-	gdb_test "tfind" "Found trace frame $i, tracepoint .*" "tfind frame $i"
-	gdb_test "print $var" "\\$\[0-9\]+ = $expval\[\r\n\]" "expect $expval"
-	set i [expr $i + 1]
-    }
-    gdb_test "tfind" "Target failed to find requested trace frame\."
-}}
-
 gdb_reinitialize_dir $srcdir/$subdir
 
 if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 } {
@@ -219,40 +182,3 @@  if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 }
 }
 
 test_fast_tracepoints
-
-# Test conditional goto and simple expression.
-test_ftrace_condition "globvar > 7" "globvar" { 8 9 10 }
-test_ftrace_condition "globvar < 4" "globvar" { 1 2 3 }
-test_ftrace_condition "globvar >= 7" "globvar" { 7 8 9 10 }
-test_ftrace_condition "globvar <= 4" "globvar" { 1 2 3 4 }
-test_ftrace_condition "globvar == 5" "globvar" { 5 }
-test_ftrace_condition "globvar != 5" "globvar" { 1 2 3 4 6 7 8 9 10 }
-test_ftrace_condition "globvar > 3 && globvar < 7" "globvar" { 4 5 6 }
-test_ftrace_condition "globvar < 3 || globvar > 7" "globvar" { 1 2 8 9 10 }
-test_ftrace_condition "(globvar << 2) + 1 == 29" "globvar" { 7 }
-test_ftrace_condition "(globvar >> 2) == 2" "globvar" { 8 9 10 }
-
-# Test emit_call by accessing trace state variables.
-test_ftrace_condition "(\$tsv = \$tsv + 2) > 10" "globvar" { 6 7 8 9 10 }
-
-# This expression is used for testing emit_reg.
-if [is_amd64_regs_target] {
-    set arg0exp "\$rdi"
-} elseif [is_x86_like_target] {
-    set arg0exp "*(int *) (\$ebp + 8)"
-} elseif { [istarget "aarch64*-*-*"] } {
-    set arg0exp "\$x0"
-} elseif [istarget "powerpc*-*-*"] {
-    set arg0exp "\$r3"
-} elseif { [istarget "s390x-*-*"] } {
-    set arg0exp "\$r2"
-} elseif { [istarget "s390-*-*"] } {
-    # On targets with 64-bit GPRs, avoid the high part.
-    set arg0exp "(\$r2 & 0xffffffff)"
-} else {
-    set arg0exp ""
-}
-
-if { "$arg0exp" != "" } {
-    test_ftrace_condition "($arg0exp > 500)" "globvar" { 6 7 8 9 10 }
-}
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index b7427ca..fa031ba 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -137,6 +137,8 @@  foreach trace_command { "trace" "ftrace" } {
     test_tracepoints $trace_command "anarg == 100 && globvar == 1" 1 18955_x86_64_failure
 
     # Test various operations to cover as many opcodes as possible.
+    test_tracepoints $trace_command "21 == 21" 10
+    test_tracepoints $trace_command "21 != 42" 10
     test_tracepoints $trace_command "21 + 21 == 42" 10
     test_tracepoints $trace_command "42 - 21 == 21" 10
     test_tracepoints $trace_command "21 * 2 == 42" 10