* [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp
2016-05-17 17:03 [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Antoine Tremblay
@ 2016-05-17 17:03 ` Antoine Tremblay
2016-05-27 12:02 ` Pedro Alves
2016-05-17 17:03 ` [PATCH 3/4] Add variable length tests for emit_ref " Antoine Tremblay
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-17 17:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch adds coverage for emit_less_unsigned.
gdb/testsuite/ChangeLog:
* gdb.trace/trace-condition.exp: Add emit_less_unsigned test.
---
gdb/testsuite/gdb.trace/trace-condition.exp | 1 +
1 file changed, 1 insertion(+)
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index fa031ba..f5b59b1 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -151,6 +151,7 @@ foreach trace_command { "trace" "ftrace" } {
test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xffffffff" 10
test_tracepoints $trace_command "~0xaaaaaaaa == 0x55555555" 10
test_tracepoints $trace_command "21 < 42" 10
+ test_tracepoints $trace_command "21U < 42U" 10
test_tracepoints $trace_command "42 <= 42" 10
test_tracepoints $trace_command "42 >= 42" 10
test_tracepoints $trace_command "42 > 21" 10
--
2.8.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp
@ 2016-05-17 17:03 Antoine Tremblay
2016-05-17 17:03 ` [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp Antoine Tremblay
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-17 17:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
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(-)
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
--
2.8.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] Add variable length tests for emit_ref in trace-condition.exp
2016-05-17 17:03 [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Antoine Tremblay
2016-05-17 17:03 ` [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp Antoine Tremblay
@ 2016-05-17 17:03 ` Antoine Tremblay
2016-05-27 12:05 ` Pedro Alves
2016-05-17 17:03 ` [PATCH 4/4] Add tests for 64bit values " Antoine Tremblay
2016-05-27 12:00 ` [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Pedro Alves
3 siblings, 1 reply; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-17 17:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch add variable length tests for emit_ref by reading the variable
passed as argument of 8 to 64 bit.
gdb/testsuite/ChangeLog:
* gdb.trace/trace-condition.c (marker): Adapt signature to 8 to 64
bits types.
(main): Adapt to 8 to 64 bits types.
* gdb.trace/trace-condition.exp: Add new tests.
---
gdb/testsuite/gdb.trace/trace-condition.c | 8 +++++---
gdb/testsuite/gdb.trace/trace-condition.exp | 7 +++++--
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/gdb/testsuite/gdb.trace/trace-condition.c b/gdb/testsuite/gdb.trace/trace-condition.c
index 94bb30c..f0b1863 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.c
+++ b/gdb/testsuite/gdb.trace/trace-condition.c
@@ -16,8 +16,9 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "trace-common.h"
+#include <inttypes.h>
-int globvar;
+int64_t globvar;
static void
begin (void)
@@ -25,7 +26,7 @@ begin (void)
}
static void
-marker (int anarg)
+marker (int8_t arg8, int16_t arg16, int32_t arg32, int64_t arg64)
{
FAST_TRACEPOINT_LABEL(set_point);
}
@@ -41,7 +42,8 @@ main ()
begin ();
for (globvar = 1; globvar < 11; ++globvar)
- marker (globvar * 100);
+ marker (globvar, globvar + (1 << 8), globvar + (1 << 16),
+ globvar + (1LL << 32));
end ();
return 0;
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index f5b59b1..8c770f4 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -132,9 +132,12 @@ foreach trace_command { "trace" "ftrace" } {
test_tracepoints $trace_command "\$$pcreg == *set_point" 10
# Can we read local variables?
- test_tracepoints $trace_command "anarg == 100 || anarg == 200" 2 18955_x86_64_failure
+ test_tracepoints $trace_command "arg8 == 1 || arg8 == 2" 2 18955_x86_64_failure
+ test_tracepoints $trace_command "arg16 == 257 || arg16 == 258" 2 18955_x86_64_failure
+ test_tracepoints $trace_command "arg32 == 65537 || arg32 == 65538" 2 18955_x86_64_failure
+ test_tracepoints $trace_command "arg64 == 4294967297 || arg64 == 4294967298" 2 18955_x86_64_failure
# Can we read global variables?
- test_tracepoints $trace_command "anarg == 100 && globvar == 1" 1 18955_x86_64_failure
+ test_tracepoints $trace_command "arg16 == 257 && globvar == 1" 1 18955_x86_64_failure
# Test various operations to cover as many opcodes as possible.
test_tracepoints $trace_command "21 == 21" 10
--
2.8.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] Add tests for 64bit values in trace-condition.exp
2016-05-17 17:03 [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Antoine Tremblay
2016-05-17 17:03 ` [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp Antoine Tremblay
2016-05-17 17:03 ` [PATCH 3/4] Add variable length tests for emit_ref " Antoine Tremblay
@ 2016-05-17 17:03 ` Antoine Tremblay
2016-05-27 12:08 ` Pedro Alves
2016-05-27 12:00 ` [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Pedro Alves
3 siblings, 1 reply; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-17 17:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch adds tests for emit operations with 64 bit values. It takes
special care to avoid mistakes that one could make on a 32bit architecture
using 64bit values.
gdb/testsuite/ChangeLog:
* gdb.trace/trace-condition.exp: Add 64bit tests.
---
gdb/testsuite/gdb.trace/trace-condition.exp | 36 +++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index 8c770f4..4e129e8 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -163,4 +163,40 @@ 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
+
+ #Test operations with 64 bit operands.
+ test_tracepoints $trace_command "0xabababab00000000 == 0xcdcdcdcd00000000" 0
+ test_tracepoints $trace_command "!0xabababab00000000 == 0" 10
+ test_tracepoints $trace_command "0xababababcdcdcdcd + 0x1111111143434343 == 0xbcbcbcbd11111110" 10
+ test_tracepoints $trace_command "0xababababcdcdcdcd - 0x11111111dc111111 == 0x9a9a9a99f1bcbcbc" 10
+ test_tracepoints $trace_command "0xababababcdcdcdcd * 0xababababcdcdcdcd == 0x55320eeb91c3f629" 10
+ test_tracepoints $trace_command "0x00abababcdcdcdcd << 1 == 0x15757579b9b9b9a" 10
+ test_tracepoints $trace_command "0x00abababcdcdcdcd << 46 == 0x7373400000000000" 10
+ #signed rsh
+ test_tracepoints $trace_command "0x00abababcdcdcdcd >> 1 == 0x55d5d5e6e6e6e6" 10
+ test_tracepoints $trace_command "0x00abababcdcdcdcd >> 46 == 0x2ae" 10
+ #unsigned rsh
+ test_tracepoints $trace_command "0xababababcdcdcdcdUL >> 1 == 0x55d5d5d5e6e6e6e6UL" 10
+ test_tracepoints $trace_command "0xababababcdcdcdcdUL >> 46 == 0x2aeaeUL" 10
+ test_tracepoints $trace_command "-(0x00abababcdcdcdcd << 1) == -0x15757579b9b9b9a" 10
+ test_tracepoints $trace_command "-(0x00abababcdcdcdcd >> 1) == -0x55d5d5e6e6e6e6" 10
+ test_tracepoints $trace_command "(0xababababcdcdcdcd & 0x00000fffffffffff) == 0x00000babcdcdcdcd" 10
+ test_tracepoints $trace_command "(0xababababcdcdcdcd | 0x00000fffffffffff) == 0xababafffffffffff" 10
+ test_tracepoints $trace_command "(0xaaaaaaaaaaaaaaaa ^ 0x5555555555555555) == 0xffffffffffffffff" 10
+ test_tracepoints $trace_command "~0xaaaaaaaaaaaaaaaa == 0x5555555555555555" 10
+ #less unsigned
+ test_tracepoints $trace_command "0x99999999bbbbbbbb < 0xaaaaaaaaaaaaaaaa" 10
+ test_tracepoints $trace_command "0x99999999bbbbbbbb < 0x99999999cccccccc" 10
+ test_tracepoints $trace_command "0x99999999bbbbbbbb <= 0x99999999bbbbbbbb" 10
+ #less signed
+ test_tracepoints $trace_command "0x09999999bbbbbbbb < 0x0aaaaaaaaaaaaaaa" 10
+ test_tracepoints $trace_command "0x09999999bbbbbbbb < 0x09999999cccccccc" 10
+ test_tracepoints $trace_command "0x09999999bbbbbbbb <= 0x09999999bbbbbbbb" 10
+ test_tracepoints $trace_command "0x0aaaaaaaaaaaaaaa > 0x09999999bbbbbbbb" 10
+ test_tracepoints $trace_command "(0xabababab00000000 == 0xcdcdcdcd00000000 ? 1 : 0) == 0" 10
+ test_tracepoints $trace_command "(0xabababab00000000 != 0xcdcdcdcd00000000 ? 1 : 0) == 1" 10
+ test_tracepoints $trace_command "(0x09999999bbbbbbbb < 0x09999999cccccccc ? 1 : 0) == 1" 10 18955_i386_failure
+ test_tracepoints $trace_command "(0x09999999bbbbbbbb <= 0x09999999bbbbbbbb ? 1 : 0) == 1" 10
+ test_tracepoints $trace_command "(0x09999999bbbbbbbb >= 0x09999999bbbbbbbb ? 1 : 0) == 1" 10
+ test_tracepoints $trace_command "(0x0aaaaaaaaaaaaaaa > 0x09999999bbbbbbbb ? 1 : 0) == 1" 10
}
--
2.8.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp
2016-05-17 17:03 [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Antoine Tremblay
` (2 preceding siblings ...)
2016-05-17 17:03 ` [PATCH 4/4] Add tests for 64bit values " Antoine Tremblay
@ 2016-05-27 12:00 ` Pedro Alves
2016-05-27 12:35 ` Antoine Tremblay
3 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-05-27 12:00 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp
2016-05-17 17:03 ` [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp Antoine Tremblay
@ 2016-05-27 12:02 ` Pedro Alves
2016-05-27 12:36 ` Antoine Tremblay
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-05-27 12:02 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
> index fa031ba..f5b59b1 100644
> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
> @@ -151,6 +151,7 @@ foreach trace_command { "trace" "ftrace" } {
> test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xffffffff" 10
> test_tracepoints $trace_command "~0xaaaaaaaa == 0x55555555" 10
> test_tracepoints $trace_command "21 < 42" 10
> + test_tracepoints $trace_command "21U < 42U" 10
Shouldn't we add:
test_tracepoints $trace_command "42U < 21U" 0
?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] Add variable length tests for emit_ref in trace-condition.exp
2016-05-17 17:03 ` [PATCH 3/4] Add variable length tests for emit_ref " Antoine Tremblay
@ 2016-05-27 12:05 ` Pedro Alves
2016-05-27 13:04 ` Antoine Tremblay
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-05-27 12:05 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
> This patch add variable length tests for emit_ref by reading the variable
> passed as argument of 8 to 64 bit.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.trace/trace-condition.c (marker): Adapt signature to 8 to 64
> bits types.
> (main): Adapt to 8 to 64 bits types.
> * gdb.trace/trace-condition.exp: Add new tests.
Looks OK to me, but same comment as previous patches.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] Add tests for 64bit values in trace-condition.exp
2016-05-17 17:03 ` [PATCH 4/4] Add tests for 64bit values " Antoine Tremblay
@ 2016-05-27 12:08 ` Pedro Alves
2016-05-27 12:36 ` Antoine Tremblay
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-05-27 12:08 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
> This patch adds tests for emit operations with 64 bit values. It takes
> special care to avoid mistakes that one could make on a 32bit architecture
> using 64bit values.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.trace/trace-condition.exp: Add 64bit tests.
> ---
> gdb/testsuite/gdb.trace/trace-condition.exp | 36 +++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
> index 8c770f4..4e129e8 100644
> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
> @@ -163,4 +163,40 @@ 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
> +
> + #Test operations with 64 bit operands.
Space after # ? More instances below.
(And same comment as previous patches.)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp
2016-05-27 12:00 ` [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Pedro Alves
@ 2016-05-27 12:35 ` Antoine Tremblay
2016-05-27 12:48 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-27 12:35 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp
2016-05-27 12:02 ` Pedro Alves
@ 2016-05-27 12:36 ` Antoine Tremblay
0 siblings, 0 replies; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-27 12:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
Pedro Alves writes:
> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
>
>
>> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
>> index fa031ba..f5b59b1 100644
>> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
>> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
>> @@ -151,6 +151,7 @@ foreach trace_command { "trace" "ftrace" } {
>> test_tracepoints $trace_command "(0xaaaaaaaa ^ 0x55555555) == 0xffffffff" 10
>> test_tracepoints $trace_command "~0xaaaaaaaa == 0x55555555" 10
>> test_tracepoints $trace_command "21 < 42" 10
>> + test_tracepoints $trace_command "21U < 42U" 10
>
> Shouldn't we add:
>
> test_tracepoints $trace_command "42U < 21U" 0
>
> ?
Yes as previous comments, will do.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] Add tests for 64bit values in trace-condition.exp
2016-05-27 12:08 ` Pedro Alves
@ 2016-05-27 12:36 ` Antoine Tremblay
0 siblings, 0 replies; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-27 12:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
Pedro Alves writes:
> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
>> This patch adds tests for emit operations with 64 bit values. It takes
>> special care to avoid mistakes that one could make on a 32bit architecture
>> using 64bit values.
>>
>> gdb/testsuite/ChangeLog:
>>
>> * gdb.trace/trace-condition.exp: Add 64bit tests.
>> ---
>> gdb/testsuite/gdb.trace/trace-condition.exp | 36 +++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
>> index 8c770f4..4e129e8 100644
>> --- a/gdb/testsuite/gdb.trace/trace-condition.exp
>> +++ b/gdb/testsuite/gdb.trace/trace-condition.exp
>> @@ -163,4 +163,40 @@ 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
>> +
>> + #Test operations with 64 bit operands.
>
> Space after # ? More instances below.
Fixed, thanks.
>
> (And same comment as previous patches.)
Yep.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp
2016-05-27 12:35 ` Antoine Tremblay
@ 2016-05-27 12:48 ` Pedro Alves
2016-05-27 12:51 ` Antoine Tremblay
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-05-27 12:48 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp
2016-05-27 12:48 ` Pedro Alves
@ 2016-05-27 12:51 ` Antoine Tremblay
0 siblings, 0 replies; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-27 12:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] Add variable length tests for emit_ref in trace-condition.exp
2016-05-27 12:05 ` Pedro Alves
@ 2016-05-27 13:04 ` Antoine Tremblay
0 siblings, 0 replies; 14+ messages in thread
From: Antoine Tremblay @ 2016-05-27 13:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
Pedro Alves writes:
> On 05/17/2016 06:03 PM, Antoine Tremblay wrote:
>> This patch add variable length tests for emit_ref by reading the variable
>> passed as argument of 8 to 64 bit.
>>
>> gdb/testsuite/ChangeLog:
>>
>> * gdb.trace/trace-condition.c (marker): Adapt signature to 8 to 64
>> bits types.
>> (main): Adapt to 8 to 64 bits types.
>> * gdb.trace/trace-condition.exp: Add new tests.
>
> Looks OK to me, but same comment as previous patches.
Note that I don't think the same comment applies here as tests check for
1 or 2 frames only. So you could not get an unconditional error.
test_tracepoints $trace_command "arg8 == 1 || arg8 == 2" 2
For example will test 2 values and collect 2 frames max to pass.
Thanks,
Antoine
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-05-27 13:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-17 17:03 [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Antoine Tremblay
2016-05-17 17:03 ` [PATCH 2/4] Add emit_less_unsigned test in trace-condition.exp Antoine Tremblay
2016-05-27 12:02 ` Pedro Alves
2016-05-27 12:36 ` Antoine Tremblay
2016-05-17 17:03 ` [PATCH 3/4] Add variable length tests for emit_ref " Antoine Tremblay
2016-05-27 12:05 ` Pedro Alves
2016-05-27 13:04 ` Antoine Tremblay
2016-05-17 17:03 ` [PATCH 4/4] Add tests for 64bit values " Antoine Tremblay
2016-05-27 12:08 ` Pedro Alves
2016-05-27 12:36 ` Antoine Tremblay
2016-05-27 12:00 ` [PATCH 1/4] Move trace conditions tests from ftrace.exp to trace-condition.exp Pedro Alves
2016-05-27 12:35 ` Antoine Tremblay
2016-05-27 12:48 ` Pedro Alves
2016-05-27 12:51 ` Antoine Tremblay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).