[1/5] Add counter-cases for trace-condition.exp tests

Message ID 1464375916-16369-1-git-send-email-antoine.tremblay@ericsson.com
State New, archived
Headers

Commit Message

Antoine Tremblay May 27, 2016, 7:05 p.m. UTC
  In trace-condition.exp, tests are done by doing a conditional tracepoint
and validating that the trace contains all the frames that could be
collected if that condition is true.

E.g. test_tracepoints $trace_command "21 + 21 == 42" 10

This will always return true and collect the 10 frames possible to collect
with the test program.

However, if the condition evaluation is broken such that the condition is
unconditional we will not notice this problem.

This patch adds counter-cases to such conditions like so:

$trace_command "21 + 11 == 42" 0

This way such a problem would be noticed.

gdb/testsuite/ChangeLog:

	* gdb.trace/trace-condition.exp: Add counter-case tests.
---
 gdb/testsuite/gdb.trace/trace-condition.exp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
  

Comments

Pedro Alves May 30, 2016, 10:50 a.m. UTC | #1
On 05/27/2016 08:05 PM, Antoine Tremblay wrote:
> In trace-condition.exp, tests are done by doing a conditional tracepoint
> and validating that the trace contains all the frames that could be
> collected if that condition is true.
> 
> E.g. test_tracepoints $trace_command "21 + 21 == 42" 10
> 
> This will always return true and collect the 10 frames possible to collect
> with the test program.
> 
> However, if the condition evaluation is broken such that the condition is
> unconditional we will not notice this problem.
> 
> This patch adds counter-cases to such conditions like so:
> 
> $trace_command "21 + 11 == 42" 0
> 
> This way such a problem would be noticed.

Thanks for doing this.

> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.trace/trace-condition.exp: Add counter-case tests.
> ---
>  gdb/testsuite/gdb.trace/trace-condition.exp | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
> index b7427ca..f9eaf31 100644
> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
> @@ -157,4 +157,25 @@ foreach trace_command { "trace" "ftrace" } {
>      test_tracepoints $trace_command "(42 >= 42 ? 0 : 1) == 0" 10
>      test_tracepoints $trace_command "(42 > 21 ? 0 : 1) == 0" 10 18955_i386_failure
>      test_tracepoints $trace_command "\$trace_timestamp >= 0" 10
> +
> +    # Counter-cases tests.
> +    test_tracepoints $trace_command "21 + 21 == 11" 0
> +    test_tracepoints $trace_command "42 - 21 == 11" 0
> +    test_tracepoints $trace_command "21 * 2 == 11" 0
> +    test_tracepoints $trace_command "21 << 1 == 11" 0
> +    test_tracepoints $trace_command "42 >> 1 == 11" 0
> +    test_tracepoints $trace_command "-(21 << 1) == -11" 0
> +    test_tracepoints $trace_command "-42 >> 1 == -11" 0
> +    test_tracepoints $trace_command "(0xabababab & 0x0000ffff) == 0xffff" 0
> +    test_tracepoints $trace_command "(0xabababab | 0x0000ffff) == 0xeeeedddd" 0
> +    test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xaaaaaaaa" 0
> +    test_tracepoints $trace_command "~0xaaaaaaaa == 0x11111111" 0
> +    test_tracepoints $trace_command "61 < 42" 0
> +    test_tracepoints $trace_command "42 <= 11" 0
> +    test_tracepoints $trace_command "11 >= 42" 0
> +    test_tracepoints $trace_command "11 > 21" 0
> +    test_tracepoints $trace_command "(66 < 42 ? 0 : 1) == 0" 0 18955_i386_failure
> +    test_tracepoints $trace_command "(66 <= 42 ? 0 : 1) == 0" 0
> +    test_tracepoints $trace_command "(11 >= 42 ? 0 : 1) == 0" 0
> +    test_tracepoints $trace_command "(11 > 21 ? 0 : 1) == 0" 0 18955_i386_failure

I'm surprised to see these split into their own section though.
I'd think it'd be better to keep each along side the corresponding test,
to make it easier to keep them in sync:

     test_tracepoints $trace_command "21 + 21 == 42" 10
+    test_tracepoints $trace_command "21 + 21 == 11" 0
     test_tracepoints $trace_command "42 - 21 == 21" 10
+    test_tracepoints $trace_command "42 - 21 == 11" 0

etc.

Could you do that?

>  }
> 

Thanks,
Pedro Alves
  
Antoine Tremblay May 30, 2016, 2:20 p.m. UTC | #2
Pedro Alves writes:

