Re: [PATCH] Add CTF support to GDB [5] Add test for CTF function

[Hui Zhu <teawater at gmail dot com>]

Hi Yao and Joel,

Thanks for your review.

On Mon, Jan 14, 2013 at 3:03 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 01/14/2013 11:48 AM, Hui Zhu wrote:
>>
>> --- a/testsuite/gdb.trace/Makefile.in
>> +++ b/testsuite/gdb.trace/Makefile.in
>> @@ -5,7 +5,7 @@ srcdir = @srcdir@
>>
>>   PROGS = ax backtrace deltrace disconnected-tracing infotrace packetlen \
>>         passc-dyn passcount report save-trace tfile tfind tracecmd tsv \
>> -       unavailable while-dyn while-stepping
>> +       unavailable while-dyn while-stepping ctf
>
>
> Looks we should keep the alphabetic order of this list.

Fixed.

>
>> --- /dev/null
>> +++ b/testsuite/gdb.trace/ctf.exp
>> +
>> +#Test "tsave -ctf"
>> +
>> +gdb_test "trace 24" "Tracepoint \[0-9\]+ at .*"
>
>
> We can use 'gdb_get_line_number' instead of hard-code the line number.

Fixed.

>
>> +gdb_trace_setactions "set action for line 24" "" \
>> +       "collect \$local" "^$"
>> +gdb_test "trace 25" "Tracepoint \[0-9\]+ at .*"
>
>
> Likewise.
>
>> +gdb_trace_setactions "set action for line 25" "" \
>> +       "collect \$reg" "^$"
>> +
>> +gdb_test_no_output "tstart"
>> +
>> +gdb_test "break end" "Breakpoint ${decimal} at .*"
>> +gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*"
>> +gdb_test_no_output "tstop"
>> +
>> +gdb_test "tsave -ctf $ctfdir" "Trace data saved to directory
>> \'$ctfdir\'."
>> +
>
>
> What if GDB is built without ctf support?

Because tsave -ctf is not base on the babeltrace.   So it is not a
issue.  Please see the last part of
http://sourceware.org/ml/gdb-patches/2012-12/msg00492.html to see the
introduce about that.

And I added a check unsupport in "target ctf" because it need support
of babeltrace.


>
>> +set ret [exec whereis babeltrace]
>
>
> We should use 'remote_exec host' instead of 'exec' and I am not sure it is
> portable to check 'babeltrace' exist by 'whereis'.

I found an example in gas-defs.exp and update ctf.exp according to gas_version.

>
>> +if { [string compare "babeltrace:" $ret] == 0 } then {
>> +    unsupported "babeltrace check ctf directory"
>> +} else {
>> +    set ret [catch {exec babeltrace $ctfdir} results]
>
>
> remote_exec host 'babeltrace $ctfdir'
>
>> +    if { $ret != 0 } then {
>> +       fail "babeltrace open ctf directory"
>> +       return -1
>> +    }
>> +}
>> +
>> +
>> +#Test "target ctf"
>> +
>> +gdb_test_no_output "set confirm off"
>> +gdb_test_no_output "target ctf $ctfdir"
>> +
>> +gdb_test "tfind 0" ".*Found trace frame 0.*"
>> +gdb_test "tdump" ".*b = 1.*a = 0.*i = 0.*"
>> +gdb_test "print \$b" ".* = 1.*"
>> +gdb_test "print \$a" ".* = 0.*"
>> +gdb_test "print \$i" ".* = 0.*"
>> +
>> +gdb_test "tfind 1" ".*Found trace frame 1.*"
>> +
>> +gdb_test "tfind 2" ".*Found trace frame 2.*"
>> +gdb_test "tdump" ".*b = 2.*a = 1.*i = 1.*"
>> +gdb_test "print \$b" ".* = 2.*"
>> +gdb_test "print \$a" ".* = 1.*"
>> +gdb_test "print \$i" ".* = 1.*"
>> +
>
>
> I don't run it but there must be some duplicated test results in gdb.sum,
> such as "tdump", "print $a", and etc.
>
> It is better to run 'tfind' until there is no trace frame in the trace file,
> to make sure no unexpected trace frame is generated.

Fixed.

>
>> +
>> +#Clean
>> +
>> +exec rm -rf $ctfdir
>
>
> remote_exec host "rm -rf $ctfdir"
> --
> Yao (éå)


And I added copyright header in ctf.c.

Post a new version.  Please help me review it.

Thanks,
Hui

2013-01-15  Hui Zhu  <hui_zhu@mentor.com>

	* gdb.trace/Makefile.in (PROGS): Add ctf.
	* gdb.trace/ctf.c: New file.
	* gdb.trace/ctf.exp: New file.

Attachment: ctf-test.txt
Description: Text document