Re: [PATCH] testsuite: add testsuite for aarch64 floating point

[Pedro Alves <palves at redhat dot com>]

> [PATCH] testsuite: add testsuite for aarch64 floating point

s/add testsuite/add test/


> Bug 17457: validate correct floating point registers index
> and fpsr/fpcr registers
> 
> gdb/testsuite/
> 2014-10-08  Catalin Udma  <catalin.udma@freescale.com>
> 
>         * gdb.arch/aarch64-fp.c: New file.
>         * gdb.arch/aarch64-fp.exp: New file.

Please add the PR number to the ChangeLog entry, like:

gdb/testsuite/
2014-10-08  Catalin Udma  <catalin.udma@freescale.com>

	PR server/17457
	* gdb.arch/aarch64-fp.c: New file.
	* gdb.arch/aarch64-fp.exp: New file.

Once this is final, please merge it with the patch that fixes the
bug, and push them as a single commit.  (Remember to put the PR
line in the ChangeLog entry of the other patch as well.)

> diff --git a/gdb/testsuite/gdb.arch/aarch64-fp.c b/gdb/testsuite/gdb.arch/aarch64-fp.c
> new file mode 100644
> index 0000000..d217784
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-fp.c
> @@ -0,0 +1,39 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2008-2014 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/>.  */
> +
> +#include <stdio.h>

This seems unnecessary.

> +
> +int main (void)

We try to follow formatting conventions in the tests too, so:

int
main (void)

> +{


> +
> +send_gdb "show endian\n"
> +gdb_expect {
> +    -re "(.* )(big|little)( endian.*)$gdb_prompt $" {
> +        pass "endianness"
> +        set endianness $expect_out(2,string)
> +    }
> +    -re ".*$gdb_prompt $" {
> +        fail "couldn't get endianness"
> +    }
> +    timeout { fail "(timeout) endianness" }
> +}

Please use gdb_test and gdb_test_multiple unless really
necessary.  Write:

set endianness "little"
set test "show endian"
gdb_test_multiple $test $test {
    -re "(.* )(big|little)( endian.*)$gdb_prompt $" {
        set endianness $expect_out(2,string)
        pass "endianness"
    }
}

Defaulting endianness also makes sure that if GDB fails
that test for some reason, we won't get tcl errors due to
using unset variables.  Usually, we'll do

set endianness ""
gdb_test_multiple $test $test {
...
}
if { $endianness=="" } {
   return -1
}

but defaulting here seems a little better to at least
try a few more tests.

> +
> +
> +gdb_test "break ${srcfile}:37" \
> +    "Breakpoint $decimal at 0x\[0-9a-fA-F\]+: file .*${srcfile}, line 37\\\." \
> +    "Set the breakpoint after setting the fp registers"

Use gdb_get_line_number instead of hard coding line numbers.

Also, please use lowercase in the test messages.  "set" ...

> +
> +gdb_test "continue" \
> +    "Continuing.*Breakpoint $decimal.*" \
> +    "Continue until breakpoint"

... "continue", etc., throughout.

> +
> +if {$endianness == "little"} {
> +    set reg_value0 "0x1f1e1d1c1b1a19181716151413121110"
> +    set reg_value1 "0x2f2e2d2c2b2a29282726252423222120"
> +} else {
> +    set reg_value0 "0x101112131415161718191a1b1c1d1e1f"
> +    set reg_value1 "0x202122232425262728292a2b2c2d2e2f"
> +}
> +
> +gdb_test "info registers q0" \
> +    "q0.*{u = $reg_value0, s = $reg_value0.*" \
> +    "Check register q0 value"
> +
> +gdb_test "info registers q1" \
> +    "q1.*{u = $reg_value1, s = $reg_value1.*" \
> +    "Check register q1 value"
> +
> +gdb_test "info registers v0" \
> +    "v0.*$reg_value0}}}" \
> +    "Check register v0 value"
> +
> +gdb_test "info registers v1" \
> +    "v1.*$reg_value1}}}" \
> +    "Check register v1 value"
> +
> +gdb_test "info registers fpsr" \
> +    "fpsr.*0x\[0-9a-fA-F\].*" \
> +    "Check register fpsr value"
> +
> +gdb_test "info registers fpcr" \
> +    "fpcr.*0x\[0-9a-fA-F\].*" \
> +    "Check register fpcr value"
> +

Thanks,
Pedro Alves