Re: [PATCH 6/6] gdb.trace: Add a testcase for tdesc in tfile.

Pedro Alves
2016-02-10T13:50:01Z

On 02/06/2016 08:54 PM, Marcin KoÅ›cielnicki wrote:
> This tests whether $ymm15 can be correctly collected and printed from
> tfile. It covers:
>
...
> - using proper tdesc in encoding tracepoint actions (without that,
> internal error happens due to $ymm15h being considered unavailable)

> This test requires https://sourceware.org/ml/gdb-patches/2016-01/msg00115.html

I'll take a look at that one soon.

>
> gdb/testsuite/ChangeLog | 5 +++
> gdb/testsuite/gdb.trace/tfile-avx.c | 48 ++++++++++++++++++++
> gdb/testsuite/gdb.trace/tfile-avx.exp | 82 +++++++++++++++++++++++++++++++++++

Looks mostly OK. Thanks much for doing all of this. Some comments below.

> 3 files changed, 135 insertions(+)
> create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.c
> create mode 100644 gdb/testsuite/gdb.trace/tfile-avx.exp
>
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 046f112..c9a596c 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-02-06 Marcin KoÅ›cielnicki <koriakin@0x04.net>
> +
> + * gdb.trace/tfile-avx.c: New test.
> + * gdb.trace/tfile-avx.exp: New test.
> +
> 2016-02-04 Yao Qi <yao.qi@linaro.org>
>
> * gdb.base/foll-exec-mode.c: Include limits.h.
> diff --git a/gdb/testsuite/gdb.trace/tfile-avx.c b/gdb/testsuite/gdb.trace/tfile-avx.c
> new file mode 100644
> index 0000000..71107b8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/tfile-avx.c
> @@ -0,0 +1,48 @@
> +/* 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/>. */
> +
> +/*
> + * Test program for reading target description from tfile: collects AVX
> + * registers on x86_64.
> + */
> +
> +#include <immintrin.h>
> +
> +void
> +dummy (void)
> +{}
> +
> +static void
> +end (void)
> +{}
> +
> +int main (void)

Tests should follow GNU/GDB coding conventions too, unless
required for the functionality of the test.

So write:

void
dummy (void)
{
}

int
main (void)

...

> +{
> + register __v8si a asm("ymm15") = {
> + 0x12340001,
> + 0x12340002,
> + 0x12340003,
> + 0x12340004,
> + 0x12340005,
> + 0x12340006,
> + 0x12340007,
> + 0x12340008,
> + };
> + asm volatile ("traceme: call dummy" : : "x" (a));
> + end ();
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/tfile-avx.exp b/gdb/testsuite/gdb.trace/tfile-avx.exp
> new file mode 100644
> index 0000000..1acf4cb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/tfile-avx.exp
> @@ -0,0 +1,82 @@
> +# 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/>.
> +
> +if { ! [istarget "x86_64*-*-*linux*"] } {
> + verbose "Skipping tfile AVX test (target is not x86_64 linux)."
> + return
> +}

Check is_amd64_regs_target instead, to cover i386-linux/-m64

> +
> +load_lib "trace-support.exp"
> +
> +standard_testfile
> +
> +if {[prepare_for_testing $testfile.exp $testfile $srcfile \
> + [list debug nowarnings additional_flags=-mavx]]} {
> + return -1

Why nowarnings?

> +}
> +
> +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
> +}
> +
> +send_gdb "print \$ymm15\n"
> +gdb_expect {
> + -re "\\$\[0-9\]+ = void.*$gdb_prompt \$" {
> + verbose "Skipping tfile AVX test (target doesn't support AVX)."
> + return
> + }
> + -re "\\$\[0-9\]+ = \\{.*}.*$gdb_prompt \$" {
> + # All is well.
> + }
> +}

Please use gdb_test_multiple instead of raw send_gdb/gdb_expect.

No backslash in "$gdb_prompt \$".

Use $decimal, or just drop that "\\$\[0-9\]+" part.

> +
> +gdb_test "trace traceme" ".*"
> +
> +gdb_trace_setactions "set actions for tracepoint" "" \
> + "collect \$ymm15" "^$"
> +
> +gdb_breakpoint "end"
> +
> +gdb_test_no_output "tstart"
> +
> +gdb_test "continue" ".*Breakpoint \[0-9\]+, end .*"

$decimal

> +
> +set tracefile [standard_output_file ${testfile}]
> +
> +# Save trace frames to tfile.
> +gdb_test "tsave ${tracefile}.tf" \
> + "Trace data saved to file '${tracefile}.tf'.*" \
> + "save tfile trace file"
> +
> +# Change target to tfile.
> +set test "change to tfile target"
> +gdb_test_multiple "target tfile ${tracefile}.tf" "$test" {
> + -re "A program is being debugged already. Kill it. .y or n. " {
> + send_gdb "y\n"
> + exp_continue
> + }
> + -re "$gdb_prompt $" {
> + pass "$test"
> + }
> +}

gdb_test handles queries:

gdb_test \
"target tfile ${tracefile}.tf" "" \
"change to tfile target" \
"A program is being debugged already. Kill it. .y or n. $" \
"y"

> +
> +gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
> +
> +gdb_test "print/x \$ymm15.v8_int32" "\\$\[0-9\]+ = \\{0x12340001, .*, 0x12340008}.*"
>

$decimal, or just drop the value number match. Drop .* at the end to tighten
the regex (gdb_test expects \r\n before the prompt).

gdb_test "print/x \$ymm15.v8_int32" " = \\{0x12340001, .*, 0x12340008}"

Thanks,
Pedro Alves