> On 05/27/2016 08:05 PM, Antoine Tremblay wrote:
>> In trace-condition.exp, tests are done by doing a conditional tracepoint
>> and validating that the trace contains all the frames that could be
>> collected if that condition is true.
>> 
>> E.g. test_tracepoints $trace_command "21 + 21 == 42" 10
>> 
>> This will always return true and collect the 10 frames possible to collect
>> with the test program.
>> 
>> However, if the condition evaluation is broken such that the condition is
>> unconditional we will not notice this problem.
>> 
>> This patch adds counter-cases to such conditions like so:
>> 
>> $trace_command "21 + 11 == 42" 0
>> 
>> This way such a problem would be noticed.
>
> Thanks for doing this.
>
>> 
>> gdb/testsuite/ChangeLog:
>> 
>> 	* gdb.trace/trace-condition.exp: Add counter-case tests.
>> ---
>>  gdb/testsuite/gdb.trace/trace-condition.exp | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>> 
>> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
>> index b7427ca..f9eaf31 100644
>> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
>> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
>> @@ -157,4 +157,25 @@ foreach trace_command { "trace" "ftrace" } {
>>      test_tracepoints $trace_command "(42 >= 42 ? 0 : 1) == 0" 10
>>      test_tracepoints $trace_command "(42 > 21 ? 0 : 1) == 0" 10 18955_i386_failure
>>      test_tracepoints $trace_command "\$trace_timestamp >= 0" 10
>> +
>> +    # Counter-cases tests.
>> +    test_tracepoints $trace_command "21 + 21 == 11" 0
>> +    test_tracepoints $trace_command "42 - 21 == 11" 0
>> +    test_tracepoints $trace_command "21 * 2 == 11" 0
>> +    test_tracepoints $trace_command "21 << 1 == 11" 0
>> +    test_tracepoints $trace_command "42 >> 1 == 11" 0
>> +    test_tracepoints $trace_command "-(21 << 1) == -11" 0
>> +    test_tracepoints $trace_command "-42 >> 1 == -11" 0
>> +    test_tracepoints $trace_command "(0xabababab & 0x0000ffff) == 0xffff" 0
>> +    test_tracepoints $trace_command "(0xabababab | 0x0000ffff) == 0xeeeedddd" 0
>> +    test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xaaaaaaaa" 0
>> +    test_tracepoints $trace_command "~0xaaaaaaaa == 0x11111111" 0
>> +    test_tracepoints $trace_command "61 < 42" 0
>> +    test_tracepoints $trace_command "42 <= 11" 0
>> +    test_tracepoints $trace_command "11 >= 42" 0
>> +    test_tracepoints $trace_command "11 > 21" 0
>> +    test_tracepoints $trace_command "(66 < 42 ? 0 : 1) == 0" 0 18955_i386_failure
>> +    test_tracepoints $trace_command "(66 <= 42 ? 0 : 1) == 0" 0
>> +    test_tracepoints $trace_command "(11 >= 42 ? 0 : 1) == 0" 0
>> +    test_tracepoints $trace_command "(11 > 21 ? 0 : 1) == 0" 0 18955_i386_failure
>
> I'm surprised to see these split into their own section though.
> I'd think it'd be better to keep each along side the corresponding test,
> to make it easier to keep them in sync:
>
>      test_tracepoints $trace_command "21 + 21 == 42" 10
> +    test_tracepoints $trace_command "21 + 21 == 11" 0
>      test_tracepoints $trace_command "42 - 21 == 21" 10
> +    test_tracepoints $trace_command "42 - 21 == 11" 0
>
> etc.
>
> Could you do that?
>

Yes actually I did that at first, but found it very confusing with so
many similar lines together so I continued using another block.

But like you sugested in patch 5 review I can add a line between each
test case/counter.

I'll do that and repost.

Thanks,
Antoine
  

Patch

diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index b7427ca..f9eaf31 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -157,4 +157,25 @@  foreach trace_command { "trace" "ftrace" } {
     test_tracepoints $trace_command "(42 >= 42 ? 0 : 1) == 0" 10
     test_tracepoints $trace_command "(42 > 21 ? 0 : 1) == 0" 10 18955_i386_failure
     test_tracepoints $trace_command "\$trace_timestamp >= 0" 10
+
+    # Counter-cases tests.
+    test_tracepoints $trace_command "21 + 21 == 11" 0
+    test_tracepoints $trace_command "42 - 21 == 11" 0
+    test_tracepoints $trace_command "21 * 2 == 11" 0
+    test_tracepoints $trace_command "21 << 1 == 11" 0
+    test_tracepoints $trace_command "42 >> 1 == 11" 0
+    test_tracepoints $trace_command "-(21 << 1) == -11" 0
+    test_tracepoints $trace_command "-42 >> 1 == -11" 0
+    test_tracepoints $trace_command "(0xabababab & 0x0000ffff) == 0xffff" 0
+    test_tracepoints $trace_command "(0xabababab | 0x0000ffff) == 0xeeeedddd" 0
+    test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xaaaaaaaa" 0
+    test_tracepoints $trace_command "~0xaaaaaaaa == 0x11111111" 0
+    test_tracepoints $trace_command "61 < 42" 0
+    test_tracepoints $trace_command "42 <= 11" 0
+    test_tracepoints $trace_command "11 >= 42" 0
+    test_tracepoints $trace_command "11 > 21" 0
+    test_tracepoints $trace_command "(66 < 42 ? 0 : 1) == 0" 0 18955_i386_failure
+    test_tracepoints $trace_command "(66 <= 42 ? 0 : 1) == 0" 0
+    test_tracepoints $trace_command "(11 >= 42 ? 0 : 1) == 0" 0
+    test_tracepoints $trace_command "(11 > 21 ? 0 : 1) == 0" 0 18955_i386_failure
 }